dcql_query & presentation_definition are not strings #422
Conversation
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
force-pushed
the
dcql-is-json-object
branch
from
3743c4b to
7136c9a
Compare
Co-authored-by: Christian Bormann <[email protected]>
| @@ -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. | |||
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
When this is a DC API call, this is an object (not JSON-encoded).
| : A JSON object containing a Presentation Definition. See (#request_presentation_definition) for more details. | |
| : An object containing a Presentation Definition. See (#request_presentation_definition) for more details. |
There was a problem hiding this comment.
If we're going to make that change for that reason I suggest doing it consistently everywhere we use the term 'JSON object' in a separate PR.
Co-authored-by: Daniel Fett <[email protected]>
|
|
||
| `presentation_definition_uri`: | ||
| : A string containing an HTTPS URL pointing to a resource where a Presentation Definition JSON object can be retrieved. See (#request_presentation_definition_uri) for more details. | ||
|
|
||
| `dcql_query`: | ||
| : A string containing a JSON-encoded DCQL query as defined in (#dcql_query). | ||
| : A JSON object containing a DCQL query as defined in (#dcql_query). |
There was a problem hiding this comment.
As above
| : A JSON object containing a DCQL query as defined in (#dcql_query). | |
| : An object containing a DCQL query as defined in (#dcql_query). |
Co-authored-by: Daniel Fett <[email protected]>
|
Confirmed directly with Daniel that he's happy for this to be merged, I transferred his comment about JSON object into a new issue: #446 4 approvals & open a few weeks - merging! |
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