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

cacheLifetime in template.xml is ignored #6301

Closed
ampaze opened this issue Oct 15, 2021 · 11 comments
Closed

cacheLifetime in template.xml is ignored #6301

ampaze opened this issue Oct 15, 2021 · 11 comments
Labels
Bug Error or unexpected behavior of already existing functionality

Comments

@ampaze
Copy link
Contributor

ampaze commented Oct 15, 2021

Q A
Sulu Version 2.2.16
PHP Version 8.0.8

Actual Behavior

<cacheLifetime>xx</cacheLifetime> is ignored

Expected Behavior

<cacheLifetime>xx</cacheLifetime> should be used

Steps to Reproduce

In your template.xml set

<cacheLifetime>20</cacheLifetime>

in sulu_http_cache.yaml set

sulu_http_cache:
    cache:
        max_age: 10
        shared_max_age: 10

The resulting response header is

Cache-Control: max-age=10, public, s-maxage=10

@ampaze ampaze added the Bug Error or unexpected behavior of already existing functionality label Oct 15, 2021
@alexander-schranz
Copy link
Member

This is the correct behaviour the <cacheLifetime>20</cacheLifetime> has no effect on the Cache-Control header because Sulu uses a custom X-Reverse-Proxy-TTL to cache the response on the server.

This is common practice in server side caching, to control how long the cache for your own cache server is and set s-maxage for third party proxy caches.

So for completeness:

x-reverse-proxy-ttl: Cachelifteime for your own cache server (symfony, varnish, ..)
s-maxage: Cachelifetime on third party proxy servers
max-age: Cachelifetime on client browser

If your cache server does not support x-reverse-proxy-ttl you need to convert the x-reverse-proxy-ttl to s-maxage by create your overwrite the SuluHttpCache and add a custom subscriber converting the ttl to s-maxage.

@alexander-schranz
Copy link
Member

The easist way would be I think add your own listener: https://github.com/sulu/skeleton/blob/dcc0a19dbb978d82729411546d9f7d4db50d5b47/src/Kernel.php#L38 here instead of overwrite the SuluHttpCache class.

@ampaze
Copy link
Contributor Author

ampaze commented Oct 15, 2021

Thanks for the detailed explanation.

In my opinion it is not clear (as there is no documentation) that cacheLifetime is for server side caching only.

So Sulu does not have any built in way to set the cache-control header on a template per template basis?

@ampaze ampaze closed this as completed Oct 15, 2021
@alexander-schranz
Copy link
Member

alexander-schranz commented Oct 15, 2021

No sulu supports out of the box symfony http cache which is already configured for using the x-reverse-proxy-ttl and varnish which is in the docs also configured using the x-reverse-proxy-ttl (Sulu Varnish Docs). There are currently no others yet official supported. Still we want in future make this kind of things easier and want to support all fos http cache clients out of the box (this includes nginx, fastly at current state).

@alexander-schranz
Copy link
Member

alexander-schranz commented Oct 15, 2021

Want to also mention currently only varnish and symfony http cache (with our the psr6store) supports tag based caching. Things like nginx or third party services cloudfront as I remember does not support tag based caching / cache invalidation.

@ampaze
Copy link
Contributor Author

ampaze commented Oct 15, 2021

Server side caching is not at the core of this issue, I just want to say to the browser you can cache pages of this template for x amount of time.

So server side caching (symfony, varnish or something else) is another topic.
Especially as you are already using a custom header to control the service side cache (x-reverse-proxy-ttl) which is by design unrelated to the max-age header.

The only way to set a max-age header seems to be with

sulu_http_cache:
    cache:
        max_age: 10
        shared_max_age: 10

@ampaze
Copy link
Contributor Author

ampaze commented Oct 15, 2021

Want to also mention currently only varnish and symfony http cache (with our the psr6store) supports tag based caching. Things like nginx or third party services cloudfront as I remember does not support tag based caching / cache invalidation.

Thanks, yes I know, tag cache invalidation is not that important at the moment.

@alexander-schranz
Copy link
Member

@ampaze The cache config is all part of the SuluHttpCache Bundle and if no adapter is configured nothing will be cache or configured as it is then like you are in dev mode where you don't want any cache headers. As a workaround currently you could go with registering the enhancer service manually in your project services.yml:

https://github.com/sulu/sulu/blob/2.x/src/Sulu/Bundle/HttpCacheBundle/Resources/config/cache-lifetime-enhancer.xml#L6-L13

@ampaze
Copy link
Contributor Author

ampaze commented Oct 15, 2021

Using my own CacheLifetimeEnhancer is a good idea!

I had to extend CacheLifetimeEnhancer instead of just implementing the interface, because of
(in WebsiteController.php)

protected function getCacheTimeLifeEnhancer(): ?CacheLifetimeEnhancer

But it works now.

@alexander-schranz
Copy link
Member

protected function getCacheTimeLifeEnhancer(): ?CacheLifetimeEnhancer

This should definitly be the CacheLifetimeEnhancerInterface do you want to create a PR for it?

@ampaze
Copy link
Contributor Author

ampaze commented Oct 15, 2021

But it works now.

I take that back, I now also need a way to have the CacheLifetimeEnhancer be used even if no proxy_client is configured ...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Error or unexpected behavior of already existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants