From c19bdcff595675326bedd5d488f2f3f296194f88 Mon Sep 17 00:00:00 2001 From: David Buchmann Date: Tue, 23 Jul 2024 17:24:12 +0200 Subject: [PATCH] fix custom ttl with symfony HttpCache to work regardless of s-maxage --- CHANGELOG.md | 19 ++++++++++++++++--- src/SymfonyCache/CustomTtlListener.php | 9 ++++----- .../EventDispatchingHttpCache.php | 10 ++++++++++ src/SymfonyCache/Events.php | 4 ++++ .../Symfony/EventDispatchingHttpCacheTest.php | 7 ++++--- 5 files changed, 38 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cfb2f397..431ea047 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,11 +3,24 @@ Changelog See also the [GitHub releases page](https://github.com/FriendsOfSymfony/FOSHttpCache/releases). -2.16 (unreleased) +2.16 ---- -* Add flag `fallbackToSmaxage` to `CustomTtlListener` to allow controlling fallback to s-maxage if custom TTL header is not defined on the response. -* Fix for Symfony HttpCache: Do not call store if Response object is not longer cacheable after event listeners. If you use the custom TTL system, this is only a performance improvement, because the TTL of the response would still be 0. With a custom listener that changes the response explicitly to not be cached but does not change s-maxage, this bug might have led to caching responses that should not have been cached +### Symfony HttpCache + +* Add events `PRE_FORWARD` and `POST_FORWARD` to allow event listeners to alter + the request before and after it is sent to the backend. +* Changed CustomTtlListener to use the `POST_FORWARD` event instead of + `PRE_STORE`. Using PRE_STORE, requests that are not considered cacheable by + Symfony were never cached, even when they had a custom TTL header. +* Add flag `fallbackToSmaxage` to `CustomTtlListener` to allow controlling + fallback to s-maxage if custom TTL header is not defined on the response. +* Fix: Do not call store if Response object is not longer cacheable after event + listeners. If you use the custom TTL system, this is only a performance + improvement, because the TTL of the response would still be 0. With a custom + listener that changes the response explicitly to not be cached but does not + change s-maxage, this bug might have led to caching responses that should not + have been cached. 2.15.3 ------ diff --git a/src/SymfonyCache/CustomTtlListener.php b/src/SymfonyCache/CustomTtlListener.php index 613311f7..a0e80aba 100644 --- a/src/SymfonyCache/CustomTtlListener.php +++ b/src/SymfonyCache/CustomTtlListener.php @@ -78,10 +78,9 @@ public function useCustomTtl(CacheEvent $e) : 'false' ; $response->headers->set(static::SMAXAGE_BACKUP, $backup); - $response->setTtl( - $response->headers->has($this->ttlHeader) - ? $response->headers->get($this->ttlHeader) - : 0 + $response->headers->addCacheControlDirective( + 's-maxage', + $response->headers->get($this->ttlHeader, 0) ); } @@ -111,7 +110,7 @@ public function cleanResponse(CacheEvent $e) public static function getSubscribedEvents(): array { return [ - Events::PRE_STORE => 'useCustomTtl', + Events::POST_FORWARD => 'useCustomTtl', Events::POST_HANDLE => 'cleanResponse', ]; } diff --git a/src/SymfonyCache/EventDispatchingHttpCache.php b/src/SymfonyCache/EventDispatchingHttpCache.php index a67e1acc..9376e7e8 100644 --- a/src/SymfonyCache/EventDispatchingHttpCache.php +++ b/src/SymfonyCache/EventDispatchingHttpCache.php @@ -130,6 +130,16 @@ protected function invalidate(Request $request, $catch = false): Response return parent::invalidate($request, $catch); } + protected function forward(Request $request, bool $catch = false, ?Response $entry = null): Response + { + // do not abort early, if $entry is set this is a validation request + $this->dispatch(Events::PRE_FORWARD, $request, $entry); + + $response = parent::forward($request, $catch, $entry); + + return $this->dispatch(Events::POST_FORWARD, $request, $response); + } + /** * Dispatch an event if there are any listeners. * diff --git a/src/SymfonyCache/Events.php b/src/SymfonyCache/Events.php index cb81d979..a911b95c 100644 --- a/src/SymfonyCache/Events.php +++ b/src/SymfonyCache/Events.php @@ -20,6 +20,10 @@ final class Events public const POST_HANDLE = 'fos_http_cache.post_handle'; + public const PRE_FORWARD = 'fos_http_cache.pre_forward'; + + public const POST_FORWARD = 'fos_http_cache.post_forward'; + public const PRE_INVALIDATE = 'fos_http_cache.pre_invalidate'; public const PRE_STORE = 'fos_http_cache.pre_store'; diff --git a/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php b/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php index f4a0e703..f6a46ae4 100644 --- a/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php +++ b/tests/Functional/Symfony/EventDispatchingHttpCacheTest.php @@ -43,13 +43,14 @@ public function testEventListeners() $httpKernel = \Mockery::mock(HttpKernelInterface::class) ->shouldReceive('handle') - ->withArgs([$request, HttpKernelInterface::MASTER_REQUEST, true]) ->andReturn($expectedResponse) ->getMock(); $store = \Mockery::mock(StoreInterface::class) + ->shouldReceive('lookup')->andReturn(null)->times(1) + ->shouldReceive('write')->times(1) + ->shouldReceive('unlock')->times(1) // need to declare the cleanup function explicitly to avoid issue between register_shutdown_function and mockery - ->shouldReceive('cleanup') - ->atMost(1) + ->shouldReceive('cleanup')->atMost(1) ->getMock(); $kernel = new AppCache($httpKernel, $store); $kernel->addSubscriber(new CustomTtlListener());