From 142f7da735d4ddb792853938594083dafe7444aa Mon Sep 17 00:00:00 2001 From: soyuka Date: Sat, 21 Sep 2019 13:36:06 +0200 Subject: [PATCH 1/3] Ability to declare empty item operations --- features/main/operation.feature | 8 +++ src/Action/NotFoundAction.php | 29 +++++++++++ .../Symfony/Bundle/Resources/config/api.xml | 2 + .../OperationResourceMetadataFactory.php | 21 ++++++++ .../ApiPlatformExtensionTest.php | 27 +++++----- .../Document/DisableItemOperation.php | 45 +++++++++++++++++ .../Entity/DisableItemOperation.php | 49 +++++++++++++++++++ .../OperationResourceMetadataFactoryTest.php | 32 ++++++++---- 8 files changed, 191 insertions(+), 22 deletions(-) create mode 100644 src/Action/NotFoundAction.php create mode 100644 tests/Fixtures/TestBundle/Document/DisableItemOperation.php create mode 100644 tests/Fixtures/TestBundle/Entity/DisableItemOperation.php diff --git a/features/main/operation.feature b/features/main/operation.feature index b81805498ea..772460dd16d 100644 --- a/features/main/operation.feature +++ b/features/main/operation.feature @@ -55,3 +55,11 @@ Feature: Operation support } } """ + + Scenario: Get the collection of a resource that doesn't have a defined item operation + When I send a "GET" request to "/disable_item_operations" + Then the response status code should be 200 + + Scenario: Do not get a resource that doesn't have a defined item operation + When I send a "GET" request to "/disable_item_operations/1" + Then the response status code should be 404 diff --git a/src/Action/NotFoundAction.php b/src/Action/NotFoundAction.php new file mode 100644 index 00000000000..465e86e8ad6 --- /dev/null +++ b/src/Action/NotFoundAction.php @@ -0,0 +1,29 @@ + + * + * 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\Action; + +use Symfony\Component\HttpKernel\Exception\NotFoundHttpException; + +/** + * Not found action. + * + * @author Antoine Bluchet + */ +final class NotFoundAction +{ + public function __invoke() + { + throw new NotFoundHttpException(); + } +} diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index ae0a778da1b..5b07b360cf2 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -230,6 +230,8 @@ + + diff --git a/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php b/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php index eeb41baaacb..ef2590edad1 100644 --- a/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\Metadata\Resource\Factory; +use ApiPlatform\Core\Action\NotFoundAction; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; /** @@ -81,6 +82,13 @@ public function create(string $resourceClass): ResourceMetadata $resourceMetadata = $this->normalize(false, $resourceClass, $resourceMetadata, $itemOperations); } + if ($this->needsDefaultGetOperation($resourceMetadata)) { + $resourceMetadata = $resourceMetadata->withItemOperations(array_merge( + $resourceMetadata->getItemOperations(), + ['get' => ['method' => 'GET', 'read' => false, 'output' => ['class' => false], 'controller' => NotFoundAction::class]]) + ); + } + $graphql = $resourceMetadata->getGraphql(); if (null === $graphql) { $resourceMetadata = $resourceMetadata->withGraphql(['item_query' => [], 'collection_query' => [], 'delete' => [], 'update' => [], 'create' => []]); @@ -148,4 +156,17 @@ private function normalizeGraphQl(ResourceMetadata $resourceMetadata, array $ope return $resourceMetadata->withGraphql($operations); } + + private function needsDefaultGetOperation(ResourceMetadata $resourceMetadata): bool + { + $itemOperations = $resourceMetadata->getItemOperations(); + + foreach ($itemOperations as $itemOperation) { + if ('GET' === ($itemOperation['method'] ?? false)) { + return false; + } + } + + return true; + } } diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 1485971f1f4..a0d687a00ec 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -13,6 +13,7 @@ namespace ApiPlatform\Core\Tests\Bridge\Symfony\Bundle\DependencyInjection; +use ApiPlatform\Core\Action\NotFoundAction; use ApiPlatform\Core\Api\FilterInterface; use ApiPlatform\Core\Api\IdentifiersExtractorInterface; use ApiPlatform\Core\Api\IriConverterInterface; @@ -840,6 +841,7 @@ private function getPartialContainerBuilderProphecy() 'api_platform.action.documentation', 'api_platform.action.entrypoint', 'api_platform.action.exception', + 'api_platform.action.not_found', 'api_platform.action.placeholder', 'api_platform.cache.identifiers_extractor', 'api_platform.cache.metadata.property', @@ -935,23 +937,24 @@ private function getPartialContainerBuilderProphecy() 'api_platform.property_accessor' => 'property_accessor', 'api_platform.property_info' => 'property_info', 'api_platform.serializer' => 'serializer', - Pagination::class => 'api_platform.pagination', - IriConverterInterface::class => 'api_platform.iri_converter', - UrlGeneratorInterface::class => 'api_platform.router', - SerializerContextBuilderInterface::class => 'api_platform.serializer.context_builder', CollectionDataProviderInterface::class => 'api_platform.collection_data_provider', - ItemDataProviderInterface::class => 'api_platform.item_data_provider', - SubresourceDataProviderInterface::class => 'api_platform.subresource_data_provider', DataPersisterInterface::class => 'api_platform.data_persister', - ResourceNameCollectionFactoryInterface::class => 'api_platform.metadata.resource.name_collection_factory', - ResourceMetadataFactoryInterface::class => 'api_platform.metadata.resource.metadata_factory', + GroupFilter::class => 'api_platform.serializer.group_filter', + IdentifiersExtractorInterface::class => 'api_platform.identifiers_extractor.cached', + IriConverterInterface::class => 'api_platform.iri_converter', + ItemDataProviderInterface::class => 'api_platform.item_data_provider', + NotFoundAction::class => 'api_platform.action.not_found', + OperationAwareFormatsProviderInterface::class => 'api_platform.formats_provider', + Pagination::class => 'api_platform.pagination', + PropertyFilter::class => 'api_platform.serializer.property_filter', PropertyNameCollectionFactoryInterface::class => 'api_platform.metadata.property.name_collection_factory', PropertyMetadataFactoryInterface::class => 'api_platform.metadata.property.metadata_factory', ResourceClassResolverInterface::class => 'api_platform.resource_class_resolver', - PropertyFilter::class => 'api_platform.serializer.property_filter', - GroupFilter::class => 'api_platform.serializer.group_filter', - OperationAwareFormatsProviderInterface::class => 'api_platform.formats_provider', - IdentifiersExtractorInterface::class => 'api_platform.identifiers_extractor.cached', + ResourceNameCollectionFactoryInterface::class => 'api_platform.metadata.resource.name_collection_factory', + ResourceMetadataFactoryInterface::class => 'api_platform.metadata.resource.metadata_factory', + SerializerContextBuilderInterface::class => 'api_platform.serializer.context_builder', + SubresourceDataProviderInterface::class => 'api_platform.subresource_data_provider', + UrlGeneratorInterface::class => 'api_platform.router', ]; foreach ($aliases as $alias => $service) { diff --git a/tests/Fixtures/TestBundle/Document/DisableItemOperation.php b/tests/Fixtures/TestBundle/Document/DisableItemOperation.php new file mode 100644 index 00000000000..61d39d585f0 --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/DisableItemOperation.php @@ -0,0 +1,45 @@ + + * + * 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\Document; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; + +/** + * DisableItemOperation. + * + * @author Antoine Bluchet + * + * @ApiResource(itemOperations={}, collectionOperations={"get"}) + * @ODM\Document + */ +class DisableItemOperation +{ + /** + * @ODM\Id(strategy="INCREMENT", type="integer") + */ + private $id; + + /** + * @var string The dummy name + * + * @ODM\Field + */ + public $name; + + public function getId() + { + return $this->id; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/DisableItemOperation.php b/tests/Fixtures/TestBundle/Entity/DisableItemOperation.php new file mode 100644 index 00000000000..385e6cd52d1 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DisableItemOperation.php @@ -0,0 +1,49 @@ + + * + * 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\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * DisableItemOperation. + * + * @author Antoine Bluchet + * + * @ApiResource(itemOperations={}, collectionOperations={"get", "post"}) + * @ORM\Entity + */ +class DisableItemOperation +{ + /** + * @var int The id + * + * @ORM\Column(type="integer", nullable=true) + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + + /** + * @var string The dummy name + * + * @ORM\Column + */ + public $name; + + public function getId() + { + return $this->id; + } +} diff --git a/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php b/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php index b990595792d..4bdbbbdecb2 100644 --- a/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php +++ b/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php @@ -13,6 +13,8 @@ namespace ApiPlatform\Core\Tests\Metadata\Resource\Factory; +use ApiPlatform\Core\Action\NotFoundAction; +use ApiPlatform\Core\Api\OperationType; use ApiPlatform\Core\Metadata\Resource\Factory\OperationResourceMetadataFactory; use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; @@ -43,29 +45,39 @@ public function getMetadata(): iterable yield [new ResourceMetadata(null, null, null, null, [], null, [], []), new ResourceMetadata(null, null, null, $this->getOperations(['get', 'put', 'delete']), [], null, [], [])]; yield [new ResourceMetadata(null, null, null, null, [], null, [], []), new ResourceMetadata(null, null, null, $this->getOperations(['get', 'put', 'patch', 'delete']), [], null, [], []), $jsonapi]; yield [new ResourceMetadata(null, null, null, ['get'], [], null, [], []), new ResourceMetadata(null, null, null, $this->getOperations(['get']), [], null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], [], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), [], null, [], [])]; yield [new ResourceMetadata(null, null, null, ['put'], [], null, [], []), new ResourceMetadata(null, null, null, $this->getOperations(['put']), [], null, [], [])]; yield [new ResourceMetadata(null, null, null, ['delete'], [], null, [], []), new ResourceMetadata(null, null, null, $this->getOperations(['delete']), [], null, [], [])]; - yield [new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], []), new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], [])]; - yield [new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], []), new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], []), $jsonapi]; + yield [new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], []), new ResourceMetadata(null, null, null, array_merge(['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], $this->getPlaceholderOperation()), [], null, [], [])]; + yield [new ResourceMetadata(null, null, null, ['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], [], null, [], []), new ResourceMetadata(null, null, null, array_merge(['patch' => ['method' => 'PATCH', 'route_name' => 'patch']], $this->getPlaceholderOperation()), [], null, [], []), $jsonapi]; yield [new ResourceMetadata(null, null, null, ['untouched' => ['method' => 'GET']], [], null, [], []), new ResourceMetadata(null, null, null, ['untouched' => ['method' => 'GET']], [], null, [], []), $jsonapi]; - yield [new ResourceMetadata(null, null, null, ['untouched_custom' => ['route_name' => 'custom_route']], [], null, [], []), new ResourceMetadata(null, null, null, ['untouched_custom' => ['route_name' => 'custom_route']], [], null, [], []), $jsonapi]; + yield [new ResourceMetadata(null, null, null, ['untouched_custom' => ['route_name' => 'custom_route']], [], null, [], []), new ResourceMetadata(null, null, null, array_merge(['untouched_custom' => ['route_name' => 'custom_route']], $this->getPlaceholderOperation()), [], null, [], []), $jsonapi]; // Collection operations - yield [new ResourceMetadata(null, null, null, [], null, null, [], []), new ResourceMetadata(null, null, null, [], $this->getOperations(['get', 'post']), null, [], [])]; - yield [new ResourceMetadata(null, null, null, [], ['get'], null, [], []), new ResourceMetadata(null, null, null, [], $this->getOperations(['get']), null, [], [])]; - yield [new ResourceMetadata(null, null, null, [], ['post'], null, [], []), new ResourceMetadata(null, null, null, [], $this->getOperations(['post']), null, [], [])]; - yield [new ResourceMetadata(null, null, null, [], ['options' => ['method' => 'OPTIONS', 'route_name' => 'options']], null, [], []), new ResourceMetadata(null, null, null, [], ['options' => ['route_name' => 'options', 'method' => 'OPTIONS']], null, [], [])]; - yield [new ResourceMetadata(null, null, null, [], ['untouched' => ['method' => 'GET']], null, [], []), new ResourceMetadata(null, null, null, [], ['untouched' => ['method' => 'GET']], null, [], [])]; - yield [new ResourceMetadata(null, null, null, [], ['untouched_custom' => ['route_name' => 'custom_route']], null, [], []), new ResourceMetadata(null, null, null, [], ['untouched_custom' => ['route_name' => 'custom_route']], null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], null, null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), $this->getOperations(['get', 'post'], OperationType::COLLECTION), null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], ['get'], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), $this->getOperations(['get'], OperationType::COLLECTION), null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], ['post'], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), $this->getOperations(['post'], OperationType::COLLECTION), null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], ['options' => ['method' => 'OPTIONS', 'route_name' => 'options']], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), ['options' => ['route_name' => 'options', 'method' => 'OPTIONS']], null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], ['untouched' => ['method' => 'GET']], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), ['untouched' => ['method' => 'GET']], null, [], [])]; + yield [new ResourceMetadata(null, null, null, [], ['untouched_custom' => ['route_name' => 'custom_route']], null, [], []), new ResourceMetadata(null, null, null, $this->getPlaceholderOperation(), ['untouched_custom' => ['route_name' => 'custom_route']], null, [], [])]; } - private function getOperations(array $names): array + private function getOperations(array $names, $operationType = OperationType::ITEM): array { $operations = []; foreach ($names as $name) { $operations[$name] = ['method' => strtoupper($name)]; } + if (OperationType::ITEM === $operationType && !isset($operations['get'])) { + return array_merge($operations, $this->getPlaceholderOperation()); + } + return $operations; } + + private function getPlaceholderOperation(): array + { + return ['get' => ['method' => 'GET', 'read' => false, 'output' => ['class' => false], 'controller' => NotFoundAction::class]]; + } } From 15e996a617610949ef4f3fdb16e203980593df26 Mon Sep 17 00:00:00 2001 From: soyuka Date: Sat, 21 Sep 2019 13:37:29 +0200 Subject: [PATCH 2/3] JsonSchema: fix input metadata when disabled --- src/JsonSchema/SchemaFactory.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index addfe1e5d87..0d2846015e6 100644 --- a/src/JsonSchema/SchemaFactory.php +++ b/src/JsonSchema/SchemaFactory.php @@ -230,7 +230,7 @@ private function getMetadata(string $resourceClass, string $type = Schema::TYPE_ $inputOrOutput = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, $attribute, ['class' => $resourceClass], true); } - if (null === ($inputOrOutput['class'] ?? null)) { + if (false === ($inputOrOutput['class'] ?? false)) { // input or output disabled return null; } From 5aa6f20349b1481eb127699792b362ac3779c2b4 Mon Sep 17 00:00:00 2001 From: soyuka Date: Sat, 21 Sep 2019 13:37:55 +0200 Subject: [PATCH 3/3] JsonSchema: better command output sanitizing --- tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php b/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php index f6e74c1bef4..86e234df469 100644 --- a/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php +++ b/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php @@ -68,7 +68,7 @@ public function testExecuteWithTooManyOptions() { $this->tester->run(['command' => 'api:json-schema:generate', 'resource' => $this->entityClass, '--collectionOperation' => 'get', '--itemOperation' => 'get', '--type' => 'output']); - $this->assertStringStartsWith('[ERROR] You can only use one of "--itemOperation" and "--collectionOperation"', trim(str_replace(["\r", "\n"], '', $this->tester->getDisplay()))); + $this->assertStringStartsWith('[ERROR] You can only use one of "--itemOperation" and "--collectionOperation" options at the same time.', trim(preg_replace('/\s+/', ' ', $this->tester->getDisplay()))); } public function testExecuteWithJsonldFormatOption()