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

Improve etag generation #80

Merged
merged 1 commit into from
May 15, 2024
Merged

Improve etag generation #80

merged 1 commit into from
May 15, 2024

Conversation

mamhoff
Copy link
Contributor

@mamhoff mamhoff commented May 13, 2024

The Last-Modified header is a weaker cache key than the etag, and thus
we rely solely on the etag from now on. Browsers and Rails will ignore
it if it's not set while validating if a request is fresh.

@mamhoff mamhoff changed the title Use updated at for etag Improve etag generation May 13, 2024
@mamhoff mamhoff force-pushed the use-updated-at-for-etag branch 3 times, most recently from 80b2f07 to 9343574 Compare May 13, 2024 15:21
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Nice simplification! TIL

app/controllers/alchemy/json_api/pages_controller.rb Outdated Show resolved Hide resolved
app/controllers/alchemy/json_api/pages_controller.rb Outdated Show resolved Hide resolved
app/controllers/alchemy/json_api/pages_controller.rb Outdated Show resolved Hide resolved
app/controllers/alchemy/json_api/pages_controller.rb Outdated Show resolved Hide resolved
spec/requests/alchemy/json_api/pages_spec.rb Show resolved Hide resolved
spec/requests/alchemy/json_api/pages_spec.rb Outdated Show resolved Hide resolved
@mamhoff mamhoff force-pushed the use-updated-at-for-etag branch 3 times, most recently from 7d79b01 to 4adc593 Compare May 15, 2024 11:10
The Last-Modified header is a weaker cache key than the etag, and thus
we rely solely on the etag from now on. Browsers and Rails will ignore
it if it's not set while validating if a request is fresh.

Also sets the etag based on JSONAPI's typical set of params, so that we
get a different etag for different `include`, `sort`, `page`, `fields`,
or `filter` params.
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

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

Nice work 👏🏻

@tvdeyen tvdeyen enabled auto-merge May 15, 2024 11:13
@tvdeyen tvdeyen merged commit 1bd7cc0 into main May 15, 2024
16 checks passed
@tvdeyen tvdeyen deleted the use-updated-at-for-etag branch May 15, 2024 11:14
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.

2 participants