Skip to content

Commit

Permalink
Merge branch 'custom-ttl' into 2.x
Browse files Browse the repository at this point in the history
  • Loading branch information
dbu committed Jul 23, 2024
2 parents cc511e5 + 6957dee commit 7fad890
Show file tree
Hide file tree
Showing 6 changed files with 111 additions and 12 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,12 @@ Changelog

See also the [GitHub releases page](https://github.com/FriendsOfSymfony/FOSHttpCache/releases).

2.15.4
------

* 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

2.15.3
------

Expand Down
8 changes: 8 additions & 0 deletions doc/symfony-cache-configuration.rst
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,14 @@ but you can customize that in the listener constructor::
new CustomTtlListener('My-TTL-Header');

The custom header is removed before sending the response to the client.
You can enable keeping the custom header with the `keepTtlHeader` parameter::

new CustomTtlListener('My-TTL-Header', keepTtlHeader: true);

By default if the custom ttl header is not present, the listener falls back to the s-maxage cache-control directive.
To disable this behavior, you can set the `fallbackToSmaxage` parameter to false::

new CustomTtlListener('My-TTL-Header', fallbackToSmaxage: false);

.. _symfony-cache x-debugging:

Expand Down
33 changes: 22 additions & 11 deletions src/SymfonyCache/CustomTtlListener.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,11 @@ class CustomTtlListener implements EventSubscriberInterface
*/
private $keepTtlHeader;

/**
* @var bool
*/
private $fallbackToSmaxage;

/**
* Header used for backing up the s-maxage.
*
Expand All @@ -41,13 +46,15 @@ class CustomTtlListener implements EventSubscriberInterface
public const SMAXAGE_BACKUP = 'FOS-Smaxage-Backup';

/**
* @param string $ttlHeader The header name that is used to specify the time to live
* @param bool $keepTtlHeader Keep the custom TTL header on the response for later usage (e.g. debugging)
* @param string $ttlHeader The header name that is used to specify the time to live
* @param bool $keepTtlHeader Keep the custom TTL header on the response for later usage (e.g. debugging)
* @param bool $fallbackToSmaxage If the custom TTL header is not set, should s-maxage be used?
*/
public function __construct($ttlHeader = 'X-Reverse-Proxy-TTL', $keepTtlHeader = false)
public function __construct($ttlHeader = 'X-Reverse-Proxy-TTL', $keepTtlHeader = false, $fallbackToSmaxage = true)
{
$this->ttlHeader = $ttlHeader;
$this->keepTtlHeader = $keepTtlHeader;
$this->fallbackToSmaxage = $fallbackToSmaxage;
}

/**
Expand All @@ -59,15 +66,23 @@ public function __construct($ttlHeader = 'X-Reverse-Proxy-TTL', $keepTtlHeader =
public function useCustomTtl(CacheEvent $e)
{
$response = $e->getResponse();
if (!$response->headers->has($this->ttlHeader)) {

if (!$response->headers->has($this->ttlHeader)
&& true === $this->fallbackToSmaxage
) {
return;
}

$backup = $response->headers->hasCacheControlDirective('s-maxage')
? $response->headers->getCacheControlDirective('s-maxage')
: 'false'
;
$response->headers->set(static::SMAXAGE_BACKUP, $backup);
$response->setTtl($response->headers->get($this->ttlHeader));
$response->setTtl(
$response->headers->has($this->ttlHeader)
? $response->headers->get($this->ttlHeader)
: 0
);
}

/**
Expand All @@ -76,11 +91,6 @@ public function useCustomTtl(CacheEvent $e)
public function cleanResponse(CacheEvent $e)
{
$response = $e->getResponse();
if (!$response->headers->has($this->ttlHeader)
&& !$response->headers->has(static::SMAXAGE_BACKUP)
) {
return;
}

if ($response->headers->has(static::SMAXAGE_BACKUP)) {
$smaxage = $response->headers->get(static::SMAXAGE_BACKUP);
Expand All @@ -89,12 +99,13 @@ public function cleanResponse(CacheEvent $e)
} else {
$response->headers->addCacheControlDirective('s-maxage', $smaxage);
}

$response->headers->remove(static::SMAXAGE_BACKUP);
}

if (!$this->keepTtlHeader) {
$response->headers->remove($this->ttlHeader);
}
$response->headers->remove(static::SMAXAGE_BACKUP);
}

public static function getSubscribedEvents(): array
Expand Down
6 changes: 5 additions & 1 deletion src/SymfonyCache/EventDispatchingHttpCache.php
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,11 @@ protected function store(Request $request, Response $response)
{
$response = $this->dispatch(Events::PRE_STORE, $request, $response);

parent::store($request, $response);
// CustomTtlListener or other Listener or Subscribers might have changed the response to become non-cacheable, revalidate.
// Only call store for a cacheable response like Symfony core does: https://github.com/symfony/symfony/blob/v7.1.2/src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php#L409
if ($response->isCacheable()) {
parent::store($request, $response);
}
}

/**
Expand Down
33 changes: 33 additions & 0 deletions src/Test/EventDispatchingHttpCacheTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,7 @@ public function testPreStoreCalled()
{
$request = Request::create('/foo', 'GET');
$response = new Response();
$response->setTtl(42);

$httpCache = $this->getHttpCachePartialMock();
$testListener = new TestListener($this, $httpCache, $request);
Expand All @@ -230,6 +231,7 @@ public function testPreStoreResponse()
$request = Request::create('/foo', 'GET');
$regularResponse = new Response();
$preStoreResponse = new Response();
$preStoreResponse->setTtl(42);

$httpCache = $this->getHttpCachePartialMock();
$testListener = new TestListener($this, $httpCache, $request);
Expand All @@ -245,6 +247,37 @@ public function testPreStoreResponse()
$this->assertEquals(1, $testListener->preStoreCalls);
}

/**
* Assert that preStore response is used when provided.
*/
public function testPreStoreResponseNoStore()
{
$request = Request::create('/foo', 'GET');
$regularResponse = new Response();
$regularResponse->setTtl(42);
$preStoreResponse = new Response();
$preStoreResponse->setTtl(0);

$httpCache = $this->getHttpCachePartialMock();
$testListener = new TestListener($this, $httpCache, $request);
$testListener->preStoreResponse = $preStoreResponse;
$httpCache->addSubscriber($testListener);

$store = $this->createMock(StoreInterface::class);
$store->expects($this->never())->method('write');

$refHttpCache = new \ReflectionClass(HttpCache::class);
$refStore = $refHttpCache->getProperty('store');
$refStore->setAccessible(true);
$refStore->setValue($httpCache, $store);

$refHttpCache = new \ReflectionObject($httpCache);
$method = $refHttpCache->getMethod('store');
$method->setAccessible(true);
$method->invokeArgs($httpCache, [$request, $regularResponse]);
$this->assertEquals(1, $testListener->preStoreCalls);
}

/**
* Assert that preInvalidate is called.
*/
Expand Down
37 changes: 37 additions & 0 deletions tests/Unit/SymfonyCache/CustomTtlListenerTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,23 @@ public function testNoCustomTtl()
$this->assertFalse($response->headers->has(CustomTtlListener::SMAXAGE_BACKUP));
}

public function testNoCustomTtlNoFallback()
{
$ttlListener = new CustomTtlListener('X-Reverse-Proxy-TTL', false, false);
$request = Request::create('http://example.com/foo', 'GET');
$response = new Response('', 200, [
'Cache-Control' => 'max-age=30, s-maxage=33',
]);
$event = new CacheEvent($this->kernel, $request, $response);

$ttlListener->useCustomTtl($event);
$response = $event->getResponse();

$this->assertInstanceOf(Response::class, $response);
$this->assertSame('0', $response->headers->getCacheControlDirective('s-maxage'));
$this->assertTrue($response->headers->has(CustomTtlListener::SMAXAGE_BACKUP));
}

public function testCleanup()
{
$ttlListener = new CustomTtlListener();
Expand All @@ -108,6 +125,26 @@ public function testCleanup()
$this->assertFalse($response->headers->has(CustomTtlListener::SMAXAGE_BACKUP));
}

public function testCleanupNoReverseProxyTtl()
{
$ttlListener = new CustomTtlListener();
$request = Request::create('http://example.com/foo', 'GET');
$response = new Response('', 200, [
'Cache-Control' => 's-maxage=0, max-age=30',
CustomTtlListener::SMAXAGE_BACKUP => '60',
]);
$event = new CacheEvent($this->kernel, $request, $response);

$ttlListener->cleanResponse($event);
$response = $event->getResponse();

$this->assertInstanceOf(Response::class, $response);
$this->assertTrue($response->headers->hasCacheControlDirective('s-maxage'));
$this->assertSame('60', $response->headers->getCacheControlDirective('s-maxage'));
$this->assertFalse($response->headers->has('X-Reverse-Proxy-TTL'));
$this->assertFalse($response->headers->has(CustomTtlListener::SMAXAGE_BACKUP));
}

public function testCleanupNoSmaxage()
{
$ttlListener = new CustomTtlListener();
Expand Down

0 comments on commit 7fad890

Please sign in to comment.