From 6fad2588715eb8f6d0cb05390eaaab5d2a7a6eb8 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Thu, 18 Apr 2019 15:38:34 +0200 Subject: [PATCH 1/2] Fix HTTP status code when output class is set to null Also fix Behat test for when input class is set to null --- features/jsonld/input_output.feature | 47 ++++++++++++++++--- phpstan.neon.dist | 3 ++ src/EventListener/SerializeListener.php | 11 +---- .../InputOutputResourceMetadataFactory.php | 9 ++++ .../ApiPlatformExtensionTest.php | 9 ++++ tests/EventListener/SerializeListenerTest.php | 6 +-- .../DummyDtoNoInput/CreateItemAction.php | 36 ++++++++++++++ .../DummyDtoNoInput/DoubleBatAction.php | 31 ++++++++++++ ...myDtoNoInputToOutputDtoDataTransformer.php | 47 +++++++++++++++++++ .../TestBundle/Document/DummyDtoNoInput.php | 22 ++++++++- .../TestBundle/Entity/DummyDtoNoInput.php | 22 ++++++++- tests/Fixtures/app/config/config_common.yml | 12 +++++ 12 files changed, 235 insertions(+), 20 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/CreateItemAction.php create mode 100644 tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/DoubleBatAction.php create mode 100644 tests/Fixtures/TestBundle/DataTransformer/DummyDtoNoInputToOutputDtoDataTransformer.php diff --git a/features/jsonld/input_output.feature b/features/jsonld/input_output.feature index 7d3880aa633..12f1492f870 100644 --- a/features/jsonld/input_output.feature +++ b/features/jsonld/input_output.feature @@ -92,7 +92,7 @@ Feature: JSON-LD DTO input and output "ipsum": "1" } """ - Then the response status code should be 201 + Then the response status code should be 204 And the response should be empty @createSchema @@ -187,15 +187,50 @@ Feature: JSON-LD DTO input and output @createSchema Scenario: Create a resource with no input - When I send a "POST" request to "/dummy_dto_no_inputs" with body: + When I send a "POST" request to "/dummy_dto_no_inputs" + Then the response status code should be 201 + And the response should be in JSON + And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" + And the JSON should be equal to: """ { - "foo": "test", - "bar": 1 + "@context": { + "@vocab": "http://example.com/docs.jsonld#", + "hydra": "http://www.w3.org/ns/hydra/core#", + "id": "OutputDto/id", + "baz": "OutputDto/baz", + "bat": "OutputDto/bat" + }, + "@type": "DummyDtoNoInput", + "@id": "/dummy_dto_no_inputs/1", + "id": 1, + "baz": 1, + "bat": "test" + } + """ + + Scenario: Update a resource with no input + When I send a "POST" request to "/dummy_dto_no_inputs/1/double_bat" + 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/ld+json; charset=utf-8" + And the JSON should be equal to: + """ + { + "@context": { + "@vocab": "http://example.com/docs.jsonld#", + "hydra": "http://www.w3.org/ns/hydra/core#", + "id": "OutputDto/id", + "baz": "OutputDto/baz", + "bat": "OutputDto/bat" + }, + "@type": "DummyDtoNoInput", + "@id": "/dummy_dto_no_inputs/1", + "id": 1, + "baz": 1, + "bat": "testtest" } """ - Then the response status code should be 201 - And the response should be empty @!mongodb Scenario: Use messenger with an input where the handler gives a synchronous result diff --git a/phpstan.neon.dist b/phpstan.neon.dist index 89ab139a10d..563fed5c6ac 100644 --- a/phpstan.neon.dist +++ b/phpstan.neon.dist @@ -70,6 +70,9 @@ parameters: - message: '#Method ApiPlatform\\Core\\Bridge\\Doctrine\\Orm\\Util\\QueryBuilderHelper::mapJoinAliases() should return array\|string> but returns array\.#' path: %currentWorkingDirectory%/src/Bridge/Doctrine/Orm/Util/QueryBuilderHelper.php + - + message: "#Call to function method_exists\\(\\) with 'Symfony\\\\\\\\Component.+' and 'addRemovedBindingIds?' will always evaluate to false\\.#" + path: %currentWorkingDirectory%/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php - "#Call to method PHPUnit\\\\Framework\\\\Assert::assertSame\\(\\) with array\\('(collection_context|item_context|subresource_context)'\\) and array\\|bool\\|float\\|int\\|string\\|null will always evaluate to false\\.#" # https://github.com/doctrine/doctrine2/pull/7298/files - '#Strict comparison using === between null and int will always evaluate to false\.#' diff --git a/src/EventListener/SerializeListener.php b/src/EventListener/SerializeListener.php index f982711f08f..7598bcbf534 100644 --- a/src/EventListener/SerializeListener.php +++ b/src/EventListener/SerializeListener.php @@ -61,15 +61,8 @@ public function onKernelView(GetResponseForControllerResultEvent $event): void $context = $this->serializerContextBuilder->createFromRequest($request, true, $attributes); - if ( - (isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class']) - || - ( - null === $controllerResult && isset($context['input']) && \array_key_exists('class', $context['input']) && - null === $context['input']['class'] - ) - ) { - $event->setControllerResult(''); + if (isset($context['output']) && \array_key_exists('class', $context['output']) && null === $context['output']['class']) { + $event->setControllerResult(null); return; } diff --git a/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php b/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php index bdf61225db5..7702f558617 100644 --- a/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/InputOutputResourceMetadataFactory.php @@ -64,6 +64,15 @@ private function getTransformedOperations(array $operations, array $resourceAttr $operation['input'] = isset($operation['input']) ? $this->transformInputOutput($operation['input']) : $resourceAttributes['input']; $operation['output'] = isset($operation['output']) ? $this->transformInputOutput($operation['output']) : $resourceAttributes['output']; + + if ( + !isset($operation['status']) + && isset($operation['output']) + && \array_key_exists('class', $operation['output']) + && null === $operation['output']['class'] + ) { + $operation['status'] = 204; + } } return $operations; diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 0ed71bd1c78..3b516a21027 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -843,6 +843,15 @@ private function getPartialContainerBuilderProphecy() $containerBuilderProphecy->getDefinition('api_platform.http_cache.purger.varnish')->willReturn(new Definition()); + // irrelevant, but to prevent errors + // https://github.com/symfony/symfony/pull/29944 + if (method_exists(ContainerBuilder::class, 'addRemovedBindingId')) { + $containerBuilderProphecy->addRemovedBindingId(Argument::type('string'))->will(function () {}); + } elseif (method_exists(ContainerBuilder::class, 'addRemovedBindingIds')) { + // https://github.com/symfony/symfony/pull/31173 + $containerBuilderProphecy->addRemovedBindingIds(Argument::type('string'))->will(function () {}); + } + return $containerBuilderProphecy; } diff --git a/tests/EventListener/SerializeListenerTest.php b/tests/EventListener/SerializeListenerTest.php index 48414dc9fbc..0507945d7a4 100644 --- a/tests/EventListener/SerializeListenerTest.php +++ b/tests/EventListener/SerializeListenerTest.php @@ -111,9 +111,9 @@ public function testSerializeCollectionOperationWithOutputClassDisabled() $request->setRequestFormat('xml'); $eventProphecy = $this->prophesize(GetResponseForControllerResultEvent::class); - $eventProphecy->getControllerResult()->willReturn(new \stdClass())->shouldBeCalled(); - $eventProphecy->getRequest()->willReturn($request)->shouldBeCalled(); - $eventProphecy->setControllerResult('')->shouldBeCalled(); + $eventProphecy->getControllerResult()->willReturn(new \stdClass()); + $eventProphecy->getRequest()->willReturn($request); + $eventProphecy->setControllerResult(null)->shouldBeCalled(); $serializerContextBuilderProphecy = $this->prophesize(SerializerContextBuilderInterface::class); $serializerContextBuilderProphecy->createFromRequest(Argument::type(Request::class), true, Argument::type('array'))->willReturn($expectedContext)->shouldBeCalled(); diff --git a/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/CreateItemAction.php b/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/CreateItemAction.php new file mode 100644 index 00000000000..798c9a860b7 --- /dev/null +++ b/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/CreateItemAction.php @@ -0,0 +1,36 @@ + + * + * 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\TestBundle\Controller\DummyDtoNoInput; + +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput; +use Symfony\Component\HttpFoundation\Request; + +final class CreateItemAction +{ + public function __invoke(Request $request) + { + $resourceClass = $request->attributes->get('_api_resource_class'); + if (!\in_array($resourceClass, [DummyDtoNoInput::class, DummyDtoNoInputDocument::class], true)) { + throw new \InvalidArgumentException(); + } + + $data = new $resourceClass(); + + $data->lorem = 'test'; + $data->ipsum = 1; + + return $data; + } +} diff --git a/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/DoubleBatAction.php b/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/DoubleBatAction.php new file mode 100644 index 00000000000..963f53c0424 --- /dev/null +++ b/tests/Fixtures/TestBundle/Controller/DummyDtoNoInput/DoubleBatAction.php @@ -0,0 +1,31 @@ + + * + * 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\TestBundle\Controller\DummyDtoNoInput; + +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput; + +final class DoubleBatAction +{ + public function __invoke($data) + { + if (!$data instanceof DummyDtoNoInput && !$data instanceof DummyDtoNoInputDocument) { + throw new \InvalidArgumentException(); + } + + $data->lorem .= $data->lorem; + + return $data; + } +} diff --git a/tests/Fixtures/TestBundle/DataTransformer/DummyDtoNoInputToOutputDtoDataTransformer.php b/tests/Fixtures/TestBundle/DataTransformer/DummyDtoNoInputToOutputDtoDataTransformer.php new file mode 100644 index 00000000000..8f70d5b519b --- /dev/null +++ b/tests/Fixtures/TestBundle/DataTransformer/DummyDtoNoInputToOutputDtoDataTransformer.php @@ -0,0 +1,47 @@ + + * + * 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\TestBundle\DataTransformer; + +use ApiPlatform\Core\DataTransformer\DataTransformerInterface; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput; + +final class DummyDtoNoInputToOutputDtoDataTransformer implements DataTransformerInterface +{ + /** + * {@inheritdoc} + */ + public function transform($object, string $to, array $context = []) + { + if (!$object instanceof DummyDtoNoInput && !$object instanceof DummyDtoNoInputDocument) { + throw new \InvalidArgumentException(); + } + + $output = new OutputDto(); + $output->id = $object->getId(); + $output->bat = (string) $object->lorem; + $output->baz = (float) $object->ipsum; + + return $output; + } + + /** + * {@inheritdoc} + */ + public function supportsTransformation($data, string $to, array $context = []): bool + { + return ($data instanceof DummyDtoNoInput || $data instanceof DummyDtoNoInputDocument) && OutputDto::class === $to; + } +} diff --git a/tests/Fixtures/TestBundle/Document/DummyDtoNoInput.php b/tests/Fixtures/TestBundle/Document/DummyDtoNoInput.php index 05431be7d7e..ccdf40e7e71 100644 --- a/tests/Fixtures/TestBundle/Document/DummyDtoNoInput.php +++ b/tests/Fixtures/TestBundle/Document/DummyDtoNoInput.php @@ -14,6 +14,8 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Document; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\CreateItemAction; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\DoubleBatAction; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto; use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; @@ -28,7 +30,25 @@ * attributes={ * "input"=false, * "output"=OutputDto::class - * } + * }, + * collectionOperations={ + * "post"={ + * "method"="POST", + * "path"="/dummy_dto_no_inputs", + * "controller"=CreateItemAction::class, + * }, + * "get", + * }, + * itemOperations={ + * "get", + * "delete", + * "post_double_bat"={ + * "method"="POST", + * "path"="/dummy_dto_no_inputs/{id}/double_bat", + * "controller"=DoubleBatAction::class, + * "status"=200, + * }, + * }, * ) */ class DummyDtoNoInput diff --git a/tests/Fixtures/TestBundle/Entity/DummyDtoNoInput.php b/tests/Fixtures/TestBundle/Entity/DummyDtoNoInput.php index b29654ad92e..519e9f1f17a 100644 --- a/tests/Fixtures/TestBundle/Entity/DummyDtoNoInput.php +++ b/tests/Fixtures/TestBundle/Entity/DummyDtoNoInput.php @@ -14,6 +14,8 @@ namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; use ApiPlatform\Core\Annotation\ApiResource; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\CreateItemAction; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\DoubleBatAction; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Dto\OutputDto; use Doctrine\ORM\Mapping as ORM; @@ -28,7 +30,25 @@ * attributes={ * "input"=false, * "output"=OutputDto::class - * } + * }, + * collectionOperations={ + * "post"={ + * "method"="POST", + * "path"="/dummy_dto_no_inputs", + * "controller"=CreateItemAction::class, + * }, + * "get", + * }, + * itemOperations={ + * "get", + * "delete", + * "post_double_bat"={ + * "method"="POST", + * "path"="/dummy_dto_no_inputs/{id}/double_bat", + * "controller"=DoubleBatAction::class, + * "status"=200, + * }, + * }, * ) */ class DummyDtoNoInput diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index c23d8d963da..e898ca396b1 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -134,6 +134,12 @@ services: arguments: ['@doctrine'] tags: ['api_platform.filter'] + ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\CreateItemAction: + tags: ['controller.service_arguments'] + + ApiPlatform\Core\Tests\Fixtures\TestBundle\Controller\DummyDtoNoInput\DoubleBatAction: + tags: ['controller.service_arguments'] + app.config_dummy_resource.action: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Action\ConfigCustom' arguments: ['@api_platform.item_data_provider'] @@ -227,6 +233,12 @@ services: tags: - { name: 'api_platform.data_transformer' } + app.data_transformer.dummy_dto_no_input_to_output_dto: + class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\DataTransformer\DummyDtoNoInputToOutputDtoDataTransformer' + public: false + tags: + - { name: 'api_platform.data_transformer' } + app.data_transformer.recover_password_input: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\DataTransformer\RecoverPasswordInputDataTransformer' public: false From dd613057202ae5d2049b4075164044bfe1c3d89c Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Thu, 18 Apr 2019 18:35:01 +0200 Subject: [PATCH 2/2] [Swagger][OpenAPI] Add "id" path parameter for POST item operation --- .../Serializer/DocumentationNormalizer.php | 31 +++++++++---------- 1 file changed, 14 insertions(+), 17 deletions(-) diff --git a/src/Swagger/Serializer/DocumentationNormalizer.php b/src/Swagger/Serializer/DocumentationNormalizer.php index 16a4c6e6dc8..8de0623d905 100644 --- a/src/Swagger/Serializer/DocumentationNormalizer.php +++ b/src/Swagger/Serializer/DocumentationNormalizer.php @@ -390,13 +390,7 @@ private function updateGetOperation(bool $v3, \ArrayObject $pathOperation, array $pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Retrieves a %s resource.', $resourceShortName); - $parameter = [ - 'name' => 'id', - 'in' => 'path', - 'required' => true, - ]; - $v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string'; - $pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter]; + $pathOperation = $this->addItemOperationParameters($v3, $pathOperation); $successResponse = ['description' => sprintf('%s resource response', $resourceShortName)]; if ($responseDefinitionKey) { @@ -424,6 +418,11 @@ private function updatePostOperation(bool $v3, \ArrayObject $pathOperation, arra $pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Creates a %s resource.', $resourceShortName); + $userDefinedParameters = $pathOperation['parameters'] ?? null; + if (OperationType::ITEM === $operationType) { + $pathOperation = $this->addItemOperationParameters($v3, $pathOperation); + } + $responseDefinitionKey = false; $outputMetadata = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'output', ['class' => $resourceClass], true); if (null !== $outputClass = $outputMetadata['class'] ?? null) { @@ -460,12 +459,12 @@ private function updatePostOperation(bool $v3, \ArrayObject $pathOperation, arra 'description' => sprintf('The new %s resource', $resourceShortName), ]; } else { - $pathOperation['parameters'] ?? $pathOperation['parameters'] = [[ + $userDefinedParameters ?? $pathOperation['parameters'][] = [ 'name' => lcfirst($resourceShortName), 'in' => 'body', 'description' => sprintf('The new %s resource', $resourceShortName), 'schema' => ['$ref' => sprintf('#/definitions/%s', $requestDefinitionKey)], - ]]; + ]; } return $pathOperation; @@ -480,13 +479,7 @@ private function updatePutOperation(bool $v3, \ArrayObject $pathOperation, array $pathOperation['summary'] ?? $pathOperation['summary'] = sprintf('Replaces the %s resource.', $resourceShortName); - $parameter = [ - 'name' => 'id', - 'in' => 'path', - 'required' => true, - ]; - $v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string'; - $pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter]; + $pathOperation = $this->addItemOperationParameters($v3, $pathOperation); $responseDefinitionKey = false; $outputMetadata = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'output', ['class' => $resourceClass], true); @@ -540,13 +533,17 @@ private function updateDeleteOperation(bool $v3, \ArrayObject $pathOperation, st '404' => ['description' => 'Resource not found'], ]; + return $this->addItemOperationParameters($v3, $pathOperation); + } + + private function addItemOperationParameters(bool $v3, \ArrayObject $pathOperation): \ArrayObject + { $parameter = [ 'name' => 'id', 'in' => 'path', 'required' => true, ]; $v3 ? $parameter['schema'] = ['type' => 'string'] : $parameter['type'] = 'string'; - $pathOperation['parameters'] ?? $pathOperation['parameters'] = [$parameter]; return $pathOperation;