.../big_pipe/src/Render/BigPipeResponse.php | 3 +- ...cheableStreamedResponseEventDispatcherTrait.php | 2 +- .../page_cache/src/StackMiddleware/PageCache.php | 57 +++++++++++++--------- 3 files changed, 37 insertions(+), 25 deletions(-) diff --git a/core/modules/big_pipe/src/Render/BigPipeResponse.php b/core/modules/big_pipe/src/Render/BigPipeResponse.php index 2987499..1132fe2 100644 --- a/core/modules/big_pipe/src/Render/BigPipeResponse.php +++ b/core/modules/big_pipe/src/Render/BigPipeResponse.php @@ -2,9 +2,8 @@ namespace Drupal\big_pipe\Render; -use Drupal\Core\Render\HtmlResponse; -use Drupal\Core\Render\CacheableStreamedResponseEventDispatcherTrait; use Drupal\Core\Render\CacheableStreamedResponseInterface; +use Drupal\Core\Render\HtmlResponse; /** * A response that is sent in chunks by the BigPipe service. diff --git a/core/lib/Drupal/Core/Render/CacheableStreamedResponseEventDispatcherTrait.php b/core/modules/big_pipe/src/Render/CacheableStreamedResponseEventDispatcherTrait.php similarity index 98% rename from core/lib/Drupal/Core/Render/CacheableStreamedResponseEventDispatcherTrait.php rename to core/modules/big_pipe/src/Render/CacheableStreamedResponseEventDispatcherTrait.php index 2cd820b..9a5a073 100644 --- a/core/lib/Drupal/Core/Render/CacheableStreamedResponseEventDispatcherTrait.php +++ b/core/modules/big_pipe/src/Render/CacheableStreamedResponseEventDispatcherTrait.php @@ -1,6 +1,6 @@ storeResponse($request, $streamed_response); } } + + // Delegate, per https://github.com/stackphp/builder/issues/14. + if ($this->httpKernel instanceof TerminableInterface) { + $this->httpKernel->terminate($request, $response); + } } /** @@ -221,31 +226,25 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch /** @var \Symfony\Component\HttpFoundation\Response $response */ $response = $this->httpKernel->handle($request, $type, $catch); - // Currently it is not possible to cache binary file or streamed responses: - // https://github.com/symfony/symfony/issues/9128#issuecomment-25088678. - // Therefore exclude them, even for subclasses that implement - // CacheableResponseInterface. - if ($response instanceof BinaryFileResponse || $response instanceof StreamedResponse) { - return $response; - } - // The exception: streamed responses that implement - // StreamedResponseInterface: those can be cached by PageCache during the - // 'terminate' phase/event. Already set the 'X-Drupal-Cache' header, so that - // header is present as expected. + // The storing of cacheable streamed responses is deferred to terminate(). + // The 'X-Drupal-Cache' header must be set here however, since the response + // is already sent to the client before terminate() is called. // @see ::terminate() - elseif ($response instanceof CacheableStreamedResponseInterface) { - // Mark response as a cache miss. - $response->headers->set('X-Drupal-Cache', 'MISS'); + if ($response instanceof CacheableStreamedResponseInterface) { + // Mark streamed response as a cache miss, but only if caching is allowed + // for this streamed response. + if ($this->responsePolicy->check($response, $request) !== ResponsePolicyInterface::DENY) { + $response->headers->set('X-Drupal-Cache', 'MISS'); + } return $response; } - // Allow policy rules to further restrict which responses to cache. - if ($this->responsePolicy->check($response, $request) === ResponsePolicyInterface::DENY) { - return $response; + // Only set the 'X-Drupal-Cache' header if caching is allowed for this + // response. + if ($this->storeResponse($request, $response)) { + $response->headers->set('X-Drupal-Cache', 'MISS'); } - $this->storeResponse($request, $response); - return $response; } @@ -256,6 +255,8 @@ protected function fetch(Request $request, $type = self::MASTER_REQUEST, $catch * A request object. * @param \Symfony\Component\HttpFoundation\Response $response * A response object that should be stored in the page cache. + * + * @returns bool */ protected function storeResponse(Request $request, Response $response) { // Drupal's primary cache invalidation architecture is cache tags: any @@ -280,7 +281,20 @@ protected function storeResponse(Request $request, Response $response) { // so by replacing/extending this middleware service or adding another // one. if (!$response instanceof CacheableResponseInterface) { - return; + return FALSE; + } + + // Currently it is not possible to cache binary file or streamed responses: + // https://github.com/symfony/symfony/issues/9128#issuecomment-25088678. + // Therefore exclude them, even for subclasses that implement + // CacheableResponseInterface. + if ($response instanceof BinaryFileResponse || $response instanceof StreamedResponse) { + return FALSE; + } + + // Allow policy rules to further restrict which responses to cache. + if ($this->responsePolicy->check($response, $request) === ResponsePolicyInterface::DENY) { + return FALSE; } $request_time = $request->server->get('REQUEST_TIME'); @@ -312,8 +326,7 @@ protected function storeResponse(Request $request, Response $response) { $this->set($request, $response, $expire, $tags); } - // Mark response as a cache miss. - $response->headers->set('X-Drupal-Cache', 'MISS'); + return TRUE; } /**