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

feat: add etag and collections #91

Merged
merged 5 commits into from
Sep 9, 2024
Merged

Conversation

PavlenkoV
Copy link
Contributor

This update introduces the addition of ETag to collection responses, allowing clients to track versioning for individual resources within a collection. By including an ETag for each item, we enable efficient caching and conditional operations, reducing unnecessary API calls and improving performance. The ETag is added as a read-only field in the response body to ensure version consistency without altering the API's general object model.

This solution can serve as an interim approach until a standardized HATEOAS implementation is adopted.

@PavlenkoV PavlenkoV requested a review from a team as a code owner September 6, 2024 14:37
@PavlenkoV PavlenkoV force-pushed the add-etag-collections branch from fa49ad0 to 8af43d1 Compare September 6, 2024 14:47
// RESPONSE
200 OK
Content-Type: application/json
ETag: "33a64df551425fcc55e4d42a148795d9f25f89d4"

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From our convo this morning the above seems great. This line seems redundant.
Search returns a list so Header is invalid and inclusion of ETag in Body makes sense (Response Body *not Request body)
Get By ID: There is only 1 so Header works but having the Etag in both the Header and Body feels redundant.. not the worst thing but can we standardize on 1 instead of both?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. But it also feels inconsistent with an ETag being optional in a resource object as part of a collection and required when returned individually.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me its similar reasoning to including the status code in the body of error message (impacts logging, convenience, etc). In this case specifically, its the intersection of two patterns, where we want data model consistency between your resources returned in collection or singular, but also consistency to other resources by id that use ETag as the more industry norm pattern for individual results.

I think for consistency, both are necessary depending on how the consumer will make use of it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense. I can see why we wouldn't want multiple schema's to represent the same thing and/or context specific schemas.

travisgosselin and others added 2 commits September 9, 2024 13:28
Co-authored-by: Nick Clarity <[email protected]>
Signed-off-by: Travis Gosselin <[email protected]>
@travisgosselin
Copy link
Member

@omnipitous / @nickclarity / @PavlenkoV - I appreciate your expertise, effort, and review on this one.

@travisgosselin travisgosselin merged commit a6e2f45 into main Sep 9, 2024
4 checks passed
@travisgosselin travisgosselin deleted the add-etag-collections branch September 9, 2024 18:07
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

Successfully merging this pull request may close these issues.

4 participants