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

Remove or replace resourceRegistries etag #93

Closed
mauritsvanrees opened this issue Mar 29, 2022 · 2 comments · Fixed by #125
Closed

Remove or replace resourceRegistries etag #93

mauritsvanrees opened this issue Mar 29, 2022 · 2 comments · Fixed by #125

Comments

@mauritsvanrees
Copy link
Member

This was initially removed in Plone 5.2 because it was for the old portal_css/javascripts tools. I restored it in proper form for the "new" resource registries of 5.2 in PR #65, basically reading the timestamp.txt. But with Plone 6 the timestamp.txt in portal_resources is not there and not needed.

So what do we do with this etag? Remove it? Or can it be replaced with an etag that works on Plone 6?

The point of this etag is to prevent this scenario:

  • You have an html page in the Varnish cache, and this points to a resource with a unique hash.
  • You change the resource, probably by a change on the file system, and restart Plone.
  • A user requests the page, Varnish returns the cached version, and then the browser requests the resource with the old unique hash, so it gets the old version.

If the page is cached using etags, then the job of the resourceRegistries etag, is to return a different value, because it sees the resource has changed.

Can we come up with a sane replacement of this etag?

@mauritsvanrees
Copy link
Member Author

Starting with Plone 6.0.4, the portal_registry will have a modification time, and it is updated whenever the Resource Registries change or an add-on is installed. See plone/Products.CMFPlone#3771. We could use this time in the etag.

@jensens Does this make sense to you?

@jensens
Copy link
Member

jensens commented Apr 20, 2023

I think this is an excellent idea!

mauritsvanrees added a commit that referenced this issue Apr 25, 2023
…ation time.

This time is set since Plone 6.0.4.
Fixes #93.

This removes the previous code which read `timestamp.txt` from `/portal_resources/resource_overrides/production`.
This timestamp is no longer set in Plone 6, and does not influence anything.
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Apr 26, 2023
Branch: refs/heads/master
Date: 2023-04-25T23:19:25+02:00
Author: Maurits van Rees (mauritsvanrees) <[email protected]>
Commit: plone/plone.app.caching@57e7947

Update the resourceRegistries ETag to use the config registry modification time.

This time is set since Plone 6.0.4.
Fixes plone/plone.app.caching#93.

This removes the previous code which read `timestamp.txt` from `/portal_resources/resource_overrides/production`.
This timestamp is no longer set in Plone 6, and does not influence anything.

Files changed:
A news/93.feature
M plone/app/caching/operations/etags.py
M plone/app/caching/tests/test_profile_with_caching_proxy.py
M plone/app/caching/tests/test_profile_without_caching_proxy.py
Repository: plone.app.caching

Branch: refs/heads/master
Date: 2023-04-26T08:25:51+02:00
Author: Jens W. Klein (jensens) <[email protected]>
Commit: plone/plone.app.caching@4cdf6fb

Merge pull request #125 from plone/maurits-rr-etag

Update resourceRegistries ETag to use the config registry mtime

Files changed:
A news/93.feature
M plone/app/caching/operations/etags.py
M plone/app/caching/tests/test_profile_with_caching_proxy.py
M plone/app/caching/tests/test_profile_without_caching_proxy.py
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 a pull request may close this issue.

2 participants