Our API endpoint URIs are currently ad hoc and inconsistent, e.g., /api/layout/..., /xb-components, and /xb/api/.... We need a stable, predictable convention.
Create a convention that observes generally-accepted industry standards and respects Drupal-specific conventions:
REST API Tutorial URI Naming Conventions and Best Practices has a good overview. API Stylebook Design Guidelines links to concrete precedents.
We need to make sure we don't collide or conflict with other common patterns or solutions in Drupal. For example, it would be begging for problems to do anything under /api/, which someone somewhere is surely already using. We should also avoid /xb/ if we're going to use that path for administrative UI routes.
We should also consider any other paths the module uses and make sure all of them make sense together.
I could imagine, for example, something like this:
/xb/api/{resources} [ /{resourceId} [ /{subCollections} [ /{resourceId} ] ] ]
Show commandsStart within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
changes
Comments
Comment #2
traviscarden commentedComment #3
traviscarden commentedComment #4
wim leersWFM in general, with one exception:
-1 to versioning the API at this time, because that implies it's a public API. It is not. Defining a public API for this that we provide BC for is definitely not in scope.
If you really want to have it, then let's use:
(with
v0matching XB's major version:0.x)P.S.: I do not understand why
matters?
/xb-api/…vs/xb/api/…are essentially the same?Comment #5
effulgentsia commentedInstead of
/v0/, perhaps we can make it even clearer that it's an internal API by naming it/internal/?Comment #6
wim leers#5++
Comment #7
traviscarden commentedRe: versioning the API, I only proposed it to consider because it's a common industry pattern. If we're not making any stability promises for contrib modules to use it, for example, I see no reason for it. In that case, I don't think we need
/internal/either.As to avoiding
/xb/, I wanted to prevent "namespace" collisions, since we already have some routes at/xb/. But having considered it a little more, I don't think I'm actually worried about it as long as we don't anticipate wanting to use/xb/api/for anything else. In fact, there would be a certain elegance in it if we can keep everything under/xb/. Perhaps we could do something like/xb/api/and/xb/routes/or similar.I'm updating the issue description accordingly.
Comment #8
larowlanI agree with @traviscarden - versioning is a standard practice. It also means we can evolve over time. If we decide the API for an endpoint changes, we start a new version and emit deprecations from the old one.
Another alternative practise is to require consumers to set the version # via an accept header - e.g. github has
Accept: application/vnd.github.v3+jsonI think it's easier to just put it in the URL, so plus one for
v0Comment #9
wim leersThe URI naming was made consistent in #3499703: Make all XB HTTP API routes consistently prefixed, ensure they all have OpenAPI specs, and tests to keep it so 😄
That also means that versioning it today would be trivial.
@traviscarden, would you like to take that on? :D
Comment #10
traviscarden commentedOn it, @wim leers!
Comment #12
traviscarden commentedI started to create a static helper to centralize the path generation logic (because it is a little inconsistent), but most of the API path strings were in YAML and JavaScript files, so once I decided not to use it in test classes (because it would make them less expressive), there weren't enough uses left to justify its existence. I'm basically explaining why I didn't have an MR in the 5 minutes it took to do a search-and-replace. 😛 Anyway, here it is. ^
Comment #14
wim leersThanks!
Comment #15
partyka commentedThere's another merge conflict, working on resolving it.
Comment #16
wim leersDxRouteConsistencyTestis not currently passing — once it is, I'm looking forward to merging this, and I'm sure @larowlan does too 😄Comment #18
omkar-pd commentedComment #19
wim leersThanks so much! Let’s land this today so y’all can stop chasing HEAD! 🙈
Comment #20
wim leersComment #22
wim leers