From 8de3f81611e19ea6b72067676460ca7dd0f7d9a6 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Thu, 26 Sep 2019 15:33:04 +0200 Subject: [PATCH] Revert "Merge pull request #3079 from soyuka/feature/empty-item-operations" This reverts commit c277aeb8c67bff72188aa65ce896bc3b1f08c468, reversing changes made to 1088a5d26b298d5b6130da24af35124e0b3cab00. --- features/main/operation.feature | 8 --- src/Action/NotFoundAction.php | 29 ----------- .../Symfony/Bundle/Resources/config/api.xml | 2 - src/JsonSchema/SchemaFactory.php | 2 +- .../OperationResourceMetadataFactory.php | 21 -------- .../ApiPlatformExtensionTest.php | 27 +++++----- .../Document/DisableItemOperation.php | 45 ----------------- .../Entity/DisableItemOperation.php | 49 ------------------- .../Command/JsonSchemaGenerateCommandTest.php | 2 +- .../OperationResourceMetadataFactoryTest.php | 32 ++++-------- 10 files changed, 24 insertions(+), 193 deletions(-) delete mode 100644 src/Action/NotFoundAction.php delete mode 100644 tests/Fixtures/TestBundle/Document/DisableItemOperation.php delete mode 100644 tests/Fixtures/TestBundle/Entity/DisableItemOperation.php diff --git a/features/main/operation.feature b/features/main/operation.feature index 772460dd16d..b81805498ea 100644 --- a/features/main/operation.feature +++ b/features/main/operation.feature @@ -55,11 +55,3 @@ 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 deleted file mode 100644 index 465e86e8ad6..00000000000 --- a/src/Action/NotFoundAction.php +++ /dev/null @@ -1,29 +0,0 @@ - - * - * 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 f34847662f7..2508cc8ff92 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -229,8 +229,6 @@ - - diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index 0d2846015e6..addfe1e5d87 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 (false === ($inputOrOutput['class'] ?? false)) { + if (null === ($inputOrOutput['class'] ?? null)) { // input or output disabled return null; } diff --git a/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php b/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php index ef2590edad1..eeb41baaacb 100644 --- a/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php +++ b/src/Metadata/Resource/Factory/OperationResourceMetadataFactory.php @@ -13,7 +13,6 @@ namespace ApiPlatform\Core\Metadata\Resource\Factory; -use ApiPlatform\Core\Action\NotFoundAction; use ApiPlatform\Core\Metadata\Resource\ResourceMetadata; /** @@ -82,13 +81,6 @@ 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' => []]); @@ -156,17 +148,4 @@ 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 11f23d41b3f..620ef4ce6f1 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -13,7 +13,6 @@ 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; @@ -844,7 +843,6 @@ 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', @@ -940,24 +938,23 @@ private function getPartialContainerBuilderProphecy() 'api_platform.property_accessor' => 'property_accessor', 'api_platform.property_info' => 'property_info', 'api_platform.serializer' => 'serializer', - CollectionDataProviderInterface::class => 'api_platform.collection_data_provider', - DataPersisterInterface::class => 'api_platform.data_persister', - GroupFilter::class => 'api_platform.serializer.group_filter', - IdentifiersExtractorInterface::class => 'api_platform.identifiers_extractor.cached', + 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', - 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', + 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', PropertyNameCollectionFactoryInterface::class => 'api_platform.metadata.property.name_collection_factory', PropertyMetadataFactoryInterface::class => 'api_platform.metadata.property.metadata_factory', ResourceClassResolverInterface::class => 'api_platform.resource_class_resolver', - 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', + 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', ]; foreach ($aliases as $alias => $service) { diff --git a/tests/Fixtures/TestBundle/Document/DisableItemOperation.php b/tests/Fixtures/TestBundle/Document/DisableItemOperation.php deleted file mode 100644 index 61d39d585f0..00000000000 --- a/tests/Fixtures/TestBundle/Document/DisableItemOperation.php +++ /dev/null @@ -1,45 +0,0 @@ - - * - * 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 deleted file mode 100644 index 385e6cd52d1..00000000000 --- a/tests/Fixtures/TestBundle/Entity/DisableItemOperation.php +++ /dev/null @@ -1,49 +0,0 @@ - - * - * 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/JsonSchema/Command/JsonSchemaGenerateCommandTest.php b/tests/JsonSchema/Command/JsonSchemaGenerateCommandTest.php index 86e234df469..f6e74c1bef4 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" options at the same time.', trim(preg_replace('/\s+/', ' ', $this->tester->getDisplay()))); + $this->assertStringStartsWith('[ERROR] You can only use one of "--itemOperation" and "--collectionOperation"', trim(str_replace(["\r", "\n"], '', $this->tester->getDisplay()))); } public function testExecuteWithJsonldFormatOption() diff --git a/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php b/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php index 4bdbbbdecb2..b990595792d 100644 --- a/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php +++ b/tests/Metadata/Resource/Factory/OperationResourceMetadataFactoryTest.php @@ -13,8 +13,6 @@ 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; @@ -45,39 +43,29 @@ 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, 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, ['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, ['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, array_merge(['untouched_custom' => ['route_name' => 'custom_route']], $this->getPlaceholderOperation()), [], 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]; // Collection operations - 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, [], [])]; + 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, [], [])]; } - private function getOperations(array $names, $operationType = OperationType::ITEM): array + private function getOperations(array $names): 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]]; - } }