Make API compositional and use style guide naming convention.

[dlongley]

This PR changes the name for the issuing endpoint and the compose endpoint to fall in line with the proposed style guide naming conventions. It also changes the compose API to only compose a credential -- the output of which can then be sent to the issue API. IMO, this gives us a cleaner separation of concerns and better composition API/reuse.

[mavarley]

This appears to be in line with the style guide where HTTP Methods are preferred over defining specific endpoints that would achieve the same.

[peacekeeper]

I pretty much disagree with both changes in this PR, which should probably be broken up into two separate PRs.

1. It makes no sense to me to have an issuing API that only composes but doesn't sign a credential. When would you use that? This should be a single operation, as it was in v0.0.1.
2. I agree with the naming convention, but with this PR the naming convention is applied to only one of the endpoints. Both endpoints are for issuing a credential. It's inconsistent style to name one /credentials and the other one /credentials/compose. Either use nouns in both cases, or verbs in both cases.

Also, we have no experience yet whether passing in a fully constructed credential to the API (new endpoint added in v0.0.2) or letting the API construct a credential (original endpoint in v0.0.1) will be the more common case. Until we learn more about that, we shouldn't name the endpoints in a way that gives the impression that one is preferred or more important than the other.

[troyronda]

I’m also concerned with the idea of "already composed" mode is being at the root of /credentials path while "compose" is in a subpath. I do like shortening the names (e.g., /credentials/compose is an improvement).

[OR13]

Instead of continuing to debate on this PR, I suggest we close this, and address the API style guide first... we appear to have some pretty fundamental disagreements regarding design, which need to be addressed first.

#42
https://github.com/w3c-ccg/vc-issuer-http-api/issues/41

[dlongley]

@troyronda,

I’m also concerned with the idea of "already composed" mode is being at the root of /credentials path while "compose" is in a subpath.

The reason for this is to follow restful naming convention per the proposed style guide. The /credentials/compose API proposed here does not create or store a resource on the server. It is a controller that performs a function. The /credentials API creates a credential reference resource on the server after issuing the credential.

@peacekeeper,

It makes no sense to me to have an issuing API that only composes but doesn't sign a credential. When would you use that?

You'd use it to compose the credential right before you submit it to the /credentials endpoint that issues it. That allows users to use the template function, if desired and it is supported. Just like you would call two different local functions, where the first function is optional. This matches the requirements (and optionality) of the API.

[troyronda]

It also changes the compose API to only compose a credential

@dlongley I see. I didn't understand from the API description that you were constructing the flow this way.

Composes a credential and returns it in the response body. Support of this part of the API is OPTIONAL for implementations.

[troyronda]

dismissing my review.

[troyronda]

The /credentials API creates a credential reference resource on the server after issuing the credential.

So if I understand the proposal from @dlongley properly:

• issuance implies creating a credential reference on the server.
• compose does not issue (nor store a reference on the server).
• client can't issue without creating a reference on the server with the current API.

Due to this last assumption in the proposal, /credentials is being used for both issuance and storage. (rather than having /credentials only for storage along with /credentials/issue and /credentials/compose for their operations).

[dlongley]

@troyronda,

That's right. Nothing is stored on the server without it also having been issued and there's no option to only issue a credential without storing a reference to it. If you want or need to use another service to compose the credential via a template/other options before submitting it to be issued, the /credentials/compose method can be used if made available by the implementer.

[troyronda]

@dlongley Ok that makes sense under those assumptions.

Are there alternative situations where:

• a separate system is responsible for storing the credential references? if so, they might want a /credentials that has a fully issued credential in the payload.
• someone is not planning to store the credential reference.

(those scenarios could impact the consistency of a combined issue + store operation on /credentials)

[troyronda]

Noted the atomicity of issuance and credential reference discussion in https://github.com/w3c-ccg/vc-issuer-http-api/issues/45

[llorllale]

I don't like the direction this PR takes. Composability should be carefully weighed against performance with these network calls.

The overall picture isn't clear to me yet, but I'm on the same boat as @peacekeeper for now - I don't see the usefulness of this particular decomposition ("issuance" + "saving"). And there will be functional overlaps (like "linting").

[OR13]

I'm in favor of closing this PR, getting better alignment on the API style guide and merging the specs so we don't have as many places to look, split focus on these API design issues.

[llorllale]

Would've needed to rename the schema type btw.

[dlongley]

This is on hold until after some interop has occurred on the current version. This can be picked up later as more is learned from implementation experience on the current version.

[OR13]

I suggest closing this PR, and waiting till we are in 1 repo before attempting any further changes.

[OR13]

I am closing this PR, we need to merge the specs.