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

Stable image scales use weak instead of strong caching without plone.app.imaging #100

Closed
mauritsvanrees opened this issue Jul 9, 2021 · 1 comment · Fixed by #101
Closed

Comments

@mauritsvanrees
Copy link
Member

This is a stable image scale with a unique id and can be strongly cached:

  • some-image/@@images/71292397-ba62-4ca0-a6d6-37707e151e35.jpeg

These are unstable image scales which may change when the image is edited, so should be weakly cached:

  • some-image/@@images/image
  • some-image/@@images/image/preview

What happens in various Plone versions?

  • Plone 4.3: all use strong caching.
  • Plone 5.2 Python 2.7 Dexterity Image with plone.app.imaging available: stable uid scales are strongly cached, others are weakly cached. Perfect!
  • Plone 5.2 Python 2.7 Dexterity Image without plone.app.imaging available: all are weakly cached.
  • Plone 5.2 Python 3 Dexterity Image: all are weakly cached.

We are only interested in fixing 5.2. Check it like this:

  • Use the Plone coredev branch 5.2 on Python 2.7.
  • Call bin/instance-archetypes fg so plone.app.imaging is available.
  • Create a standard Plone Site. So no Archetypes content, only dexterity.
  • In the caching control panel import one of the standard caching profile, and enable caching.
  • Create an Image.
  • On the network tab of your browser developer tools check what headers are given for the various scales. For the unstable ones you will see headers X-Cache-Operation: plone.app.caching.weakCaching and X-Cache-Rule: plone.content.file. For the stable uid scales you will see X-Cache-Operation: plone.app.caching.strongCaching and X-Cache-Rule: plone.stableResource.
  • Stop the instance and start it without Archetypes code: bin/instance fg.
  • Do not change anything, simply visit the same Plone Site and the same image, and check the network tab for the headers of scales. All use weak caching.
@mauritsvanrees
Copy link
Member Author

Oops, pressed Enter too quickly... Well, the above nicely illustrates the problem. Now let's see about an explanation.

This tiny commit in plone.app.caching 1.2.0, used in Plone 5, is important, and you may want to read the comments.

Previously, plone.app.caching had this:

<cache:ruleset ruleset="plone.stableResource" for="plone.namedfile.scaling.ImageScale" />

So all scales were considered stable, and strongly cached.
With the change in 1.2.0, it became this:

<cache:ruleset ruleset="plone.content.file" for="plone.namedfile.scaling.ImageScale" />

Now all scales are considered unstable, and weakly cached.

But when your instance has plone.app.imaging, this code is active:

<cache:ruleset for=".interfaces.IStableImageScale" ruleset="plone.stableResource" />

So image scales that are marked as stable, get strong caching. The marking is done only for the uid scales, here in plone.namedfile. Where does this marker interface come from? That is in plone.namedfile.interfaces:

try:
    from plone.app.imaging.interfaces import IStableImageScale
except ImportError:
    class IStableImageScale(Interface):
        """ Marker for image scales when accessed with a UID-based URL.
        These can be cached forever using the plone.stableResource ruleset.
        """

Now you may see what is missing:

  1. When plone.app.imaging is available in the instance, the marker interface is taken from there, and the cache ruleset from plone.app.imaging zcml makes sure strong caching is enabled for this.
  2. When plone.app.imaging is not available in the instance, the cache ruleset from plone.app.imaging zcml is not loaded, so we fall back to weak caching.

Solution should be: in plone.namedfile register our own ruleset for our own marker interface. And make sure this does not conflict with the one in plone.app.imaging: register the new ruleset only when plone.app.imaging is not installed.

I will make a PR.

mauritsvanrees added a commit that referenced this issue Jul 9, 2021
When plone.app.imaging is available, this is already done.
Otherwise, we should do this ourselves.
Fixes #100.
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Jul 11, 2021
Branch: refs/heads/master
Date: 2021-07-09T16:22:29+02:00
Author: Maurits van Rees (mauritsvanrees) <[email protected]>
Commit: plone/plone.namedfile@0b0cd85

Cache stable image scales strongly.

When plone.app.imaging is available, this is already done.
Otherwise, we should do this ourselves.
Fixes plone/plone.namedfile#100.

Files changed:
A news/100.bugfix
M plone/namedfile/scaling.zcml
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2021-07-11T11:10:17+02:00
Author: Jens W. Klein (jensens) <[email protected]>
Commit: plone/plone.namedfile@8f92092

Merge pull request #101 from plone/maurits/stable-scales-stable-cache

Cache stable image scales strongly.

Files changed:
A news/100.bugfix
M plone/namedfile/scaling.zcml
mister-roboto pushed a commit to plone/buildout.coredev that referenced this issue Jul 11, 2021
Branch: refs/heads/master
Date: 2021-07-09T16:22:29+02:00
Author: Maurits van Rees (mauritsvanrees) <[email protected]>
Commit: plone/plone.namedfile@0b0cd85

Cache stable image scales strongly.

When plone.app.imaging is available, this is already done.
Otherwise, we should do this ourselves.
Fixes plone/plone.namedfile#100.

Files changed:
A news/100.bugfix
M plone/namedfile/scaling.zcml
Repository: plone.namedfile

Branch: refs/heads/master
Date: 2021-07-11T11:10:17+02:00
Author: Jens W. Klein (jensens) <[email protected]>
Commit: plone/plone.namedfile@8f92092

Merge pull request #101 from plone/maurits/stable-scales-stable-cache

Cache stable image scales strongly.

Files changed:
A news/100.bugfix
M plone/namedfile/scaling.zcml
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant