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

dcql_query & presentation_definition are not strings #422

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

jogu
Copy link
Collaborator

@jogu jogu commented Feb 11, 2025

When putting JSON into JSON for inclusion in request objects, you should definitely not encode it as a string first.

Also switch one of the examples from PE to DCQL as currently the only example showing dcql_query parameter is in DC API appendix.

closes #378

When putting JSON into JSON for inclusion in request objects,
you should definitely not encode it as a string first.

Also switch one of the examples from PE to DCQL as currently
the only example showing dcql_query parameter is in DC API
appendix.

closes #378
@jogu jogu force-pushed the dcql-is-json-object branch from 3743c4b to 7136c9a Compare February 11, 2025 09:44
Copy link
Member

@TimoGlastra TimoGlastra left a comment

Choose a reason for hiding this comment

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

Thanks! :)

examples/request/request.txt Outdated Show resolved Hide resolved
@@ -280,16 +280,18 @@ One exception to this rule is `transaction_data` parameter, and the wallets that
This specification defines the following new request parameters:

`presentation_definition`:
: A string containing a Presentation Definition JSON object. See (#request_presentation_definition) for more details.
: A JSON object containing a Presentation Definition. See (#request_presentation_definition) for more details.
Copy link
Contributor

@danielfett danielfett Feb 11, 2025

Choose a reason for hiding this comment

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

I don't think this change is helpful. This is not clear enough IMO to convey that the encoding varies between the query string (where it is a JSON-encoded string) and the request object and DC API call (where it is an object and it is not additionally JSON-encoded).

Copy link
Contributor

Choose a reason for hiding this comment

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

I sort of understand this perspective @danielfett, however I'm struggling to see what other interpretation a developer might have about how to convey a JSON object in a query parameter value?

Copy link
Collaborator Author

@jogu jogu Feb 12, 2025

Choose a reason for hiding this comment

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

@danielfett Are we at least agreed the current text is bad? It has resulted in people doing this in request objects:

"dcql_query": "{\"id\":\"ffc717a3-abaf-4ec3-9c55-a9b8e998874c\",\"name\":\"A

So this is hopefully an improvement at least. The way I've done it here (combined with the text below explaining how to put JSON objects into the url query) seems consistent with how it's done in RAR ( https://datatracker.ietf.org/doc/html/rfc9396#section-2 ), and we have the example to try and help people struggling to interpret it too.

Co-authored-by: Daniel Fett <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Should dcql_query and presentation_definition be a string when using JAR?
5 participants