-
-
Notifications
You must be signed in to change notification settings - Fork 315
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
Use page version's updated_at
timestamp as cache key
#2862
Use page version's updated_at
timestamp as cache key
#2862
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2862 +/- ##
==========================================
+ Coverage 95.86% 95.93% +0.07%
==========================================
Files 229 229
Lines 6211 6224 +13
==========================================
+ Hits 5954 5971 +17
+ Misses 257 253 -4 ☔ View full report in Codecov by Sentry. |
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.
A published version is not guaranteed
7cdcf7a
to
215ba02
Compare
c3ab762
to
de87501
Compare
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.
Code wise 👍🏻 but want to test this in some apps thoroughly before merging and releasing
Currently, it's hard for us to invalidate caches for Alchemy pages when content that's referenced through ingredients with related objects changes. For example, in a situation where a user combines Alchemy and Solidus using the `alchemy_solidus` gem, a page's cache key does not update when a product that's referenced through a SpreeProduct ingredient changes. There's a PR up on the alchemy_solidus gem that touches ingredients in these situations, and with this change, that touching can be used for breaking caches. [1] In the unlikely event the requested version is not available, we use the page's updated_at timestamp as a fallback. The core of the logic has been moved to a new `#last_modified_at` method that can be used for the last-modified header in HTTP responses. [1](AlchemyCMS/alchemy-solidus#108)
3eb93f9
to
38e4c42
Compare
Without this, the previous commit will not do what we expect. If we don´t update the page version's updated at, it will not serve as a good cache invalidator.
38e4c42
to
7383a02
Compare
We should not cache empty responses.
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.
Tested locally. Works as expected. Thanks
7597fd6
to
6787e96
Compare
What is this pull request for?
Currently, it's hard for us to invalidate caches for Alchemy pages when content that's referenced through ingredients with related objects changes.
For example, in a situation where a user combines Alchemy and Solidus using the
alchemy_solidus
gem, a page's cache key does not update when a product that's referenced through a SpreeProduct ingredient changes.There's a PR up on the alchemy_solidus gem that touches ingredients in these situations, and with this change, that touching can be used for breaking caches. [1]
1
Checklist