Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Accept contentType params for DID URL Resolution [DEV-4722] #325

Open
wants to merge 15 commits into
base: develop
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 46 additions & 0 deletions services/helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package services

import (
"net/url"
"strconv"
"strings"

"github.com/cheqd/did-resolver/types"
Expand Down Expand Up @@ -39,3 +40,48 @@ func PrepareQueries(c echo.Context) (rawQuery string, flag *string) {
func GetDidParam(c echo.Context) (string, error) {
return url.QueryUnescape(c.Param("did"))
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A separate note about function and variable names here: this is not getting the "header". It's specifically the Accept header. So it could be misleading that a lot of the function and variable names are called "header". I would suggest renaming functions and variables to call them "acceptHeader" or similar. (There's a similar comment I have made about variables called contentType below.)

func GetHighestPriorityHeaderAndParams(acceptHeader string) (string, map[string]string) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Error handling

This should work...as long as the q values are correctly within the 0.0 to 1.0 range. If they are not, there's no error handling in here to reject such values, e.g., if a client mistakenly set q to 2.0 the current code will happily take that as highest priority. I would suggest modifying this and adding better error handling. I wouldn't per se reject the request/connection entirely as this is too aggressive, but I'd suggest stripping out invalid/out-of-range q values and then re-assessing the priority order as if those invalid q values did not exist. For example, if the header was:

Accept : text/html, application/xml;q=2.9, */*;q=0.8

...the value 2.9 is non-sensical. Strip it out, and this becomes:

Accept : text/html, application/xml, */*;q=0.8

And then your current code works.

Where should this be called

Currently, seems to be called specifically when processing queries for DLRs. Should this not be done across the board for any request? Since this kind of logic theoretically applies not just to resource requests, but to all requests (including those for DID resolution).

bestHeader := ""
bestParams := make(map[string]string)
highestQ := -1.0
position := 0

// Split by ',' to separate multiple headers
headers := strings.Split(acceptHeader, ",")
for index, entry := range headers {
entry = strings.TrimSpace(entry)
parts := strings.Split(entry, ";")

params := make(map[string]string)
q := 1.0

// Parse parameters
for _, param := range parts[1:] {
param = strings.TrimSpace(param)
keyValue := strings.SplitN(param, "=", 2)
if len(keyValue) == 2 {
key, value := keyValue[0], strings.Trim(keyValue[1], "\"")
params[key] = value
}
}

// Extract q value if present
if qStr, exists := params["q"]; exists {
if parsedQ, err := strconv.ParseFloat(qStr, 64); err == nil {
q = parsedQ
}
}

// Determine the highest priority header
if q > highestQ || (q == highestQ && position > index) {
highestQ = q
position = index
bestHeader = parts[0] // Only header
delete(params, "q") // Remove q from params
bestParams = params // Update best parameters
}
}

return bestHeader, bestParams
}
8 changes: 8 additions & 0 deletions services/resource/echo_handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package resources

import (
"github.com/cheqd/did-resolver/services"
"github.com/cheqd/did-resolver/types"
"github.com/labstack/echo/v4"
)

Expand All @@ -22,6 +23,13 @@ import (
// @Failure 501 {object} types.IdentityError
// @Router /{did}/resources/{resourceId} [get]
func ResourceDataEchoHandler(c echo.Context) error {
// Get Accept header
header, params := services.GetHighestPriorityHeaderAndParams(c.Request().Header.Get(echo.HeaderAccept))

if header == string(types.JSONLD) && params["profile"] == types.W3IDDIDURL {
return services.EchoWrapHandler(&ResourceDataWithMetadataDereferencingService{})(c)
}

return services.EchoWrapHandler(&ResourceDataDereferencingService{})(c)
}

Expand Down
54 changes: 54 additions & 0 deletions services/resource/resource_data_with_metadata_dereferencing.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
package resources

import (
"net/http"

"github.com/cheqd/did-resolver/migrations"
"github.com/cheqd/did-resolver/services"
"github.com/cheqd/did-resolver/types"
"github.com/cheqd/did-resolver/utils"
)

type ResourceDataWithMetadataDereferencingService struct {
services.BaseRequestService
ResourceId string
}

func (dr *ResourceDataWithMetadataDereferencingService) Setup(c services.ResolverContext) error {
dr.IsDereferencing = true
return nil
}

func (dr *ResourceDataWithMetadataDereferencingService) SpecificPrepare(c services.ResolverContext) error {
dr.ResourceId = c.Param("resource")
return nil
}

func (dr ResourceDataWithMetadataDereferencingService) Redirect(c services.ResolverContext) error {
migratedDid := migrations.MigrateDID(dr.GetDid())

path := types.RESOLVER_PATH + migratedDid + types.RESOURCE_PATH + dr.ResourceId
return c.Redirect(http.StatusMovedPermanently, path)
}

func (dr *ResourceDataWithMetadataDereferencingService) SpecificValidation(c services.ResolverContext) error {
if !utils.IsValidUUID(dr.ResourceId) {
return types.NewInvalidDidUrlError(dr.ResourceId, dr.RequestedContentType, nil, dr.IsDereferencing)
}

// We not allow query here
if len(dr.Queries) != 0 {
return types.NewInvalidDidUrlError(dr.GetDid(), dr.RequestedContentType, nil, dr.IsDereferencing)
}
return nil
}

func (dr *ResourceDataWithMetadataDereferencingService) Query(c services.ResolverContext) error {
result, err := c.ResourceService.DereferenceResourceDataWithMetadata(dr.GetDid(), dr.ResourceId, dr.GetContentType())
if err != nil {
err.IsDereferencing = dr.IsDereferencing
return err
}

return dr.SetResponse(result)
}
33 changes: 30 additions & 3 deletions services/resource_dereference_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ import (

"strings"

resourceTypes "github.com/cheqd/cheqd-node/api/v2/cheqd/resource/v2"
"github.com/cheqd/did-resolver/types"
)

Expand Down Expand Up @@ -36,9 +35,9 @@ func (rds ResourceService) DereferenceResourceMetadata(did string, resourceId st
context = types.ResolutionSchemaJSONLD
}

contentStream := types.NewDereferencedResourceListStruct(did, []*resourceTypes.Metadata{resource.Metadata})
metadata := types.NewDereferencedResource(did, resource.Metadata)

return &types.ResourceDereferencing{Context: context, ContentStream: contentStream, DereferencingMetadata: dereferenceMetadata}, nil
return &types.ResourceDereferencing{Context: context, Metadata: metadata, DereferencingMetadata: dereferenceMetadata}, nil
}

func (rds ResourceService) DereferenceCollectionResources(did string, contentType types.ContentType) (*types.ResourceDereferencing, *types.IdentityError) {
Expand Down Expand Up @@ -79,3 +78,31 @@ func (rds ResourceService) DereferenceResourceData(did string, resourceId string

return &types.ResourceDereferencing{ContentStream: &result, DereferencingMetadata: dereferenceMetadata}, nil
}

func (rds ResourceService) DereferenceResourceDataWithMetadata(did string, resourceId string, contentType types.ContentType) (*types.ResourceDereferencing, *types.IdentityError) {
dereferenceMetadata := types.NewDereferencingMetadata(did, contentType, "")

resource, err := rds.ledgerService.QueryResource(did, strings.ToLower(resourceId))
if err != nil {
err.ContentType = contentType
return nil, err
}

var context string
if contentType == types.DIDJSONLD || contentType == types.JSONLD {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bit confused as to why this contentType is relevant for dereferencing? These are purely about DID Resolution?

As in...if it's a resource, will it ever be these two content types?

Side note: we seem to be using a lot of different terms: sometimes header (see previous comment on "header" vs "accept header"), sometimes contentType like here). It's quite variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should populdate context if it's jsonLD right. This condition is for that.

context = types.ResolutionSchemaJSONLD
}

dereferenceMetadata.ContentType = types.ContentType(resource.Metadata.MediaType)

var result types.ContentStreamI
result = types.NewDereferencedResourceData(resource.Resource.Data)
metadata := types.NewDereferencedResource(did, resource.Metadata)
if dereferenceMetadata.ContentType == types.JSON || dereferenceMetadata.ContentType == types.TEXT {
if res, err := types.NewResourceData(resource.Resource.Data); err == nil {
result = res
}
}

return &types.ResourceDereferencing{Context: context, ContentStream: result, Metadata: metadata, DereferencingMetadata: dereferenceMetadata}, nil
}
34 changes: 34 additions & 0 deletions tests/integration/rest/resource/data/positive_test.go
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unless I'm missing something, I don't see a test case here that would logically return "Accept: application/ld+json;profile="https://w3id.org/did-url-dereferencing" → Returns resource + metadata in a single response", since both of the test cases defined here would just return the resource which is JSON?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT: Maybe just needs better test descriptions

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would also like to see a test case where the Accept header is exactly the same string that a browser like Chrome sends, and cross-checking the results (since it doesn't have the DID URL dereferencing profile, you should not get resource + metadata back)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They are added under data_with_metadata folder

Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,38 @@ var _ = DescribeTable("Positive: Get resource data", func(testCase utils.Positiv
ExpectedStatusCode: http.StatusOK,
},
),

Entry(
"can get resource with an existent DID, with multiple headers without q values",
utils.PositiveTestCase{
DidURL: fmt.Sprintf(
"http://%s/1.0/identifiers/%s/resources/%s",
testconstants.TestHostAddress,
testconstants.UUIDStyleTestnetDid,
testconstants.UUIDStyleTestnetDidResourceId,
),
ResolutionType: "*/*," + string(types.JSONLD) + ";profile=" + string(types.W3IDDIDURL),
EncodingType: testconstants.DefaultEncodingType,
ExpectedEncodingType: "gzip",
ExpectedJSONPath: "../../testdata/resource_data/resource.json",
ExpectedStatusCode: http.StatusOK,
},
),

Entry(
"can get resource with an existent DID, with multiple header and q values",
utils.PositiveTestCase{
DidURL: fmt.Sprintf(
"http://%s/1.0/identifiers/%s/resources/%s",
testconstants.TestHostAddress,
testconstants.UUIDStyleTestnetDid,
testconstants.UUIDStyleTestnetDidResourceId,
),
ResolutionType: string(types.JSONLD) + ";profile=" + string(types.W3IDDIDURL) + ";q=0.5,application/json,text/plain;q=0.9,image/png;q=0.7",
EncodingType: testconstants.DefaultEncodingType,
ExpectedEncodingType: "gzip",
ExpectedJSONPath: "../../testdata/resource_data/resource.json",
ExpectedStatusCode: http.StatusOK,
},
),
)
Loading
Loading