eclipse-rdf4j / rdf4j

Eclipse RDF4J: scalable RDF for Java
https://rdf4j.org/
BSD 3-Clause "New" or "Revised" License
364 stars 163 forks source link

REST Transaction API should model each action as a separate endpoint resource #4492

Open abrokenjester opened 1 year ago

abrokenjester commented 1 year ago

Problem description

Currently, the RDF4J REST API enables transaction operations by means of a single "excute operation" endpoint that takes a great variety of different query parameters to control what specific operation is being executed. This is somewhat against the REST architectural style and also makes it hard to provide technical (OpenAPI) documentation that clearly explains how each operation works, and what combination of query params can be used.

Preferred solution

Instead of specifying the action as a query param, we should add separate endpoints for each action, e.g:

POST /transaction/<id>/add 
POST /transaction/<id>/query
POST /transaction/<id>/update

Are you interested in contributing a solution yourself?

Yes

Alternatives you've considered

No response

Anything else?

No response

pulquero commented 1 year ago

I thought REST architectural style meant HTTP verbs should be mapped to actions and URLs represent resources.

abrokenjester commented 1 year ago

I thought REST architectural style meant HTTP verbs should be mapped to actions and URLs represent resources.

It does, yes. But it all depends on what you define as a resource, and what as an action. In the current design, the transaction itself (identified by /transaction/<id>) is the resource, and an operation on that transaction is executed by a HTTP PUT, using the action parameter to specify the type of action. You can think of the way we model transactions as a "receipt" almost, with each operation executed on the transaction as an additional line item on that receipt.

One reason this is (slightly) at odds with RESTful design is that PUT is supposed to be an idempotent operation, and its semantics conventionally are creation or complete replacement of the identified resource. But we are not creating or replacing the transaction, we are modifying its current state.

The design proposed on this ticket treats each kind of transaction operation as a resource in its own right. So the resource we are manipulating is not the transaction itself, but the transaction operations. Each transaction operation is done by creating a new (unnamed) "operation resource" as part of the transaction. We'd likely use POST instead of PUT for this, as we are no longer manipulating an identified resource, but are adding additional items of a type of resource.

You could argue that that is a fine (and perhaps not very important) distinction. The other reason I have for changing this is more pragmatic: by separating out the actions as independent endpoints/resources, we can make our API documentation much clearer. If you look at our current OpenAPI doc for the transaction action endpoint, you'll see that it has a large number of possible parameters and various different response types, and it's very unclear which combinations of actions and parameters make sense. Apart from docs, creating separate endpoints also gives a more maintainable representation in code, with the possibility of separate controllers for each action instead of one "god class" transaction controller (not saying we couldn't break up that god class without this, but it would more naturally align with the API design).

pulquero commented 1 year ago

From a consumer point of view, it would be good if the transaction API was the same as the non-transaction API. That is, you have your conventional query/update endpoint, with /statements subpath etc. This is a bit similar to the existing design, but you just reproduce the non-tx url structure instead of overloading everything into an action parameter.

abrokenjester commented 1 year ago

From a consumer point of view, it would be good if the transaction API was the same as the non-transaction API. That is, you have your conventional query/update endpoint, with /statements subpath etc. This is a bit similar to the existing design, but you just reproduce the non-tx url structure instead of overloading everything into an action parameter.

That is an interesting idea - let me mull that over for a bit.

benherber commented 6 months ago

Hey we were also looking into this as we've run into some confusion with the api as well. I see this is slated for 5.0.0, is that still the case? If not, and we would want to contribute it back post-5.x.x, how would we go about doing that? Would we be comfortable doing restful versioning (i.e., all the new routes behind a new /v5/ prefix) and deprecate the old routes to allow people to migrate at their own pace?