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

Todo: Invalidate cache of webresource #3505

Closed
jensens opened this issue Apr 14, 2022 · 11 comments · Fixed by #3771
Closed

Todo: Invalidate cache of webresource #3505

jensens opened this issue Apr 14, 2022 · 11 comments · Fixed by #3771

Comments

@jensens
Copy link
Sponsor Member

jensens commented Apr 14, 2022

As discussed with @pbauer and @MrTango

  • provide utility function in resources.utils.bundles_changed to write timestamp (int) at Plone root to _webresources_bundle_changed.
  • add value of _webresources_bundle_changed to cache hash of rendered resource viewlet
  • add subscriber to plone.registry record lifecycle events and call above function if a prefix plone.bundle... changes.
  • if in theme a "clear cache" is requested, call above function.
@mauritsvanrees
Copy link
Sponsor Member

I like it.
Thise timestamp could also be used as value of the resourceRegistries etag.

@jensens
Copy link
Sponsor Member Author

jensens commented Apr 14, 2022

  • if in theme a "clear cache" is requested, call above function.

Probably it is better if there (if there isn't already) is a event in plone.app.theming to subscribe to. Otherwise we may introduce circular dependencies...

@jensens
Copy link
Sponsor Member Author

jensens commented Aug 25, 2022

In Resource Registry Control Panel we have the possibility to go to debug mode and back. I think this is no longer needed. Correct me if I am wrong (or re-open)

@jensens jensens closed this as completed Aug 25, 2022
@MrTango
Copy link
Contributor

MrTango commented Sep 9, 2022

What does that back and forth do for cache invalidation?

@jensens
Copy link
Sponsor Member Author

jensens commented Sep 9, 2022

the hash is calcultated again, so a new url is used if the referenced file changed.

@fredvd
Copy link
Sponsor Member

fredvd commented Mar 13, 2023

@jensens How has the cache invalidation now been implemented? I checked Products.CMFPlone /resources but I cannot find anything that looks like "bundles_changed" or bundle_changed or how the cache works.

@jensens
Copy link
Sponsor Member Author

jensens commented Mar 14, 2023

@fredvd The UniqueResourceTraverser ++webresource++SOMETHING is registered w/o any specific functionality except it inserts the HASH to the URL. Hash calculation and URL generation is performed by webresource itself from the given file. If the file changes, the hash changes, so the generated URL changes. Invalidation happens automatically. The development settings are only controlling the caching of the whole rendered viewlet, since rendering can be expensive due to hash calculation.

@mauritsvanrees
Copy link
Sponsor Member

@jensens What I am seeing now, is that in production mode the scripts and styles viewlets are cached in volatile attributes on the site root, and are not changed until a restart, or until you enable and disable debug mode.

So what happens when you install an add-on with an extra bundle? Nothing:

  • Install plone.session.
  • No extra style link is added in the viewlet.
  • Click debug mode.
  • Bingo, the extra style is there.

I don't know if this is what you meant yourself when creating this issue. :-)

It would be nice if this worked without a Manager needing to remember to do this. In Plone 5.2 we had an import step for this, that checked if registry.xml contained changes to the resource registries, and then combined bundles, or whatever it did. Maybe something similar could be done in Plone 6?

@jensens
Copy link
Sponsor Member Author

jensens commented Mar 14, 2023

in production mode the scripts and styles viewlets are cached in volatile attributes on the site root, and are not changed until a restart, or until you enable and disable debug mode.

That is the idea of "production".

So what happens when you install an add-on with an extra bundle? Nothing.

Indeed.

I don't know if this is what you meant yourself when creating this issue. :-)

probably - and I could have forgotten about that. Not that the old resource registry was better here, but we can do better.

So I reopen and put it on the 6.1 list.

@jensens jensens reopened this Mar 14, 2023
@jensens jensens added this to the Plone 6.1 milestone Mar 14, 2023
mauritsvanrees added a commit that referenced this issue Apr 14, 2023
…or activating an add-on.

This avoids needing a restart before seeing changes when you run in production mode.
Fixes [issue 3505](#3505).
@mauritsvanrees
Copy link
Sponsor Member

I think solving this would be a bugfix and does not need to wait until 6.1. It does need careful consideration though.

I have created PR #3771.

mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Apr 18, 2023
Branch: refs/heads/master
Date: 2023-04-14T19:04:08+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/Products.CMFPlone@7a5d9ad

Clear the resource viewlet cache when changing the resource registry or activating an add-on.

This avoids needing a restart before seeing changes when you run in production mode.
Fixes [issue 3505](plone/Products.CMFPlone#3505).

Files changed:
A Products/CMFPlone/resources/eventhandlers.py
A news/3505.bugfix
M Products/CMFPlone/controlpanel/browser/resourceregistry.py
M Products/CMFPlone/resources/browser/resource.py
M Products/CMFPlone/resources/configure.zcml
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2023-04-17T23:52:45+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/Products.CMFPlone@58b1d5f

Add a last modification time of the resource registry.

We update this when changing anything related: when changing the resource registry in its control panel or activating an add-on.
This avoids needing a restart before seeing changes when you run in production mode.

Files changed:
M Products/CMFPlone/controlpanel/browser/resourceregistry.py
M Products/CMFPlone/resources/browser/resource.py
M Products/CMFPlone/resources/eventhandlers.py
M news/3505.bugfix
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2023-04-18T00:04:02+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/Products.CMFPlone@440da1c

Changes in resource settings ARE applied directly, also in production mode.

So remove this from the status message that you see on the resource registry control panel.

Files changed:
M Products/CMFPlone/controlpanel/browser/resourceregistry.pt
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2023-04-18T00:04:41+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/Products.CMFPlone@1575a6a

Merge branch 'master' into maurits-clear-resource-viewlet-caches

Files changed:
A news/3753.bugfix
M Products/CMFPlone/browser/search.py
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2023-04-18T14:43:05+02:00
Author: Maurits van Rees (mauritsvanrees) <maurits@vanrees.org>
Commit: plone/Products.CMFPlone@32db132

Registry utility may not exist, especially at site creation time.

Files changed:
M Products/CMFPlone/resources/browser/resource.py
Repository: Products.CMFPlone

Branch: refs/heads/master
Date: 2023-04-18T16:28:51+02:00
Author: Jens W. Klein (jensens) <jk@kleinundpartner.at>
Commit: plone/Products.CMFPlone@9c2e519

Merge pull request #3771 from plone/maurits-clear-resource-viewlet-caches

Clear the resource viewlet cache on relevant changes

Files changed:
A Products/CMFPlone/resources/eventhandlers.py
A news/3505.bugfix
M Products/CMFPlone/controlpanel/browser/resourceregistry.pt
M Products/CMFPlone/controlpanel/browser/resourceregistry.py
M Products/CMFPlone/resources/browser/resource.py
M Products/CMFPlone/resources/configure.zcml
@yurj
Copy link
Contributor

yurj commented Jan 11, 2024

My idea: #3771 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment