Skip to content

Commit

Permalink
fix custom ttl with symfony HttpCache to work regardless of s-maxage
Browse files Browse the repository at this point in the history
  • Loading branch information
dbu committed Jul 23, 2024
1 parent 1f95522 commit 330179b
Show file tree
Hide file tree
Showing 5 changed files with 35 additions and 7 deletions.
19 changes: 16 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
------
Expand Down
2 changes: 1 addition & 1 deletion src/SymfonyCache/CustomTtlListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ public function cleanResponse(CacheEvent $e)
public static function getSubscribedEvents(): array
{
return [
Events::PRE_STORE => 'useCustomTtl',
Events::POST_FORWARD => 'useCustomTtl',
Events::POST_HANDLE => 'cleanResponse',
];
}
Expand Down
10 changes: 10 additions & 0 deletions src/SymfonyCache/EventDispatchingHttpCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down
4 changes: 4 additions & 0 deletions src/SymfonyCache/Events.php
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down
7 changes: 4 additions & 3 deletions tests/Functional/Symfony/EventDispatchingHttpCacheTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down

0 comments on commit 330179b

Please sign in to comment.