w3c-ccg / traceability-interop

Verifiable Credentials for Supply Chain Interoperability Specification for HTTP
https://w3id.org/traceability/interoperability
Other
28 stars 10 forks source link

Should "type" allow `string` in addition to `array of string`? #333

Closed brownoxford closed 2 years ago

brownoxford commented 2 years ago

Should the schema nodes that contain a type property allow both string and array of string? I'm referring specifically to these items, but more may be relevant:

This came up because the hyperledger/aries-framework-go project allows for both in a credentials/issue request body, and condenses type arrays of length one to a bare string in the result body.

I did some poking around and found the following:

trace-interop defines credential.type as array of string https://github.com/w3c-ccg/traceability-interop/blob/main/docs/openapi/schemas/Credential.yml

vc-api defines credential.type as array of string https://github.com/w3c-ccg/vc-api/blob/main/components/Credential.yml

vc-data-model is not explicit, but suggests array usage in all examples and has multiple elements in the array https://w3c.github.io/vc-data-model/#types

vc-imp-guide shows examples with bare string in addition to string array https://w3c.github.io/vc-imp-guide/

json-ld allows single string or string array for specifying node type using @type https://www.w3.org/TR/json-ld/#specifying-the-type

hyperledger accepts either string or array of string converts to array of string internally converts to single string on output if there is only one array element

I'm proposing that we modify the definition of type for the Credential and Presentation schema items to allow for bare string values.

Existing schema:

  type:
    type: array
    items:
      type: string

Proposed schema:

  type:
    oneOf:
    - type: string
    - type: array
      items:
        type: string
mkhraisha commented 2 years ago

I'm a +1 to the proposed schema , its a relatively small change but it ends up making it pretty easy to be interoperable with a bigger audience

nissimsan commented 2 years ago

This seems to be going in the opposite direction of this: https://github.com/w3c-ccg/traceability-vocab/pull/524

TallTed commented 2 years ago

I believe the reason for going array-only via https://github.com/w3c-ccg/traceability-vocab/pull/524 was that Transmute tools didn't/couldn't/wouldn't handle oneOf.

https://github.com/w3c-ccg/traceability-vocab/pull/524 having been settled by removing the oneOf and forcing array, I think that this issue going the other way sets up an obvious conflict between the two repos (vocab and interop).

We might try to get all parties who feel strongly about this onto one call to hash it out... or we might just want to accept the resolution of https://github.com/w3c-ccg/traceability-vocab/pull/524, and go array-only.

brownoxford commented 2 years ago

I will leave the "ready-for-pr" label, but please continue to comment with your thoughts on this.

Please note that we recently committed PR #319 which allows error response property details to be either string, []string or object. Whichever way we go on this, we should be eventually consistent with how we treat these kind of convenience specifications.

My $0.02 from a coding standpoint is that it is less complex to go from string or []string to json than it is to go the other way. I would propose that we put the burden of the more complex code on the implementors of these endpoints than on the consumers, as there are likely to be more consumers than implementors.

If someone from Transmute could please add context regarding the oneOf handling issues that @TallTed mentioned, that would be helpful (@BenjaminMoe @nissimsan).

We should also make sure that we update documentation to be explicit about the choices made here, and not rely solely on the openapi spec.

brownoxford commented 2 years ago

We seem to be moving away from allowing both string and []string for items. I propose we close this issue.

nissimsan commented 2 years ago

Agreed on the call, let's not do this.