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

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

Open
abrokenjester opened this issue Mar 18, 2023 · 5 comments · May be fixed by #4519
Open

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

abrokenjester opened this issue Mar 18, 2023 · 5 comments · May be fixed by #4519
Assignees
Labels
📶 enhancement issue is a new feature or improvement
Milestone

Comments

@abrokenjester
Copy link
Contributor

abrokenjester commented Mar 18, 2023

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

@abrokenjester abrokenjester added the 📶 enhancement issue is a new feature or improvement label Mar 18, 2023
@abrokenjester abrokenjester changed the title REST Transaction protocol should model separate operations as separate endpoint resources REST Transaction API should model each action as a separate endpoint resource Mar 18, 2023
@abrokenjester abrokenjester self-assigned this Mar 18, 2023
@abrokenjester abrokenjester added this to the 4.3.0 milestone Mar 18, 2023
@abrokenjester abrokenjester linked a pull request Apr 24, 2023 that will close this issue
5 tasks
@abrokenjester abrokenjester linked a pull request Apr 24, 2023 that will close this issue
5 tasks
@pulquero
Copy link

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

@abrokenjester
Copy link
Contributor Author

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
Copy link

pulquero commented May 1, 2023

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
Copy link
Contributor Author

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.

@hmottestad hmottestad modified the milestones: 4.3.0, 5.0.0 May 19, 2023
@benherber
Copy link

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📶 enhancement issue is a new feature or improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants