core/modules/rest/rest.services.yml | 2 +- .../EventSubscriber/ResourceResponseSubscriber.php | 33 ++++++++++----------- .../rest/tests/src/Kernel/RequestHandlerTest.php | 3 -- .../ResourceResponseSubscriberTest.php | 34 +++++++++++----------- 4 files changed, 33 insertions(+), 39 deletions(-) diff --git a/core/modules/rest/rest.services.yml b/core/modules/rest/rest.services.yml index 385c4bd..81eb451 100644 --- a/core/modules/rest/rest.services.yml +++ b/core/modules/rest/rest.services.yml @@ -28,7 +28,7 @@ services: logger.channel.rest: parent: logger.channel_base arguments: ['rest'] - resource_response.subscriber: + rest.resource_response.subscriber: class: Drupal\rest\EventSubscriber\ResourceResponseSubscriber tags: - { name: event_subscriber } diff --git a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php index 514f4ee..34dff52 100644 --- a/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php +++ b/core/modules/rest/src/EventSubscriber/ResourceResponseSubscriber.php @@ -16,7 +16,7 @@ use Symfony\Component\Serializer\SerializerInterface; /** - * Response subscriber to handle resource responses. + * Response subscriber that serializes and removes ResourceResponses' data. */ class ResourceResponseSubscriber implements EventSubscriberInterface { @@ -58,12 +58,12 @@ public function __construct(SerializerInterface $serializer, RendererInterface $ } /** - * Processes attachments for ResourceResponse responses. + * Serializes ResourceResponse responses' data, and removes that data. * * @param \Symfony\Component\HttpKernel\Event\FilterResponseEvent $event * The event to process. */ - public function onRespond(FilterResponseEvent $event) { + public function onResponse(FilterResponseEvent $event) { $response = $event->getResponse(); if (!$response instanceof ResourceResponseInterface) { return; @@ -150,19 +150,14 @@ protected function renderResponseBody(Request $request, ResourceResponseInterfac // If there is data to send, serialize and set it as the response body. if ($data !== NULL) { - if ($response instanceof CacheableResponseInterface) { - $context = new RenderContext(); - $output = $this->renderer - ->executeInRenderContext($context, function () use ($serializer, $data, $format) { - return $serializer->serialize($data, $format); - }); - - if (!$context->isEmpty()) { - $response->addCacheableDependency($context->pop()); - } - } - else { - $output = $serializer->serialize($data, $format); + $context = new RenderContext(); + $output = $this->renderer + ->executeInRenderContext($context, function () use ($serializer, $data, $format) { + return $serializer->serialize($data, $format); + }); + + if ($response instanceof CacheableResponseInterface && !$context->isEmpty()) { + $response->addCacheableDependency($context->pop()); } $response->setContent($output); @@ -174,7 +169,9 @@ protected function renderResponseBody(Request $request, ResourceResponseInterfac * Flattens a fully rendered resource response. * * Ensures that complex data structures in ResourceResponse::getResponseData() - * are not serialized. + * are not serialized. Not doing this means that caching this response object + * requires unserializing the PHP data when reading this response object from + * cache, which can be very costly, and is unnecessary. * * @param \Drupal\rest\ResourceResponseInterface $response * A fully rendered resource response. @@ -199,7 +196,7 @@ protected function flattenResponse(ResourceResponseInterface $response) { * {@inheritdoc} */ public static function getSubscribedEvents() { - $events[KernelEvents::RESPONSE][] = ['onRespond']; + $events[KernelEvents::RESPONSE][] = ['onResponse']; return $events; } diff --git a/core/modules/rest/tests/src/Kernel/RequestHandlerTest.php b/core/modules/rest/tests/src/Kernel/RequestHandlerTest.php index 336e483..6d13b2b 100644 --- a/core/modules/rest/tests/src/Kernel/RequestHandlerTest.php +++ b/core/modules/rest/tests/src/Kernel/RequestHandlerTest.php @@ -2,7 +2,6 @@ namespace Drupal\Tests\rest\Kernel; -use Drupal\Component\Serialization\Json; use Drupal\Core\Entity\EntityStorageInterface; use Drupal\Core\Routing\RouteMatch; use Drupal\KernelTests\KernelTestBase; @@ -10,8 +9,6 @@ use Drupal\rest\RequestHandler; use Drupal\rest\ResourceResponse; use Drupal\rest\RestResourceConfigInterface; -use Prophecy\Argument; -use Prophecy\Prophecy\MethodProphecy; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\Routing\Route; diff --git a/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php b/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php index 9f0e775..87c9059 100644 --- a/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php +++ b/core/modules/rest/tests/src/Unit/EventSubscriber/ResourceResponseSubscriberTest.php @@ -30,7 +30,7 @@ class ResourceResponseSubscriberTest extends UnitTestCase { /** - * @covers ::onRespond + * @covers ::onResponse * @dataProvider providerTestSerialization */ public function testSerialization($data, $expected_response = FALSE) { @@ -45,7 +45,7 @@ public function testSerialization($data, $expected_response = FALSE) { HttpKernelInterface::MASTER_REQUEST, $handler_response ); - $resource_response_subscriber->onRespond($event); + $resource_response_subscriber->onResponse($event); // Content is a serialized version of the data we provided. $this->assertEquals($expected_response !== FALSE ? $expected_response : json_encode($data), $event->getResponse()->getContent()); @@ -54,15 +54,15 @@ public function testSerialization($data, $expected_response = FALSE) { public function providerTestSerialization() { return [ // The default data for \Drupal\rest\ResourceResponse. - [NULL, ''], - [''], - ['string'], - ['Complex \ string $%^&@ with unicode ΑΒΓΔΕΖΗΘΙΚΛΜΝΞΟΣὨ'], - [[]], - [['test']], - [['test' => 'foobar']], - [TRUE], - [FALSE], + 'default' => [NULL, ''], + 'empty string' => [''], + 'simple string' => ['string'], + 'complex string' => ['Complex \ string $%^&@ with unicode ΑΒΓΔΕΖΗΘΙΚΛΜΝΞΟΣὨ'], + 'empty array' => [[]], + 'numeric array' => [['test']], + 'associative array' => [['test' => 'foobar']], + 'boolean true' => [TRUE], + 'boolean false' => [FALSE], // @todo Not supported. https://www.drupal.org/node/2427811 // [new \stdClass()], // [(object) ['test' => 'foobar']], @@ -106,14 +106,14 @@ public function testResponseFormat($methods, array $supported_formats, $request_ } /** - * @covers ::onRespond + * @covers ::onResponse * @covers ::getResponseFormat * @covers ::renderResponseBody * @covers ::flattenResponse * * @dataProvider providerTestResponseFormat */ - public function testOnRespondWithCacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { + public function testOnResponseWithCacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { $rest_config_name = $this->randomMachineName(); $parameters = []; @@ -146,7 +146,7 @@ public function testOnRespondWithCacheableResponse($methods, array $supported_fo HttpKernelInterface::MASTER_REQUEST, $handler_response ); - $resource_response_subscriber->onRespond($event); + $resource_response_subscriber->onResponse($event); $final_response = $event->getResponse(); $this->assertNotInstanceOf(ResourceResponseInterface::class, $final_response); $this->assertInstanceOf(CacheableResponseInterface::class, $final_response); @@ -156,14 +156,14 @@ public function testOnRespondWithCacheableResponse($methods, array $supported_fo } /** - * @covers ::onRespond + * @covers ::onResponse * @covers ::getResponseFormat * @covers ::renderResponseBody * @covers ::flattenResponse * * @dataProvider providerTestResponseFormat */ - public function testOnRespondWithUncacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { + public function testOnResponseWithUncacheableResponse($methods, array $supported_formats, $request_format, array $request_headers, $request_body, $expected_response_format, $expected_response_content_type, $expected_response_content) { $rest_config_name = $this->randomMachineName(); $parameters = []; @@ -196,7 +196,7 @@ public function testOnRespondWithUncacheableResponse($methods, array $supported_ HttpKernelInterface::MASTER_REQUEST, $handler_response ); - $resource_response_subscriber->onRespond($event); + $resource_response_subscriber->onResponse($event); $final_response = $event->getResponse(); $this->assertNotInstanceOf(ResourceResponseInterface::class, $final_response); $this->assertNotInstanceOf(CacheableResponseInterface::class, $final_response);