From 2e487c58416278f87f4d3f571c90431a033d05c2 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Tue, 8 Jan 2019 16:13:32 +0100 Subject: [PATCH 1/6] Add test for unsupported Accept header which happens to match a Symfony built-in format --- src/EventListener/AddFormatListener.php | 2 +- tests/EventListener/AddFormatListenerTest.php | 62 ++++++++++++------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/src/EventListener/AddFormatListener.php b/src/EventListener/AddFormatListener.php index 89f6ee6dbc3..f8d079f0ec9 100644 --- a/src/EventListener/AddFormatListener.php +++ b/src/EventListener/AddFormatListener.php @@ -23,7 +23,7 @@ use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; /** - * Chooses the format to user according to the Accept header and supported formats. + * Chooses the format to use according to the Accept header and supported formats. * * @author Kévin Dunglas */ diff --git a/tests/EventListener/AddFormatListenerTest.php b/tests/EventListener/AddFormatListenerTest.php index 7eae772c470..6000f02a946 100644 --- a/tests/EventListener/AddFormatListenerTest.php +++ b/tests/EventListener/AddFormatListenerTest.php @@ -32,7 +32,7 @@ public function testNoResourceClass() $request = new Request(); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); @@ -50,11 +50,11 @@ public function testSupportedRequestFormat() $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -69,11 +69,11 @@ public function testRespondFlag() $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -91,10 +91,11 @@ public function testUnsupportedRequestFormat() $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); + $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -108,11 +109,11 @@ public function testSupportedAcceptHeader() $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml, application/json;q=0.9, */*;q=0.8'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -126,11 +127,11 @@ public function testAcceptAllHeader() $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml;q=0.9, */*;q=0.8'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -148,11 +149,30 @@ public function testUnsupportedAcceptHeader() $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml;q=0.9'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); + $event = $eventProphecy->reveal(); + + $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); + + $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); + $listener->onKernelRequest($event); + } + + public function testUnsupportedAcceptHeaderSymfonyBuiltInFormat() + { + $this->expectException(NotAcceptableHttpException::class); + $this->expectExceptionMessage('Requested format "text/xml" is not supported. Supported MIME types are "application/json".'); + + $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request->headers->set('Accept', 'text/xml'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -167,11 +187,11 @@ public function testInvalidAcceptHeader() $request->headers->set('Accept', 'invalid'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -184,11 +204,11 @@ public function testAcceptHeaderTakePrecedenceOverRequestFormat() $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['application/xml'], 'json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['application/xml'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -204,11 +224,11 @@ public function testInvalidRouteFormat() $request = new Request([], [], ['_api_resource_class' => 'Foo', '_format' => 'invalid']); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -220,11 +240,11 @@ public function testResourceClassSupportedRequestFormat() $request->setRequestFormat('csv'); $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); + $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes(['resource_class' => 'Foo', 'collection_operation_name' => 'get', 'receive' => true])->willReturn(['csv' => ['text/csv']])->shouldBeCalled(); + $formatsProviderProphecy->getFormatsFromAttributes(['resource_class' => 'Foo', 'collection_operation_name' => 'get', 'receive' => true])->willReturn(['csv' => ['text/csv']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); From b7420de17d6e20136382dbea0fa3d74ef646fdd7 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Tue, 8 Jan 2019 16:24:09 +0100 Subject: [PATCH 2/6] Add test for supported Accept header which happens to match a Symfony built-in format --- tests/EventListener/AddFormatListenerTest.php | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/tests/EventListener/AddFormatListenerTest.php b/tests/EventListener/AddFormatListenerTest.php index 6000f02a946..ecea01e8b50 100644 --- a/tests/EventListener/AddFormatListenerTest.php +++ b/tests/EventListener/AddFormatListenerTest.php @@ -121,6 +121,24 @@ public function testSupportedAcceptHeader() $this->assertSame('json', $request->getRequestFormat()); } + public function testSupportedAcceptHeaderSymfonyBuiltInFormat() + { + $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request->headers->set('Accept', 'application/json'); + + $eventProphecy = $this->prophesize(GetResponseEvent::class); + $eventProphecy->getRequest()->willReturn($request); + $event = $eventProphecy->reveal(); + + $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); + $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['jsonld' => ['application/ld+json', 'application/json']]); + + $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); + $listener->onKernelRequest($event); + + $this->assertSame('jsonld', $request->getRequestFormat()); + } + public function testAcceptAllHeader() { $request = new Request([], [], ['_api_resource_class' => 'Foo']); From 95c25be8cd355a83f9fa5bbfb6efa12f30e36449 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Tue, 8 Jan 2019 17:01:29 +0100 Subject: [PATCH 3/6] Use our own FormatMatcher instead of Request::getFormat --- src/Api/FormatMatcher.php | 58 +++++++++++++++++ src/EventListener/AddFormatListener.php | 24 ++++--- src/EventListener/DeserializeListener.php | 16 +++-- tests/EventListener/AddFormatListenerTest.php | 65 +++++++------------ 4 files changed, 104 insertions(+), 59 deletions(-) create mode 100644 src/Api/FormatMatcher.php diff --git a/src/Api/FormatMatcher.php b/src/Api/FormatMatcher.php new file mode 100644 index 00000000000..6e507cc971d --- /dev/null +++ b/src/Api/FormatMatcher.php @@ -0,0 +1,58 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Api; + +/** + * Matches a mime type to a format. + * + * @internal + */ +final class FormatMatcher +{ + private $formats; + + public function __construct(array $formats) + { + $normalizedFormats = []; + foreach ($formats as $format => $mimeTypes) { + $normalizedFormats[$format] = (array) $mimeTypes; + } + $this->formats = $normalizedFormats; + } + + /** + * Gets the format associated with the mime type. + * + * Adapted from {@see \Symfony\Component\HttpFoundation\Request::getFormat}. + */ + public function getFormat(string $mimeType): ?string + { + $canonicalMimeType = null; + $pos = strpos($mimeType, ';'); + if (false !== $pos) { + $canonicalMimeType = trim(substr($mimeType, 0, $pos)); + } + + foreach ($this->formats as $format => $mimeTypes) { + if (\in_array($mimeType, $mimeTypes, true)) { + return $format; + } + if (null !== $canonicalMimeType && \in_array($canonicalMimeType, $mimeTypes, true)) { + return $format; + } + } + + return null; + } +} diff --git a/src/EventListener/AddFormatListener.php b/src/EventListener/AddFormatListener.php index f8d079f0ec9..72e53713dda 100644 --- a/src/EventListener/AddFormatListener.php +++ b/src/EventListener/AddFormatListener.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\EventListener; +use ApiPlatform\Core\Api\FormatMatcher; use ApiPlatform\Core\Api\FormatsProviderInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Util\RequestAttributesExtractor; @@ -33,6 +34,7 @@ final class AddFormatListener private $formats = []; private $mimeTypes; private $formatsProvider; + private $formatMatcher; /** * @throws InvalidArgumentException @@ -43,14 +45,13 @@ public function __construct(Negotiator $negotiator, /* FormatsProviderInterface if (\is_array($formatsProvider)) { @trigger_error('Using an array as formats provider is deprecated since API Platform 2.3 and will not be possible anymore in API Platform 3', E_USER_DEPRECATED); $this->formats = $formatsProvider; + } else { + if (!$formatsProvider instanceof FormatsProviderInterface) { + throw new InvalidArgumentException(sprintf('The "$formatsProvider" argument is expected to be an implementation of the "%s" interface.', FormatsProviderInterface::class)); + } - return; + $this->formatsProvider = $formatsProvider; } - if (!$formatsProvider instanceof FormatsProviderInterface) { - throw new InvalidArgumentException(sprintf('The "$formatsProvider" argument is expected to be an implementation of the "%s" interface.', FormatsProviderInterface::class)); - } - - $this->formatsProvider = $formatsProvider; } /** @@ -69,6 +70,7 @@ public function onKernelRequest(GetResponseEvent $event) if (null !== $this->formatsProvider) { $this->formats = $this->formatsProvider->getFormatsFromAttributes(RequestAttributesExtractor::extractAttributes($request)); } + $this->formatMatcher = new FormatMatcher($this->formats); $this->populateMimeTypes(); $this->addRequestFormats($request, $this->formats); @@ -86,11 +88,11 @@ public function onKernelRequest(GetResponseEvent $event) /** @var string|null $accept */ $accept = $request->headers->get('Accept'); if (null !== $accept) { - if (null === $acceptHeader = $this->negotiator->getBest($accept, $mimeTypes)) { + if (null === $mediaType = $this->negotiator->getBest($accept, $mimeTypes)) { throw $this->getNotAcceptableHttpException($accept, $mimeTypes); } - $request->setRequestFormat($request->getFormat($acceptHeader->getType())); + $request->setRequestFormat($this->formatMatcher->getFormat($mediaType->getType())); return; } @@ -116,12 +118,14 @@ public function onKernelRequest(GetResponseEvent $event) } /** - * Adds API formats to the HttpFoundation Request. + * Adds the supported formats to the request. + * + * This is necessary for {@see Request::getMimeType} and {@see Request::getMimeTypes} to work. */ private function addRequestFormats(Request $request, array $formats) { foreach ($formats as $format => $mimeTypes) { - $request->setFormat($format, $mimeTypes); + $request->setFormat($format, (array) $mimeTypes); } } diff --git a/src/EventListener/DeserializeListener.php b/src/EventListener/DeserializeListener.php index 841242e4884..0a81c0bea3b 100644 --- a/src/EventListener/DeserializeListener.php +++ b/src/EventListener/DeserializeListener.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\EventListener; +use ApiPlatform\Core\Api\FormatMatcher; use ApiPlatform\Core\Api\FormatsProviderInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Serializer\SerializerContextBuilderInterface; @@ -34,6 +35,7 @@ final class DeserializeListener private $serializerContextBuilder; private $formats = []; private $formatsProvider; + private $formatMatcher; /** * @throws InvalidArgumentException @@ -45,14 +47,13 @@ public function __construct(SerializerInterface $serializer, SerializerContextBu if (\is_array($formatsProvider)) { @trigger_error('Using an array as formats provider is deprecated since API Platform 2.3 and will not be possible anymore in API Platform 3', E_USER_DEPRECATED); $this->formats = $formatsProvider; + } else { + if (!$formatsProvider instanceof FormatsProviderInterface) { + throw new InvalidArgumentException(sprintf('The "$formatsProvider" argument is expected to be an implementation of the "%s" interface.', FormatsProviderInterface::class)); + } - return; + $this->formatsProvider = $formatsProvider; } - if (!$formatsProvider instanceof FormatsProviderInterface) { - throw new InvalidArgumentException(sprintf('The "$formatsProvider" argument is expected to be an implementation of the "%s" interface.', FormatsProviderInterface::class)); - } - - $this->formatsProvider = $formatsProvider; } /** @@ -78,6 +79,7 @@ public function onKernelRequest(GetResponseEvent $event) if (null !== $this->formatsProvider) { $this->formats = $this->formatsProvider->getFormatsFromAttributes($attributes); } + $this->formatMatcher = new FormatMatcher($this->formats); $format = $this->getFormat($request); $context = $this->serializerContextBuilder->createFromRequest($request, false, $attributes); @@ -110,7 +112,7 @@ private function getFormat(Request $request): string throw new NotAcceptableHttpException('The "Content-Type" header must exist.'); } - $format = $request->getFormat($contentType); + $format = $this->formatMatcher->getFormat($contentType); if (null === $format || !isset($this->formats[$format])) { $supportedMimeTypes = []; foreach ($this->formats as $mimeTypes) { diff --git a/tests/EventListener/AddFormatListenerTest.php b/tests/EventListener/AddFormatListenerTest.php index ecea01e8b50..4b85bec772d 100644 --- a/tests/EventListener/AddFormatListenerTest.php +++ b/tests/EventListener/AddFormatListenerTest.php @@ -17,6 +17,7 @@ use ApiPlatform\Core\EventListener\AddFormatListener; use Negotiation\Negotiator; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Symfony\Component\HttpFoundation\Request; use Symfony\Component\HttpKernel\Event\GetResponseEvent; use Symfony\Component\HttpKernel\Exception\NotAcceptableHttpException; @@ -36,17 +37,16 @@ public function testNoResourceClass() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes()->shouldNotBeCalled(); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); - $this->assertNull($request->getFormat('application/vnd.notexist')); + $this->assertNull($request->getRequestFormat(null)); } public function testSupportedRequestFormat() { - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -54,7 +54,7 @@ public function testSupportedRequestFormat() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['xml' => ['text/xml']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -73,7 +73,7 @@ public function testRespondFlag() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['text/xml']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['xml' => ['text/xml']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -87,7 +87,7 @@ public function testUnsupportedRequestFormat() $this->expectException(NotAcceptableHttpException::class); $this->expectExceptionMessage('Requested format "text/xml" is not supported. Supported MIME types are "application/json".'); - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -95,7 +95,7 @@ public function testUnsupportedRequestFormat() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -105,7 +105,7 @@ public function testUnsupportedRequestFormat() public function testSupportedAcceptHeader() { - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml, application/json;q=0.9, */*;q=0.8'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -113,7 +113,7 @@ public function testSupportedAcceptHeader() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -123,7 +123,7 @@ public function testSupportedAcceptHeader() public function testSupportedAcceptHeaderSymfonyBuiltInFormat() { - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'application/json'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -131,7 +131,7 @@ public function testSupportedAcceptHeaderSymfonyBuiltInFormat() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['jsonld' => ['application/ld+json', 'application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['jsonld' => ['application/ld+json', 'application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -141,7 +141,7 @@ public function testSupportedAcceptHeaderSymfonyBuiltInFormat() public function testAcceptAllHeader() { - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml;q=0.9, */*;q=0.8'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -149,7 +149,7 @@ public function testAcceptAllHeader() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -163,7 +163,7 @@ public function testUnsupportedAcceptHeader() $this->expectException(NotAcceptableHttpException::class); $this->expectExceptionMessage('Requested format "text/html, application/xhtml+xml, application/xml;q=0.9" is not supported. Supported MIME types are "application/octet-stream", "application/json".'); - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'text/html, application/xhtml+xml, application/xml;q=0.9'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -171,7 +171,7 @@ public function testUnsupportedAcceptHeader() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['binary' => ['application/octet-stream'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -182,7 +182,7 @@ public function testUnsupportedAcceptHeaderSymfonyBuiltInFormat() $this->expectException(NotAcceptableHttpException::class); $this->expectExceptionMessage('Requested format "text/xml" is not supported. Supported MIME types are "application/json".'); - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'text/xml'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -190,7 +190,7 @@ public function testUnsupportedAcceptHeaderSymfonyBuiltInFormat() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -201,7 +201,7 @@ public function testInvalidAcceptHeader() $this->expectException(NotAcceptableHttpException::class); $this->expectExceptionMessage('Requested format "invalid" is not supported. Supported MIME types are "application/json".'); - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'invalid'); $eventProphecy = $this->prophesize(GetResponseEvent::class); @@ -209,7 +209,7 @@ public function testInvalidAcceptHeader() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -217,7 +217,7 @@ public function testInvalidAcceptHeader() public function testAcceptHeaderTakePrecedenceOverRequestFormat() { - $request = new Request([], [], ['_api_resource_class' => 'Foo']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); $request->headers->set('Accept', 'application/json'); $request->setRequestFormat('xml'); @@ -226,7 +226,7 @@ public function testAcceptHeaderTakePrecedenceOverRequestFormat() $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['xml' => ['application/xml'], 'json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['xml' => ['application/xml'], 'json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); @@ -239,38 +239,19 @@ public function testInvalidRouteFormat() $this->expectException(NotFoundHttpException::class); $this->expectExceptionMessage('Format "invalid" is not supported'); - $request = new Request([], [], ['_api_resource_class' => 'Foo', '_format' => 'invalid']); + $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get', '_format' => 'invalid']); $eventProphecy = $this->prophesize(GetResponseEvent::class); $eventProphecy->getRequest()->willReturn($request); $event = $eventProphecy->reveal(); $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes([])->willReturn(['json' => ['application/json']]); + $formatsProviderProphecy->getFormatsFromAttributes(Argument::any())->willReturn(['json' => ['application/json']]); $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); $listener->onKernelRequest($event); } - public function testResourceClassSupportedRequestFormat() - { - $request = new Request([], [], ['_api_resource_class' => 'Foo', '_api_collection_operation_name' => 'get']); - $request->setRequestFormat('csv'); - - $eventProphecy = $this->prophesize(GetResponseEvent::class); - $eventProphecy->getRequest()->willReturn($request); - $event = $eventProphecy->reveal(); - - $formatsProviderProphecy = $this->prophesize(FormatsProviderInterface::class); - $formatsProviderProphecy->getFormatsFromAttributes(['resource_class' => 'Foo', 'collection_operation_name' => 'get', 'receive' => true])->willReturn(['csv' => ['text/csv']]); - - $listener = new AddFormatListener(new Negotiator(), $formatsProviderProphecy->reveal()); - $listener->onKernelRequest($event); - - $this->assertSame('csv', $request->getRequestFormat()); - $this->assertSame('text/csv', $request->getMimeType($request->getRequestFormat())); - } - public function testBadFormatsProviderParameterThrowsException() { $this->expectException(\ApiPlatform\Core\Exception\InvalidArgumentException::class); From b14e1b274a9f6cfac93d27e402d87ad057cbc977 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lukas=20L=C3=BCcke?= Date: Wed, 9 Jan 2019 17:03:55 +0100 Subject: [PATCH 4/6] Check item resource class in mutation (#2441) This prevents passing IRIs belonging to different resource classes --- features/graphql/mutation.feature | 15 +++++++++++++++ .../Factory/ItemMutationResolverFactory.php | 6 ++++++ 2 files changed, 21 insertions(+) diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index 60344cb79b4..332f8a54aa3 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -122,6 +122,21 @@ Feature: GraphQL mutation support And the JSON node "data.deleteFoo.id" should be equal to "/foos/1" And the JSON node "data.deleteFoo.clientMutationId" should be equal to "anotherId" + Scenario: Trigger an error trying to delete item of different resource + When I send the following GraphQL request: + """ + mutation { + deleteFoo(input: {id: "/dummies/1", clientMutationId: "myId"}) { + id + clientMutationId + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/json" + And the JSON node "errors[0].message" should be equal to 'Item "/dummies/1" did not match expected type "ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo".' + Scenario: Delete an item with composite identifiers through a mutation Given there are Composite identifier objects When I send the following GraphQL request: diff --git a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php index b5d33a3e3d2..8f185f06153 100644 --- a/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php +++ b/src/GraphQl/Resolver/Factory/ItemMutationResolverFactory.php @@ -23,6 +23,7 @@ use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; use ApiPlatform\Core\Security\ResourceAccessCheckerInterface; +use ApiPlatform\Core\Util\ClassInfoTrait; use ApiPlatform\Core\Validator\Exception\ValidationException; use ApiPlatform\Core\Validator\ValidatorInterface; use GraphQL\Error\Error; @@ -39,6 +40,7 @@ */ final class ItemMutationResolverFactory implements ResolverFactoryInterface { + use ClassInfoTrait; use FieldsToAttributesTrait; use ResourceAccessCheckerTrait; @@ -83,6 +85,10 @@ public function __invoke(string $resourceClass = null, string $rootClass = null, } catch (ItemNotFoundException $e) { throw Error::createLocatedError(sprintf('Item "%s" not found.', $args['input']['id']), $info->fieldNodes, $info->path); } + + if ($resourceClass !== $this->getObjectClass($item)) { + throw Error::createLocatedError(sprintf('Item "%s" did not match expected type "%s".', $args['input']['id'], $resourceClass), $info->fieldNodes, $info->path); + } } $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass); From fa8d31b7183ead1787b3c257b5db4edd16b45eac Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Wed, 9 Jan 2019 17:02:38 +0100 Subject: [PATCH 5/6] Fix normalization of raw collections (not API resources) --- .../CollectionFiltersNormalizer.php | 11 +- src/Hydra/Serializer/CollectionNormalizer.php | 29 ++- .../AbstractCollectionNormalizer.php | 38 +++- tests/Fixtures/Foo.php | 20 ++ tests/Fixtures/NotAResource.php | 2 +- .../CollectionFiltersNormalizerTest.php | 144 +++++++++++- .../Serializer/CollectionNormalizerTest.php | 211 +++++++++++++++++- 7 files changed, 414 insertions(+), 41 deletions(-) create mode 100644 tests/Fixtures/Foo.php diff --git a/src/Hydra/Serializer/CollectionFiltersNormalizer.php b/src/Hydra/Serializer/CollectionFiltersNormalizer.php index d69a70b2744..ff5d750484d 100644 --- a/src/Hydra/Serializer/CollectionFiltersNormalizer.php +++ b/src/Hydra/Serializer/CollectionFiltersNormalizer.php @@ -17,6 +17,7 @@ use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\Api\FilterLocatorTrait; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use Psr\Container\ContainerInterface; use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface; @@ -74,7 +75,15 @@ public function normalize($object, $format = null, array $context = []) return $data; } - $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); + try { + $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); + } catch (InvalidArgumentException $e) { + if (!isset($context['resource_class'])) { + return $data; + } + + throw $e; + } $resourceMetadata = $this->resourceMetadataFactory->create($resourceClass); $operationName = $context['collection_operation_name'] ?? null; diff --git a/src/Hydra/Serializer/CollectionNormalizer.php b/src/Hydra/Serializer/CollectionNormalizer.php index 91b8f4825c5..a4ce0b24cb7 100644 --- a/src/Hydra/Serializer/CollectionNormalizer.php +++ b/src/Hydra/Serializer/CollectionNormalizer.php @@ -18,6 +18,7 @@ use ApiPlatform\Core\Api\ResourceClassResolverInterface; use ApiPlatform\Core\DataProvider\PaginatorInterface; use ApiPlatform\Core\DataProvider\PartialPaginatorInterface; +use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\JsonLd\ContextBuilderInterface; use ApiPlatform\Core\JsonLd\Serializer\JsonLdContextTrait; use ApiPlatform\Core\Serializer\ContextTrait; @@ -65,15 +66,18 @@ public function supportsNormalization($data, $format = null) public function normalize($object, $format = null, array $context = []) { if (isset($context['api_sub_level'])) { - $data = []; - foreach ($object as $index => $obj) { - $data[$index] = $this->normalizer->normalize($obj, $format, $context); + return $this->normalizeRawCollection($object, $format, $context); + } + + try { + $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); + } catch (InvalidArgumentException $e) { + if (!isset($context['resource_class'])) { + return $this->normalizeRawCollection($object, $format, $context); } - return $data; + throw $e; } - - $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); $data = $this->addJsonLdContext($this->contextBuilder, $resourceClass, $context); $context = $this->initContext($resourceClass, $context); @@ -109,4 +113,17 @@ public function hasCacheableSupportsMethod(): bool { return true; } + + /** + * Normalizes a raw collection (not API resources). + */ + private function normalizeRawCollection($object, $format = null, array $context = []): array + { + $data = []; + foreach ($object as $index => $obj) { + $data[$index] = $this->normalizer->normalize($obj, $format, $context); + } + + return $data; + } } diff --git a/src/Serializer/AbstractCollectionNormalizer.php b/src/Serializer/AbstractCollectionNormalizer.php index 198ff7a3bfc..1310ca378f9 100644 --- a/src/Serializer/AbstractCollectionNormalizer.php +++ b/src/Serializer/AbstractCollectionNormalizer.php @@ -16,6 +16,7 @@ use ApiPlatform\Core\Api\ResourceClassResolverInterface; use ApiPlatform\Core\DataProvider\PaginatorInterface; use ApiPlatform\Core\DataProvider\PartialPaginatorInterface; +use ApiPlatform\Core\Exception\InvalidArgumentException; use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareInterface; use Symfony\Component\Serializer\Normalizer\NormalizerAwareTrait; @@ -28,7 +29,9 @@ */ abstract class AbstractCollectionNormalizer implements NormalizerInterface, NormalizerAwareInterface, CacheableSupportsMethodInterface { - use ContextTrait { initContext as protected; } + use ContextTrait { + initContext as protected; + } use NormalizerAwareTrait; /** @@ -68,19 +71,21 @@ public function hasCacheableSupportsMethod(): bool */ public function normalize($object, $format = null, array $context = []) { - $data = []; if (isset($context['api_sub_level'])) { - foreach ($object as $index => $obj) { - $data[$index] = $this->normalizer->normalize($obj, $format, $context); + return $this->normalizeRawCollection($object, $format, $context); + } + + try { + $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); + } catch (InvalidArgumentException $e) { + if (!isset($context['resource_class'])) { + return $this->normalizeRawCollection($object, $format, $context); } - return $data; + throw $e; } - - $context = $this->initContext( - $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true), - $context - ); + $data = []; + $context = $this->initContext($resourceClass, $context); return array_merge_recursive( $data, @@ -89,6 +94,19 @@ public function normalize($object, $format = null, array $context = []) ); } + /** + * Normalizes a raw collection (not API resources). + */ + protected function normalizeRawCollection($object, $format = null, array $context = []): array + { + $data = []; + foreach ($object as $index => $obj) { + $data[$index] = $this->normalizer->normalize($obj, $format, $context); + } + + return $data; + } + /** * Gets the pagination configuration. * diff --git a/tests/Fixtures/Foo.php b/tests/Fixtures/Foo.php new file mode 100644 index 00000000000..d7215f4997e --- /dev/null +++ b/tests/Fixtures/Foo.php @@ -0,0 +1,20 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures; + +class Foo +{ + public $id; + public $bar; +} diff --git a/tests/Fixtures/NotAResource.php b/tests/Fixtures/NotAResource.php index 9da944afe45..b7323769792 100644 --- a/tests/Fixtures/NotAResource.php +++ b/tests/Fixtures/NotAResource.php @@ -14,7 +14,7 @@ namespace ApiPlatform\Core\Tests\Fixtures; /** - * This class is mapped as an API resource. + * This class is not mapped as an API resource. * * @author Kévin Dunglas */ diff --git a/tests/Hydra/Serializer/CollectionFiltersNormalizerTest.php b/tests/Hydra/Serializer/CollectionFiltersNormalizerTest.php index 69324dfaebc..2b930f623bf 100644 --- a/tests/Hydra/Serializer/CollectionFiltersNormalizerTest.php +++ b/tests/Hydra/Serializer/CollectionFiltersNormalizerTest.php @@ -14,14 +14,19 @@ namespace ApiPlatform\Core\Tests\Hydra\Serializer; use ApiPlatform\Core\Api\FilterCollection; +use ApiPlatform\Core\Api\OperationType; use ApiPlatform\Core\Api\ResourceClassResolverInterface; use ApiPlatform\Core\Bridge\Doctrine\Orm\Filter\FilterInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Hydra\Serializer\CollectionFiltersNormalizer; +use ApiPlatform\Core\Hydra\Serializer\CollectionNormalizer; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; +use ApiPlatform\Core\Tests\Fixtures\Foo; +use ApiPlatform\Core\Tests\Fixtures\NotAResource; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Psr\Container\ContainerInterface; use Symfony\Component\Serializer\Normalizer\CacheableSupportsMethodInterface; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; @@ -49,24 +54,141 @@ public function testSupportsNormalization() $this->assertTrue($normalizer->hasCacheableSupportsMethod()); } - public function testDoNothingIfSubLevel() + public function testNormalizeNonResourceCollection() { - $dummy = new Dummy(); + $notAResourceA = new NotAResource('A', 'buzz'); + $notAResourceB = new NotAResource('B', 'bzzt'); + + $data = [$notAResourceA, $notAResourceB]; + + $normalizedNotAResourceA = [ + 'foo' => 'A', + 'bar' => 'buzz', + ]; + + $normalizedNotAResourceB = [ + 'foo' => 'B', + 'bar' => 'bzzt', + ]; $decoratedProphecy = $this->prophesize(NormalizerInterface::class); - $decoratedProphecy->normalize($dummy, null, ['api_sub_level' => true])->willReturn(['name' => 'foo'])->shouldBeCalled(); + $decoratedProphecy->normalize($data, CollectionNormalizer::FORMAT, Argument::any())->willReturn([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ]); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); - $resourceClassResolverProphecy->getResourceClass()->shouldNotBeCalled(); + $resourceClassResolverProphecy->getResourceClass($data, null, true)->willThrow(InvalidArgumentException::class); - $normalizer = new CollectionFiltersNormalizer( - $decoratedProphecy->reveal(), - $this->prophesize(ResourceMetadataFactoryInterface::class)->reveal(), - $resourceClassResolverProphecy->reveal(), - $this->prophesize(ContainerInterface::class)->reveal() - ); + $filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + + $normalizer = new CollectionFiltersNormalizer($decoratedProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $filterLocatorProphecy->reveal()); - $this->assertEquals(['name' => 'foo'], $normalizer->normalize($dummy, null, ['api_sub_level' => true])); + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + ]); + + $this->assertEquals([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ], $actual); + } + + public function testNormalizeSubLevelResourceCollection() + { + $fooOne = new Foo(); + $fooOne->id = 1; + $fooOne->bar = 'baz'; + + $fooThree = new Foo(); + $fooThree->id = 3; + $fooThree->bar = 'bzz'; + + $data = [$fooOne, $fooThree]; + + $normalizedFooOne = [ + '@id' => '/foos/1', + '@type' => 'Foo', + 'bar' => 'baz', + ]; + + $normalizedFooThree = [ + '@id' => '/foos/3', + '@type' => 'Foo', + 'bar' => 'bzz', + ]; + + $decoratedProphecy = $this->prophesize(NormalizerInterface::class); + $decoratedProphecy->normalize($data, CollectionNormalizer::FORMAT, Argument::allOf( + Argument::withEntry('resource_class', Foo::class), + Argument::withEntry('api_sub_level', true) + ))->willReturn([ + $normalizedFooOne, + $normalizedFooThree, + ]); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + + $filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + + $normalizer = new CollectionFiltersNormalizer($decoratedProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $filterLocatorProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + 'collection_operation_name' => 'get', + 'operation_type' => OperationType::COLLECTION, + 'resource_class' => Foo::class, + 'api_sub_level' => true, + ]); + + $this->assertEquals([ + $normalizedFooOne, + $normalizedFooThree, + ], $actual); + } + + public function testNormalizeSubLevelNonResourceCollection() + { + $notAResourceA = new NotAResource('A', 'buzz'); + $notAResourceB = new NotAResource('B', 'bzzt'); + + $data = [$notAResourceA, $notAResourceB]; + + $normalizedNotAResourceA = [ + 'foo' => 'A', + 'bar' => 'buzz', + ]; + + $normalizedNotAResourceB = [ + 'foo' => 'B', + 'bar' => 'bzzt', + ]; + + $decoratedProphecy = $this->prophesize(NormalizerInterface::class); + $decoratedProphecy->normalize($data, CollectionNormalizer::FORMAT, Argument::any())->willReturn([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ]); + + $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass($data, null, true)->willThrow(InvalidArgumentException::class); + + $filterLocatorProphecy = $this->prophesize(ContainerInterface::class); + + $normalizer = new CollectionFiltersNormalizer($decoratedProphecy->reveal(), $resourceMetadataFactoryProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $filterLocatorProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + 'api_sub_level' => true, + ]); + + $this->assertEquals([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ], $actual); } public function testDoNothingIfNoFilter() diff --git a/tests/Hydra/Serializer/CollectionNormalizerTest.php b/tests/Hydra/Serializer/CollectionNormalizerTest.php index b23243aa9d9..2f1d6896d48 100644 --- a/tests/Hydra/Serializer/CollectionNormalizerTest.php +++ b/tests/Hydra/Serializer/CollectionNormalizerTest.php @@ -14,13 +14,18 @@ namespace ApiPlatform\Core\Tests\Hydra\Serializer; use ApiPlatform\Core\Api\IriConverterInterface; +use ApiPlatform\Core\Api\OperationType; use ApiPlatform\Core\Api\ResourceClassResolverInterface; use ApiPlatform\Core\DataProvider\PaginatorInterface; use ApiPlatform\Core\DataProvider\PartialPaginatorInterface; +use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Hydra\Serializer\CollectionNormalizer; use ApiPlatform\Core\JsonLd\ContextBuilderInterface; use ApiPlatform\Core\Serializer\AbstractItemNormalizer; +use ApiPlatform\Core\Tests\Fixtures\Foo; +use ApiPlatform\Core\Tests\Fixtures\NotAResource; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Symfony\Component\Serializer\Normalizer\NormalizerInterface; use Symfony\Component\Serializer\SerializerInterface; @@ -47,22 +52,204 @@ public function testSupportsNormalize() $this->assertTrue($normalizer->hasCacheableSupportsMethod()); } - public function testNormalizeApiSubLevel() + public function testNormalizeResourceCollection() { + $fooOne = new Foo(); + $fooOne->id = 1; + $fooOne->bar = 'baz'; + + $fooThree = new Foo(); + $fooThree->id = 3; + $fooThree->bar = 'bzz'; + + $data = [$fooOne, $fooThree]; + + $normalizedFooOne = [ + '@id' => '/foos/1', + '@type' => 'Foo', + 'bar' => 'baz', + ]; + + $normalizedFooThree = [ + '@id' => '/foos/3', + '@type' => 'Foo', + 'bar' => 'bzz', + ]; + + $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); + $contextBuilderProphecy->getResourceContextUri(Foo::class)->willReturn('/contexts/Foo'); + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); - $resourceClassResolverProphecy->getResourceClass(['foo' => 'bar'], null, true)->willReturn('Foo')->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass($data, Foo::class, true)->willReturn(Foo::class); - $iriConvert = $this->prophesize(IriConverterInterface::class); - $contextBuilder = $this->prophesize(ContextBuilderInterface::class); - $contextBuilder->getResourceContextUri('Foo')->willReturn('/contexts/Foo'); - $iriConvert->getIriFromResourceClass('Foo')->willReturn('/foo/1'); - $itemNormalizer = $this->prophesize(AbstractItemNormalizer::class); - $itemNormalizer->normalize('bar', null, ['jsonld_has_context' => true, 'api_sub_level' => true, 'resource_class' => 'Foo'])->willReturn(22); + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy->getIriFromResourceClass(Foo::class)->willReturn('/foos'); - $normalizer = new CollectionNormalizer($contextBuilder->reveal(), $resourceClassResolverProphecy->reveal(), $iriConvert->reveal()); + $delegateNormalizerProphecy = $this->prophesize(NormalizerInterface::class); + $delegateNormalizerProphecy->normalize($fooOne, CollectionNormalizer::FORMAT, Argument::allOf( + Argument::withEntry('resource_class', Foo::class), + Argument::withEntry('api_sub_level', true) + ))->willReturn($normalizedFooOne); + $delegateNormalizerProphecy->normalize($fooThree, CollectionNormalizer::FORMAT, Argument::allOf( + Argument::withEntry('resource_class', Foo::class), + Argument::withEntry('api_sub_level', true) + ))->willReturn($normalizedFooThree); - $normalizer->setNormalizer($itemNormalizer->reveal()); - $this->assertEquals(['@context' => '/contexts/Foo', '@id' => '/foo/1', '@type' => 'hydra:Collection', 'hydra:member' => [0 => '22'], 'hydra:totalItems' => 1], $normalizer->normalize(['foo' => 'bar'], null)); + $normalizer = new CollectionNormalizer($contextBuilderProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $iriConverterProphecy->reveal()); + $normalizer->setNormalizer($delegateNormalizerProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + 'collection_operation_name' => 'get', + 'operation_type' => OperationType::COLLECTION, + 'resource_class' => Foo::class, + ]); + + $this->assertEquals([ + '@context' => '/contexts/Foo', + '@id' => '/foos', + '@type' => 'hydra:Collection', + 'hydra:member' => [ + $normalizedFooOne, + $normalizedFooThree, + ], + 'hydra:totalItems' => 2, + ], $actual); + } + + public function testNormalizeNonResourceCollection() + { + $notAResourceA = new NotAResource('A', 'buzz'); + $notAResourceB = new NotAResource('B', 'bzzt'); + + $data = [$notAResourceA, $notAResourceB]; + + $normalizedNotAResourceA = [ + 'foo' => 'A', + 'bar' => 'buzz', + ]; + + $normalizedNotAResourceB = [ + 'foo' => 'B', + 'bar' => 'bzzt', + ]; + + $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + $resourceClassResolverProphecy->getResourceClass($data, null, true)->willThrow(InvalidArgumentException::class); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $delegateNormalizerProphecy = $this->prophesize(NormalizerInterface::class); + $delegateNormalizerProphecy->normalize($notAResourceA, CollectionNormalizer::FORMAT, Argument::any())->willReturn($normalizedNotAResourceA); + $delegateNormalizerProphecy->normalize($notAResourceB, CollectionNormalizer::FORMAT, Argument::any())->willReturn($normalizedNotAResourceB); + + $normalizer = new CollectionNormalizer($contextBuilderProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $iriConverterProphecy->reveal()); + $normalizer->setNormalizer($delegateNormalizerProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + ]); + + $this->assertEquals([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ], $actual); + } + + public function testNormalizeSubLevelResourceCollection() + { + $fooOne = new Foo(); + $fooOne->id = 1; + $fooOne->bar = 'baz'; + + $fooThree = new Foo(); + $fooThree->id = 3; + $fooThree->bar = 'bzz'; + + $data = [$fooOne, $fooThree]; + + $normalizedFooOne = [ + '@id' => '/foos/1', + '@type' => 'Foo', + 'bar' => 'baz', + ]; + + $normalizedFooThree = [ + '@id' => '/foos/3', + '@type' => 'Foo', + 'bar' => 'bzz', + ]; + + $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $delegateNormalizerProphecy = $this->prophesize(NormalizerInterface::class); + $delegateNormalizerProphecy->normalize($fooOne, CollectionNormalizer::FORMAT, Argument::allOf( + Argument::withEntry('resource_class', Foo::class), + Argument::withEntry('api_sub_level', true) + ))->willReturn($normalizedFooOne); + $delegateNormalizerProphecy->normalize($fooThree, CollectionNormalizer::FORMAT, Argument::allOf( + Argument::withEntry('resource_class', Foo::class), + Argument::withEntry('api_sub_level', true) + ))->willReturn($normalizedFooThree); + + $normalizer = new CollectionNormalizer($contextBuilderProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $iriConverterProphecy->reveal()); + $normalizer->setNormalizer($delegateNormalizerProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + 'collection_operation_name' => 'get', + 'operation_type' => OperationType::COLLECTION, + 'resource_class' => Foo::class, + 'api_sub_level' => true, + ]); + + $this->assertEquals([ + $normalizedFooOne, + $normalizedFooThree, + ], $actual); + } + + public function testNormalizeSubLevelNonResourceCollection() + { + $notAResourceA = new NotAResource('A', 'buzz'); + $notAResourceB = new NotAResource('B', 'bzzt'); + + $data = [$notAResourceA, $notAResourceB]; + + $normalizedNotAResourceA = [ + 'foo' => 'A', + 'bar' => 'buzz', + ]; + + $normalizedNotAResourceB = [ + 'foo' => 'B', + 'bar' => 'bzzt', + ]; + + $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); + + $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); + + $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + + $delegateNormalizerProphecy = $this->prophesize(NormalizerInterface::class); + $delegateNormalizerProphecy->normalize($notAResourceA, CollectionNormalizer::FORMAT, Argument::any())->willReturn($normalizedNotAResourceA); + $delegateNormalizerProphecy->normalize($notAResourceB, CollectionNormalizer::FORMAT, Argument::any())->willReturn($normalizedNotAResourceB); + + $normalizer = new CollectionNormalizer($contextBuilderProphecy->reveal(), $resourceClassResolverProphecy->reveal(), $iriConverterProphecy->reveal()); + $normalizer->setNormalizer($delegateNormalizerProphecy->reveal()); + + $actual = $normalizer->normalize($data, CollectionNormalizer::FORMAT, [ + 'api_sub_level' => true, + ]); + + $this->assertEquals([ + $normalizedNotAResourceA, + $normalizedNotAResourceB, + ], $actual); } public function testNormalizePaginator() @@ -73,7 +260,7 @@ public function testNormalizePaginator() '@id' => '/foo/1', '@type' => 'hydra:Collection', 'hydra:member' => [ - 0 => [ + [ 'name' => 'Kévin', 'friend' => 'Smail', ], From 5dcfe2e5d5be072a7baf1a56ed2a33273a542bd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 14 Jan 2019 18:12:56 +0100 Subject: [PATCH 6/6] Update changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e17018d87ea..75cbf120d74 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +## 2.3.6 + +* Fix normalization of raw collections (not API resources) +* Fix content negotiation format matching + ## 2.3.5 * GraphQL: compatibility with `webonyx/graphql-php` 0.13