-
Notifications
You must be signed in to change notification settings - Fork 50
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
Initial draft of the major area of the rest contract #4
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This proposal looks good to me.
I noted several areas where additional examples would be useful to provide.
- GET | ||
Returns a single entity. | ||
|
||
- HEAD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this checking for existence or is this a mechanism to check for authorization? An example would be useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have opened a dedicated issue to discuss that #5
- PUT | ||
Replaces the state of the target resource with the supplied request body. | ||
|
||
- PATCH |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An example of PUT/PATCH would be useful to see. In particular, I am curious to see your thoughts for item updates. What components/relationships (metadata, item state, other relationships) would be handled by each http request type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use PATCH to update the value of a metadata without touch (and send) the information about the metadatafield, place, authority, etc.
- DELETE | ||
Deletes the resource exposed. | ||
|
||
### On sub-path of a single resource endpoint) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
List some of the sample paths. Item might be a good sample type. Can relationships also be conveyed in a PUT/PATCH request for an owning item, or are they always managed by an individual endpoint?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that we need to postpone this discussion to the specific endpoint. Not sure if it is possible make a general statement on that. PATCH is more likely to be able to work on relations as what is not specified is assumed untouched, PUT should replace anything that is allowed to set...
- DELETE | ||
Unbinds the association. Return 405 Method Not Allowed if the association is non-optional | ||
|
||
### Error codes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
401 Unauthenticated and 403 Unauthorized could also be returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you are right, these are enough general situation to be noted in this introduction instead to be hidden in the single endpoint documentation
README.md
Outdated
- **sort**: the criteria to use. Ordering is specified appending a comma and the keyword asc or desc to the criteria name (i.e. title,asc). Unknown sort criteria and/or ordering keyword produce an error response with Http Code 400 | ||
|
||
### Response | ||
The HAL document always includes the following attributes |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HAL also returns a page object that contains the following properties: size, totalElements, totalPages, number.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oops... I miss to list the attributes here :) I'm going to update the PR
### Error Code | ||
400 Bad Request. If an unknown sort criteria is requested or a not valid ordering keyword is specified | ||
|
||
## ETags & conditional headers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What code will be responsible for generating/managing ETags? Is this provided by the framework, or will DSpace REST need to implement this? Where will an ETag be provided in a response? How will one be used in a subsequent request. An example would be useful here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to be implemented in a common layer of our DSpace 7 REST API. I have created an issue for that https://jira.duraspace.org/browse/DS-3527
## Endpoints | ||
At the root of the api a HAL document MUST list all the primary endpoints allowing full discovery of the current version of the API. | ||
|
||
[Documentation of the defined endpoints](endpoints.md) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be useful to note a recommended mechanism for creating (1)a plugin that is intended for redistribution and (2)a local customization that an instance might develop.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is outside the scope of the REST contract. We need to document that as part of the REST 7 documentation for developers
I have created an issue on behalf of you to keep track of this task
https://jira.duraspace.org/browse/DS-3528
@terrywbrady I think to have addressed the most immediate point in your comments. Feel free to press the merge button if it is ok. |
I write a first draft of the initial page of our Rest contract to move thing ahead and facilitate more discussion.
Probably it will be better to split the different sections in dedicated sub-pages. I have reserved the (not yet created) endpoints.md page for the discussion around each endpoint but I have preferred to keep all the other things in a single page at the start so that anyone can fastly check "the whole"