Skip to content

Commit 662eb17

Browse files
ToflarToflar
committed
[HttpCache] Do not call terminate() on cache hit
1 parent 6597825 commit 662eb17

File tree

4 files changed

+129
-7
lines changed

4 files changed

+129
-7
lines changed

‎src/Symfony/Component/HttpKernel/HttpCache/HttpCache.php‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,11 @@ class HttpCache implements HttpKernelInterface, TerminableInterface
7878
* the cache can serve a stale response when an error is encountered (default: 60).
7979
* This setting is overridden by the stale-if-error HTTP Cache-Control extension
8080
* (see RFC 5861).
81+
*
82+
* * terminate_on_cache_hit Specifies if the kernel.terminate event should be dispatched even when the cache
83+
* was hit (default: true).
84+
* Unless your application needs to process events on cache hits, it is recommended
85+
* to set this to false to avoid having to bootstrap the Symfony framework on a cache hit.
8186
*/
8287
public function __construct(HttpKernelInterface $kernel, StoreInterface $store, SurrogateInterface $surrogate = null, array $options = [])
8388
{
@@ -98,6 +103,7 @@ public function __construct(HttpKernelInterface $kernel, StoreInterface $store,
98103
'stale_if_error' => 60,
99104
'trace_level' => 'none',
100105
'trace_header' => 'X-Symfony-Cache',
106+
'terminate_on_cache_hit' => true,
101107
], $options);
102108

103109
if (!isset($options['trace_level'])) {
@@ -238,6 +244,15 @@ public function handle(Request $request, int $type = HttpKernelInterface::MAIN_R
238244
*/
239245
public function terminate(Request $request, Response $response)
240246
{
247+
// Do not call any listeners in case of a cache hit.
248+
// This ensures identical behavior as if you had a separate
249+
// reverse caching proxy such as Varnish and the like.
250+
if ($this->options['terminate_on_cache_hit']) {
251+
trigger_deprecation('symfony/http-kernel', '6.2', 'Setting "terminate_on_cache_hit" to "true" is deprecated and will be changed to "false" in Symfony 7.0.');
252+
} elseif (\in_array('fresh', $this->traces[$this->getTraceKey($request)] ?? [], true)) {
253+
return;
254+
}
255+
241256
if ($this->getKernel() instanceof TerminableInterface) {
242257
$this->getKernel()->terminate($request, $response);
243258
}

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTest.php‎

Lines changed: 105 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,11 +11,13 @@
1111

1212
namespace Symfony\Component\HttpKernel\Tests\HttpCache;
1313

14+
use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait;
15+
use Symfony\Component\EventDispatcher\EventDispatcher;
1416
use Symfony\Component\HttpFoundation\Request;
1517
use Symfony\Component\HttpFoundation\Response;
18+
use Symfony\Component\HttpKernel\Event\TerminateEvent;
1619
use Symfony\Component\HttpKernel\HttpCache\Esi;
1720
use Symfony\Component\HttpKernel\HttpCache\HttpCache;
18-
use Symfony\Component\HttpKernel\HttpCache\Store;
1921
use Symfony\Component\HttpKernel\HttpCache\StoreInterface;
2022
use Symfony\Component\HttpKernel\HttpKernelInterface;
2123
use Symfony\Component\HttpKernel\Kernel;
@@ -25,6 +27,8 @@
2527
*/
2628
class HttpCacheTest extends HttpCacheTestCase
2729
{
30+
use ExpectDeprecationTrait;
31+
2832
public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
2933
{
3034
$storeMock = $this->getMockBuilder(StoreInterface::class)
@@ -33,7 +37,7 @@ public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
3337

3438
// does not implement TerminableInterface
3539
$kernel = new TestKernel();
36-
$httpCache = new HttpCache($kernel, $storeMock);
40+
$httpCache = new HttpCache($kernel, $storeMock, null, ['terminate_on_cache_hit' => false]);
3741
$httpCache->terminate(Request::create('/'), new Response());
3842

3943
$this->assertFalse($kernel->terminateCalled, 'terminate() is never called if the kernel class does not implement TerminableInterface');
@@ -47,10 +51,108 @@ public function testTerminateDelegatesTerminationOnlyForTerminableInterface()
4751
$kernelMock->expects($this->once())
4852
->method('terminate');
4953

50-
$kernel = new HttpCache($kernelMock, $storeMock);
54+
$kernel = new HttpCache($kernelMock, $storeMock, null, ['terminate_on_cache_hit' => false]);
5155
$kernel->terminate(Request::create('/'), new Response());
5256
}
5357

58+
public function testDoesNotCallTerminateOnFreshResponse()
59+
{
60+
$terminateEvents = [];
61+
62+
$eventDispatcher = $this->createMock(EventDispatcher::class);
63+
$eventDispatcher
64+
->expects($this->any())
65+
->method('dispatch')
66+
->with($this->callback(function ($event) use (&$terminateEvents) {
67+
if ($event instanceof TerminateEvent) {
68+
$terminateEvents[] = $event;
69+
}
70+
71+
return true;
72+
}));
73+
74+
$this->setNextResponse(
75+
200,
76+
[
77+
'ETag' => '1234',
78+
'Cache-Control' => 'public, s-maxage=60',
79+
],
80+
'Hello World',
81+
null,
82+
$eventDispatcher
83+
);
84+
85+
$this->request('GET', '/');
86+
$this->assertHttpKernelIsCalled();
87+
$this->assertEquals(200, $this->response->getStatusCode());
88+
$this->assertTraceContains('miss');
89+
$this->assertTraceContains('store');
90+
$this->cache->terminate($this->request, $this->response);
91+
92+
sleep(2);
93+
94+
$this->request('GET', '/');
95+
$this->assertHttpKernelIsNotCalled();
96+
$this->assertEquals(200, $this->response->getStatusCode());
97+
$this->assertTraceContains('fresh');
98+
$this->assertEquals(2, $this->response->headers->get('Age'));
99+
$this->cache->terminate($this->request, $this->response);
100+
101+
$this->assertCount(1, $terminateEvents);
102+
}
103+
104+
/**
105+
* @group legacy
106+
*/
107+
public function testDoesCallTerminateOnFreshResponseIfConfigured()
108+
{
109+
$this->expectDeprecation('Since symfony/http-kernel 6.2: Setting "terminate_on_cache_hit" to "true" is deprecated and will be changed to "false" in Symfony 7.0.');
110+
111+
$terminateEvents = [];
112+
113+
$eventDispatcher = $this->createMock(EventDispatcher::class);
114+
$eventDispatcher
115+
->expects($this->any())
116+
->method('dispatch')
117+
->with($this->callback(function ($event) use (&$terminateEvents) {
118+
if ($event instanceof TerminateEvent) {
119+
$terminateEvents[] = $event;
120+
}
121+
122+
return true;
123+
}));
124+
125+
$this->setNextResponse(
126+
200,
127+
[
128+
'ETag' => '1234',
129+
'Cache-Control' => 'public, s-maxage=60',
130+
],
131+
'Hello World',
132+
null,
133+
$eventDispatcher
134+
);
135+
$this->cacheConfig['terminate_on_cache_hit'] = true;
136+
137+
$this->request('GET', '/');
138+
$this->assertHttpKernelIsCalled();
139+
$this->assertEquals(200, $this->response->getStatusCode());
140+
$this->assertTraceContains('miss');
141+
$this->assertTraceContains('store');
142+
$this->cache->terminate($this->request, $this->response);
143+
144+
sleep(2);
145+
146+
$this->request('GET', '/');
147+
$this->assertHttpKernelIsNotCalled();
148+
$this->assertEquals(200, $this->response->getStatusCode());
149+
$this->assertTraceContains('fresh');
150+
$this->assertEquals(2, $this->response->headers->get('Age'));
151+
$this->cache->terminate($this->request, $this->response);
152+
153+
$this->assertCount(2, $terminateEvents);
154+
}
155+
54156
public function testPassesOnNonGetHeadRequests()
55157
{
56158
$this->setNextResponse(200);

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/HttpCacheTestCase.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
namespace Symfony\Component\HttpKernel\Tests\HttpCache;
1313

1414
use PHPUnit\Framework\TestCase;
15+
use Symfony\Component\EventDispatcher\EventDispatcher;
1516
use Symfony\Component\HttpFoundation\Request;
1617
use Symfony\Component\HttpKernel\HttpCache\Esi;
1718
use Symfony\Component\HttpKernel\HttpCache\HttpCache;
@@ -124,6 +125,10 @@ public function request($method, $uri = '/', $server = [], $cookies = [], $esi =
124125
$this->cacheConfig['debug'] = true;
125126
}
126127

128+
if (!isset($this->cacheConfig['terminate_on_cache_hit'])) {
129+
$this->cacheConfig['terminate_on_cache_hit'] = false;
130+
}
131+
127132
$this->esi = $esi ? new Esi() : null;
128133
$this->cache = new HttpCache($this->kernel, $this->store, $this->esi, $this->cacheConfig);
129134
$this->request = Request::create($uri, $method, [], $cookies, [], $server);
@@ -145,9 +150,9 @@ public function getMetaStorageValues()
145150
}
146151

147152
// A basic response with 200 status code and a tiny body.
148-
public function setNextResponse($statusCode = 200, array $headers = [], $body = 'Hello World', \Closure $customizer = null)
153+
public function setNextResponse($statusCode = 200, array $headers = [], $body = 'Hello World', \Closure $customizer = null, EventDispatcher $eventDispatcher = null)
149154
{
150-
$this->kernel = new TestHttpKernel($body, $statusCode, $headers, $customizer);
155+
$this->kernel = new TestHttpKernel($body, $statusCode, $headers, $customizer, $eventDispatcher);
151156
}
152157

153158
public function setNextResponses($responses)

‎src/Symfony/Component/HttpKernel/Tests/HttpCache/TestHttpKernel.php‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ class TestHttpKernel extends HttpKernel implements ControllerResolverInterface,
2929
protected $catch = false;
3030
protected $backendRequest;
3131

32-
public function __construct($body, $status, $headers, \Closure $customizer = null)
32+
public function __construct($body, $status, $headers, \Closure $customizer = null, EventDispatcher $eventDispatcher = null)
3333
{
3434
$this->body = $body;
3535
$this->status = $status;
3636
$this->headers = $headers;
3737
$this->customizer = $customizer;
3838

39-
parent::__construct(new EventDispatcher(), $this, null, $this, true);
39+
parent::__construct($eventDispatcher ?? new EventDispatcher(), $this, null, $this, true);
4040
}
4141

4242
public function assert(\Closure $callback)

0 commit comments

Comments
 (0)