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

confusion around endpoint naming #97

Closed
mprorock opened this issue Feb 3, 2021 · 8 comments
Closed

confusion around endpoint naming #97

mprorock opened this issue Feb 3, 2021 · 8 comments
Assignees

Comments

@mprorock
Copy link
Contributor

mprorock commented Feb 3, 2021

In reviewing and working with a few things off the api, our team noticed that the current spec is inconsistent with RESTful specs and APIs that we have encountered in the past. See the REST api guide for the baseline styles for naming that we are used to encountering.

a concrete example; with an endpoint like
/credentials/issue
the plural indicates to a developer that the noun credential would be issued (verb: issue) in multiple due to the plural use of the word "credentials". the behavior however, is to issue a single credential (which is indicated in the tag, but not the RESTful path).
What i would have expected would be for the path to the method to be /credential/issue to issue a single credential, then in cases where multiple credentials were to be issued, to use the path /credentials/issue as it is written now.

quoting out from the docs linked above with a little commentary

when using the controller pattern(which I believe to be the case in these methods on the vc-http-api), singular still applies, as it does to collections:
http://api.example.com/cart-management/users/{id}/cart/checkout
http://api.example.com/song-management/users/{id}/playlist/play
if controlling only a single type of obj, without referencing a collection (in the above example "users", then it is base-url/function-group/noun/verb

any insight into why this is the case?

@OR13
Copy link
Contributor

OR13 commented Feb 3, 2021

I would not expect routes with singular resource names.... I would expect:

In collection style:

POST /credentials (issue 1 or more credentials)
GET /credentials/{id} (get 1 credential)
GET /credentials (get multiple credential)

In controller style:

POST /issue/credentials (issue 1 or more credentials)

@mprorock
Copy link
Contributor Author

mprorock commented Feb 3, 2021

@OR13 also ok with this approach in general and like it quite a bit - down side there is that overloading is not super well supported in openapi 3.0 from a clarity standpoint and it can lead to confusion in certain scenarios with sets of parameters to the api

in that case, taking an array/list by default to the post has worked well for us in the past, caveat being that you may only be posting one item in that array.

edit to add:
concern on either approach possibly breaking some existing implementations depending on how they were written? how much in production is this version of the API?

@OR13
Copy link
Contributor

OR13 commented Feb 8, 2021

given the pain of endpoint overloading, I would suggest we simply make new route names and avoid endpoint overloading.

msporny pushed a commit that referenced this issue Jul 25, 2021
Signed-off-by: Sam Hellawell <sshellawell@gmail.com>
@msporny
Copy link
Contributor

msporny commented Jan 18, 2022

Discussed on 2022-01-18 telecon:

The group decided that forcing the URL endpoints to be at the root of a domain was an anti-pattern.

The group seems to have decided on a .../noun/verb pattern, but @mprorock is concerned that this might not have been the right direction to go in.

We will continue discussion in this issue.

@mprorock
Copy link
Contributor Author

We should likely revisit the updated best practices and design guide to make sure we are in line here for the long term

@msporny
Copy link
Contributor

msporny commented Mar 20, 2022

This seems related to #271.

@OR13
Copy link
Contributor

OR13 commented Mar 21, 2022

suggest closing.

@mprorock
Copy link
Contributor Author

resolved in PR #271

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

No branches or pull requests

3 participants