From 8de3f81611e19ea6b72067676460ca7dd0f7d9a6 Mon Sep 17 00:00:00 2001 From: Teoh Han Hui Date: Thu, 26 Sep 2019 15:33:04 +0200 Subject: [PATCH 1/8] 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]]; - } } From ae79ac8c2ea9f34e6e346742585919572c29ff78 Mon Sep 17 00:00:00 2001 From: Anthony GRASSIOT Date: Fri, 27 Sep 2019 10:40:00 +0200 Subject: [PATCH 2/8] Fix BC-break when using short-syntax notation for access_control --- src/Annotation/AttributesHydratorTrait.php | 11 +++++++++++ .../TestBundle/Document/LegacySecuredDummy.php | 3 ++- .../Fixtures/TestBundle/Entity/LegacySecuredDummy.php | 3 ++- 3 files changed, 15 insertions(+), 2 deletions(-) diff --git a/src/Annotation/AttributesHydratorTrait.php b/src/Annotation/AttributesHydratorTrait.php index 62ebee0046d..4d3d8c95ec8 100644 --- a/src/Annotation/AttributesHydratorTrait.php +++ b/src/Annotation/AttributesHydratorTrait.php @@ -49,6 +49,17 @@ private function hydrateAttributes(array $values): void unset($values['attributes']); } + if (array_key_exists('accessControl', $values)) { + $values['security'] = $values['accessControl']; + @trigger_error('Attribute "accessControl" is deprecated in annotation since API Platform 2.5, prefer using "security" attribute instead', E_USER_DEPRECATED); + unset($values['accessControl']); + } + if (array_key_exists('accessControlMessage', $values)) { + $values['securityMessage'] = $values['accessControlMessage']; + @trigger_error('Attribute "accessControlMessage" is deprecated in annotation since API Platform 2.5, prefer using "securityMessage" attribute instead', E_USER_DEPRECATED); + unset($values['accessControlMessage']); + } + foreach ($values as $key => $value) { $key = (string) $key; if (!property_exists($this, $key)) { diff --git a/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php b/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php index 38b0b3765ae..fc2d7e18cb5 100644 --- a/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php +++ b/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php @@ -23,7 +23,8 @@ * @author Vincent Chalamon * * @ApiResource( - * attributes={"access_control"="is_granted('ROLE_USER')"}, + * accessControl="is_granted('ROLE_USER')", + * accessControlMessage="Nope!", * collectionOperations={ * "get", * "post"={"access_control"="is_granted('ROLE_ADMIN')"} diff --git a/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php b/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php index 65bebaf2f9c..90e885e92fe 100644 --- a/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php +++ b/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php @@ -23,7 +23,8 @@ * @author Vincent Chalamon * * @ApiResource( - * attributes={"access_control"="is_granted('ROLE_USER')"}, + * accessControl="is_granted('ROLE_USER')", + * accessControlMessage="Nope!", * collectionOperations={ * "get", * "post"={"access_control"="is_granted('ROLE_ADMIN')"} From 912fca42f64f68f60d16b55e960a06e41b11fb69 Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 27 Sep 2019 10:41:18 +0200 Subject: [PATCH 3/8] Add the ability to declare empty item operations --- features/main/operation.feature | 8 +++ src/Action/NotFoundAction.php | 27 +++++++++ .../Symfony/Bundle/Resources/config/api.xml | 2 + .../ApiPlatformExtensionTest.php | 27 +++++---- .../Document/DisableItemOperation.php | 53 +++++++++++++++++ .../Entity/DisableItemOperation.php | 57 +++++++++++++++++++ .../Command/JsonSchemaGenerateCommandTest.php | 2 +- 7 files changed, 163 insertions(+), 13 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..8c70d1c2b99 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 have disabled item operation + When I send a "GET" request to "/disable_item_operations" + Then the response status code should be 200 + + Scenario: Get a 404 response for the disabled 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..8dd73fb7090 --- /dev/null +++ b/src/Action/NotFoundAction.php @@ -0,0 +1,27 @@ + + * + * 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; + +/** + * An action which always returns HTTP 404 Not Found. Useful for disabling an operation. + */ +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 2508cc8ff92..f34847662f7 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -229,6 +229,8 @@ + + diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 620ef4ce6f1..11f23d41b3f 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; @@ -843,6 +844,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', @@ -938,23 +940,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..6d84f2a8b7d --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/DisableItemOperation.php @@ -0,0 +1,53 @@ + + * + * 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\Action\NotFoundAction; +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; + +/** + * @ApiResource( + * collectionOperations={ + * "get", + * }, + * itemOperations={ + * "get"={ + * "controller"=NotFoundAction::class, + * "read"=false, + * "output"=false, + * }, + * }, + * ) + * @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..e5f85128843 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DisableItemOperation.php @@ -0,0 +1,57 @@ + + * + * 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\Action\NotFoundAction; +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ApiResource( + * collectionOperations={ + * "get", + * }, + * itemOperations={ + * "get"={ + * "controller"=NotFoundAction::class, + * "read"=false, + * "output"=false, + * }, + * }, + * ) + * @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 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() From 320316a2c2bd82529556fea8021fc4870cd64215 Mon Sep 17 00:00:00 2001 From: Anthony GRASSIOT Date: Fri, 27 Sep 2019 14:20:19 +0200 Subject: [PATCH 4/8] fix false deprecation notice --- src/Annotation/AttributesHydratorTrait.php | 4 ++-- src/Test/DoctrineOrmFilterTestCase.php | 3 +++ tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php | 3 +++ 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Annotation/AttributesHydratorTrait.php b/src/Annotation/AttributesHydratorTrait.php index 4d3d8c95ec8..41cdebc3562 100644 --- a/src/Annotation/AttributesHydratorTrait.php +++ b/src/Annotation/AttributesHydratorTrait.php @@ -49,12 +49,12 @@ private function hydrateAttributes(array $values): void unset($values['attributes']); } - if (array_key_exists('accessControl', $values)) { + if (\array_key_exists('accessControl', $values)) { $values['security'] = $values['accessControl']; @trigger_error('Attribute "accessControl" is deprecated in annotation since API Platform 2.5, prefer using "security" attribute instead', E_USER_DEPRECATED); unset($values['accessControl']); } - if (array_key_exists('accessControlMessage', $values)) { + if (\array_key_exists('accessControlMessage', $values)) { $values['securityMessage'] = $values['accessControlMessage']; @trigger_error('Attribute "accessControlMessage" is deprecated in annotation since API Platform 2.5, prefer using "securityMessage" attribute instead', E_USER_DEPRECATED); unset($values['accessControlMessage']); diff --git a/src/Test/DoctrineOrmFilterTestCase.php b/src/Test/DoctrineOrmFilterTestCase.php index c827b043f1d..6ee44da3b68 100644 --- a/src/Test/DoctrineOrmFilterTestCase.php +++ b/src/Test/DoctrineOrmFilterTestCase.php @@ -55,6 +55,9 @@ abstract class DoctrineOrmFilterTestCase extends KernelTestCase */ protected $filterClass; + /** + * @group legacy + */ protected function setUp(): void { self::bootKernel(); diff --git a/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php b/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php index e290d87768c..10a17c57476 100644 --- a/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php +++ b/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php @@ -41,6 +41,9 @@ protected function setUp(): void $this->tester = new ApplicationTester($application); } + /** + * @group legacy + */ public function testExecuteWithAliasVersion3() { $this->tester->run(['command' => 'api:swagger:export', '--spec-version' => 3]); From 3485028e016122a0fbefa28d7cb74483613181a6 Mon Sep 17 00:00:00 2001 From: soyuka Date: Fri, 27 Sep 2019 15:29:11 +0200 Subject: [PATCH 5/8] security: Fix tests and legacy check --- src/Test/DoctrineOrmFilterTestCase.php | 3 --- tests/Annotation/ApiResourceTest.php | 13 +++++++++++++ .../Symfony/Bundle/Command/SwaggerCommandTest.php | 3 --- .../TestBundle/Document/LegacySecuredDummy.php | 3 +-- .../TestBundle/Entity/LegacySecuredDummy.php | 3 +-- 5 files changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Test/DoctrineOrmFilterTestCase.php b/src/Test/DoctrineOrmFilterTestCase.php index 6ee44da3b68..c827b043f1d 100644 --- a/src/Test/DoctrineOrmFilterTestCase.php +++ b/src/Test/DoctrineOrmFilterTestCase.php @@ -55,9 +55,6 @@ abstract class DoctrineOrmFilterTestCase extends KernelTestCase */ protected $filterClass; - /** - * @group legacy - */ protected function setUp(): void { self::bootKernel(); diff --git a/tests/Annotation/ApiResourceTest.php b/tests/Annotation/ApiResourceTest.php index 6698749918e..1fdc1fefe2c 100644 --- a/tests/Annotation/ApiResourceTest.php +++ b/tests/Annotation/ApiResourceTest.php @@ -143,4 +143,17 @@ public function testConstructWithInvalidAttribute() 'invalidAttribute' => 'exception', ]); } + + /** + * @group legacy + * @expectedDeprecation Attribute "accessControl" is deprecated in annotation since API Platform 2.5, prefer using "security" attribute instead + * @expectedDeprecation Attribute "accessControlMessage" is deprecated in annotation since API Platform 2.5, prefer using "securityMessage" attribute instead + */ + public function testWithDeprecatedAttributes() + { + new ApiResource([ + 'accessControl' => "is_granted('ROLE_USER')", + 'accessControlMessage' => 'Nope!', + ]); + } } diff --git a/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php b/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php index 10a17c57476..e290d87768c 100644 --- a/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php +++ b/tests/Bridge/Symfony/Bundle/Command/SwaggerCommandTest.php @@ -41,9 +41,6 @@ protected function setUp(): void $this->tester = new ApplicationTester($application); } - /** - * @group legacy - */ public function testExecuteWithAliasVersion3() { $this->tester->run(['command' => 'api:swagger:export', '--spec-version' => 3]); diff --git a/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php b/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php index fc2d7e18cb5..38b0b3765ae 100644 --- a/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php +++ b/tests/Fixtures/TestBundle/Document/LegacySecuredDummy.php @@ -23,8 +23,7 @@ * @author Vincent Chalamon * * @ApiResource( - * accessControl="is_granted('ROLE_USER')", - * accessControlMessage="Nope!", + * attributes={"access_control"="is_granted('ROLE_USER')"}, * collectionOperations={ * "get", * "post"={"access_control"="is_granted('ROLE_ADMIN')"} diff --git a/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php b/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php index 90e885e92fe..65bebaf2f9c 100644 --- a/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php +++ b/tests/Fixtures/TestBundle/Entity/LegacySecuredDummy.php @@ -23,8 +23,7 @@ * @author Vincent Chalamon * * @ApiResource( - * accessControl="is_granted('ROLE_USER')", - * accessControlMessage="Nope!", + * attributes={"access_control"="is_granted('ROLE_USER')"}, * collectionOperations={ * "get", * "post"={"access_control"="is_granted('ROLE_ADMIN')"} From d1e5080cbbf5fbae408e6a4d108b2b19179284ac Mon Sep 17 00:00:00 2001 From: Mahmood Bazdar Date: Mon, 30 Sep 2019 13:22:47 +0330 Subject: [PATCH 6/8] [GraphQL] Adding serialization group difference condition for item_query and collection_query types (#3083) * Adding serialization group difference condition for item_query and collection_query * Fixes * Changelog --- CHANGELOG.md | 4 + features/bootstrap/DoctrineContext.php | 25 ++++++ features/graphql/collection.feature | 26 +++++- features/graphql/input_output.feature | 2 +- features/graphql/introspection.feature | 67 +++++++++++--- features/graphql/mutation.feature | 4 +- features/graphql/query.feature | 19 +++- features/graphql/schema.feature | 58 ++++-------- src/GraphQl/Type/TypeBuilder.php | 15 ++-- ...ummyDifferentGraphQlSerializationGroup.php | 83 +++++++++++++++++ ...ummyDifferentGraphQlSerializationGroup.php | 89 +++++++++++++++++++ tests/GraphQl/Type/TypeBuilderTest.php | 66 +++++++++++--- 12 files changed, 383 insertions(+), 75 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Document/DummyDifferentGraphQlSerializationGroup.php create mode 100644 tests/Fixtures/TestBundle/Entity/DummyDifferentGraphQlSerializationGroup.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 347baa511d6..83c6c4d9959 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 2.5.0 beta 3 + +* GraphQL: Use different types (`MyTypeItem` and `MyTypeCollection`) only if serialization groups are different for `item_query` and `collection_query` (#3083) + ## 2.5.0 beta 2 * Allow to not declare GET item operation diff --git a/features/bootstrap/DoctrineContext.php b/features/bootstrap/DoctrineContext.php index 029f29c53f9..3e519762e76 100644 --- a/features/bootstrap/DoctrineContext.php +++ b/features/bootstrap/DoctrineContext.php @@ -31,6 +31,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyCustomMutation as DummyCustomMutationDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyCustomQuery as DummyCustomQueryDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDate as DummyDateDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDifferentGraphQlSerializationGroup as DummyDifferentGraphQlSerializationGroupDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoCustom as DummyDtoCustomDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoInput as DummyDtoNoInputDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyDtoNoOutput as DummyDtoNoOutputDocument; @@ -85,6 +86,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCustomMutation; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCustomQuery; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDate; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDifferentGraphQlSerializationGroup; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoCustom; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoInput; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyDtoNoOutput; @@ -1224,6 +1226,21 @@ public function thereAreDummyImmutableDateObjectsWithDummyDate(int $nb) $this->manager->flush(); } + /** + * @Given there are :nb dummy with different GraphQL serialization groups objects + */ + public function thereAreDummyWithDifferentGraphQlSerializationGroupsObjects(int $nb) + { + for ($i = 1; $i <= $nb; ++$i) { + $dummyDifferentGraphQlSerializationGroup = $this->buildDummyDifferentGraphQlSerializationGroup(); + $dummyDifferentGraphQlSerializationGroup->setName('Name #'.$i); + $dummyDifferentGraphQlSerializationGroup->setTitle('Title #'.$i); + $this->manager->persist($dummyDifferentGraphQlSerializationGroup); + } + + $this->manager->flush(); + } + /** * @Given there is a ramsey identified resource with uuid :uuid */ @@ -1583,6 +1600,14 @@ private function buildDummyDate() return $this->isOrm() ? new DummyDate() : new DummyDateDocument(); } + /** + * @return DummyDifferentGraphQlSerializationGroup|DummyDifferentGraphQlSerializationGroupDocument + */ + private function buildDummyDifferentGraphQlSerializationGroup() + { + return $this->isOrm() ? new DummyDifferentGraphQlSerializationGroup() : new DummyDifferentGraphQlSerializationGroupDocument(); + } + /** * @return DummyDtoNoInput|DummyDtoNoInputDocument */ diff --git a/features/graphql/collection.feature b/features/graphql/collection.feature index 7d376379e70..eb581455912 100644 --- a/features/graphql/collection.feature +++ b/features/graphql/collection.feature @@ -9,7 +9,7 @@ Feature: GraphQL collection support ...dummyFields } } - fragment dummyFields on DummyCollectionConnection { + fragment dummyFields on DummyConnection { edges { node { id @@ -656,3 +656,27 @@ Feature: GraphQL collection support And the response should be in JSON And the header "Content-Type" should be equal to "application/json" And the JSON node "data.dummies.edges[1].node.name_converted" should be equal to "Converted 2" + + @createSchema + Scenario: Retrieve a collection with different serialization groups for item_query and collection_query + Given there are 3 dummy with different GraphQL serialization groups objects + When I send the following GraphQL request: + """ + { + dummyDifferentGraphQlSerializationGroups { + edges { + node { + name + } + } + } + } + """ + Then the response status code should be 200 + And the response should be in JSON + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[0].node.name" should exist + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[1].node.name" should exist + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[2].node.name" should exist + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[0].node.title" should not exist + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[1].node.title" should not exist + And the JSON node "data.dummyDifferentGraphQlSerializationGroups.edges[2].node.title" should not exist diff --git a/features/graphql/input_output.feature b/features/graphql/input_output.feature index 9030cfb17f2..ee616ffc5fc 100644 --- a/features/graphql/input_output.feature +++ b/features/graphql/input_output.feature @@ -107,7 +107,7 @@ Feature: GraphQL DTO input and output { "errors": [ { - "message": "Cannot query field \"id\" on type \"DummyDtoNoOutputItem\".", + "message": "Cannot query field \"id\" on type \"DummyDtoNoOutput\".", "extensions": { "category": "graphql" }, diff --git a/features/graphql/introspection.feature b/features/graphql/introspection.feature index 35e0b883fd6..0c093266985 100644 --- a/features/graphql/introspection.feature +++ b/features/graphql/introspection.feature @@ -21,7 +21,7 @@ Feature: GraphQL introspection support When I send the following GraphQL request: """ { - type1: __type(name: "DummyProductItem") { + type1: __type(name: "DummyProduct") { description, fields { name @@ -35,7 +35,7 @@ Feature: GraphQL introspection support } } } - type2: __type(name: "DummyAggregateOfferItemConnection") { + type2: __type(name: "DummyAggregateOfferConnection") { description, fields { name @@ -49,7 +49,7 @@ Feature: GraphQL introspection support } } } - type3: __type(name: "DummyAggregateOfferItemEdge") { + type3: __type(name: "DummyAggregateOfferEdge") { description, fields { name @@ -69,12 +69,53 @@ Feature: GraphQL introspection support And the response should be in JSON And the header "Content-Type" should be equal to "application/json" And the JSON node "data.type1.description" should be equal to "Dummy Product." - And the JSON node "data.type1.fields[1].type.name" should be equal to "DummyAggregateOfferItemConnection" + And the JSON node "data.type1.fields[1].type.name" should be equal to "DummyAggregateOfferConnection" And the JSON node "data.type2.fields[0].name" should be equal to "edges" - And the JSON node "data.type2.fields[0].type.ofType.name" should be equal to "DummyAggregateOfferItemEdge" + And the JSON node "data.type2.fields[0].type.ofType.name" should be equal to "DummyAggregateOfferEdge" And the JSON node "data.type3.fields[0].name" should be equal to "node" And the JSON node "data.type3.fields[1].name" should be equal to "cursor" - And the JSON node "data.type3.fields[0].type.name" should be equal to "DummyAggregateOfferItem" + And the JSON node "data.type3.fields[0].type.name" should be equal to "DummyAggregateOffer" + + Scenario: Introspect types with different serialization groups for item_query and collection_query + When I send the following GraphQL request: + """ + { + type1: __type(name: "DummyDifferentGraphQlSerializationGroupCollection") { + description, + fields { + name + type { + name + kind + ofType { + name + kind + } + } + } + } + type2: __type(name: "DummyDifferentGraphQlSerializationGroupItem") { + description, + fields { + name + type { + name + kind + ofType { + name + kind + } + } + } + } + } + """ + 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 "data.type1.description" should be equal to "Dummy with different serialization groups for item_query and collection_query." + And the JSON node "data.type1.fields[3].name" should not exist + And the JSON node "data.type2.fields[3].name" should be equal to "title" Scenario: Introspect deprecated queries When I send the following GraphQL request: @@ -121,7 +162,7 @@ Feature: GraphQL introspection support When I send the following GraphQL request: """ { - __type(name: "DeprecatedResourceItem") { + __type(name: "DeprecatedResource") { fields(includeDeprecated: true) { name isDeprecated @@ -224,7 +265,7 @@ Feature: GraphQL introspection support When I send the following GraphQL request: """ { - __type(name: "DummyItem") { + __type(name: "Dummy") { description, fields { name @@ -250,7 +291,7 @@ Feature: GraphQL introspection support When I send the following GraphQL request: """ { - typeQuery: __type(name: "DummyGroupItem") { + typeQuery: __type(name: "DummyGroup") { description, fields { name @@ -390,7 +431,7 @@ Feature: GraphQL introspection support When I send the following GraphQL request: """ { - dummyItem: dummy(id: "/dummies/3") { + dummy: dummy(id: "/dummies/3") { name relatedDummy { id @@ -403,6 +444,6 @@ Feature: GraphQL introspection support 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 "data.dummyItem.name" should be equal to "Dummy #3" - And the JSON node "data.dummyItem.relatedDummy.name" should be equal to "RelatedDummy #3" - And the JSON node "data.dummyItem.relatedDummy.__typename" should be equal to "RelatedDummyItem" + And the JSON node "data.dummy.name" should be equal to "Dummy #3" + And the JSON node "data.dummy.relatedDummy.name" should be equal to "RelatedDummy #3" + And the JSON node "data.dummy.relatedDummy.__typename" should be equal to "RelatedDummy" diff --git a/features/graphql/mutation.feature b/features/graphql/mutation.feature index 308a24e8343..093a19015a9 100644 --- a/features/graphql/mutation.feature +++ b/features/graphql/mutation.feature @@ -61,7 +61,7 @@ Feature: GraphQL mutation support And the header "Content-Type" should be equal to "application/json" And the JSON node "data.createFoo.foo.id" should be equal to "/foos/1" And the JSON node "data.createFoo.foo._id" should be equal to 1 - And the JSON node "data.createFoo.foo.__typename" should be equal to "FooItem" + And the JSON node "data.createFoo.foo.__typename" should be equal to "Foo" And the JSON node "data.createFoo.foo.name" should be equal to "A new one" And the JSON node "data.createFoo.foo.bar" should be equal to "new" And the JSON node "data.createFoo.clientMutationId" should be equal to "myId" @@ -113,7 +113,7 @@ Feature: GraphQL mutation support And the JSON node "data.createDummy.dummy.name" should be equal to "A dummy" And the JSON node "data.createDummy.dummy.foo" should have 0 elements And the JSON node "data.createDummy.dummy.relatedDummy.name" should be equal to "RelatedDummy #1" - And the JSON node "data.createDummy.dummy.relatedDummy.__typename" should be equal to "RelatedDummyItem" + And the JSON node "data.createDummy.dummy.relatedDummy.__typename" should be equal to "RelatedDummy" And the JSON node "data.createDummy.dummy.name_converted" should be equal to "Converted" And the JSON node "data.createDummy.clientMutationId" should be equal to "myId" diff --git a/features/graphql/query.feature b/features/graphql/query.feature index a00c0826516..8fd4058ca3e 100644 --- a/features/graphql/query.feature +++ b/features/graphql/query.feature @@ -25,7 +25,7 @@ Feature: GraphQL query support { node(id: "/dummies/1") { id - ... on DummyItem { + ... on Dummy { name } } @@ -395,3 +395,20 @@ Feature: GraphQL query support } } """ + + @createSchema + Scenario: Retrieve an item with different serialization groups for item_query and collection_query + Given there are 1 dummy with different GraphQL serialization groups objects + When I send the following GraphQL request: + """ + { + dummyDifferentGraphQlSerializationGroup(id: "/dummy_different_graph_ql_serialization_groups/1") { + name + title + } + } + """ + Then the response status code should be 200 + And the header "Content-Type" should be equal to "application/json" + And the JSON node "data.dummyDifferentGraphQlSerializationGroup.name" should be equal to "Name #1" + And the JSON node "data.dummyDifferentGraphQlSerializationGroup.title" should be equal to "Title #1" diff --git a/features/graphql/schema.feature b/features/graphql/schema.feature index ccde09a41e0..75f63b9a59a 100644 --- a/features/graphql/schema.feature +++ b/features/graphql/schema.feature @@ -6,7 +6,7 @@ Feature: GraphQL schema-related features Then the command output should contain: """ ###Dummy Friend.### - type DummyFriendCollection implements Node { + type DummyFriend implements Node { id: ID! ###The id### @@ -16,37 +16,26 @@ Feature: GraphQL schema-related features name: String! } - ###Connection for DummyFriendCollection.### - type DummyFriendCollectionConnection { - edges: [DummyFriendCollectionEdge] - pageInfo: DummyFriendCollectionPageInfo! + ###Connection for DummyFriend.### + type DummyFriendConnection { + edges: [DummyFriendEdge] + pageInfo: DummyFriendPageInfo! totalCount: Int! } - ###Edge of DummyFriendCollection.### - type DummyFriendCollectionEdge { - node: DummyFriendCollection + ###Edge of DummyFriend.### + type DummyFriendEdge { + node: DummyFriend cursor: String! } ###Information about the current page.### - type DummyFriendCollectionPageInfo { + type DummyFriendPageInfo { endCursor: String startCursor: String hasNextPage: Boolean! hasPreviousPage: Boolean! } - - ###Dummy Friend.### - type DummyFriendItem implements Node { - id: ID! - - ###The id### - _id: Int! - - ###The dummy name### - name: String! - } """ Scenario: Export the GraphQL schema in SDL with comment descriptions @@ -55,7 +44,7 @@ Feature: GraphQL schema-related features Then the command output should contain: """ # Dummy Friend. - type DummyFriendCollection implements Node { + type DummyFriend implements Node { id: ID! # The id @@ -65,35 +54,24 @@ Feature: GraphQL schema-related features name: String! } - # Connection for DummyFriendCollection. - type DummyFriendCollectionConnection { - edges: [DummyFriendCollectionEdge] - pageInfo: DummyFriendCollectionPageInfo! + # Connection for DummyFriend. + type DummyFriendConnection { + edges: [DummyFriendEdge] + pageInfo: DummyFriendPageInfo! totalCount: Int! } - # Edge of DummyFriendCollection. - type DummyFriendCollectionEdge { - node: DummyFriendCollection + # Edge of DummyFriend. + type DummyFriendEdge { + node: DummyFriend cursor: String! } # Information about the current page. - type DummyFriendCollectionPageInfo { + type DummyFriendPageInfo { endCursor: String startCursor: String hasNextPage: Boolean! hasPreviousPage: Boolean! } - - # Dummy Friend. - type DummyFriendItem implements Node { - id: ID! - - # The id - _id: Int! - - # The dummy name - name: String! - } """ diff --git a/src/GraphQl/Type/TypeBuilder.php b/src/GraphQl/Type/TypeBuilder.php index 4c1312123f6..b557ce76b25 100644 --- a/src/GraphQl/Type/TypeBuilder.php +++ b/src/GraphQl/Type/TypeBuilder.php @@ -61,11 +61,14 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadata $ } $shortName .= 'Payload'; } - if ('item_query' === $queryName) { - $shortName .= 'Item'; - } - if ('collection_query' === $queryName) { - $shortName .= 'Collection'; + if (('item_query' === $queryName || 'collection_query' === $queryName) + && $resourceMetadata->getGraphqlAttribute('item_query', 'normalization_context', [], true) !== $resourceMetadata->getGraphqlAttribute('collection_query', 'normalization_context', [], true)) { + if ('item_query' === $queryName) { + $shortName .= 'Item'; + } + if ('collection_query' === $queryName) { + $shortName .= 'Collection'; + } } if ($wrapped && null !== $mutationName) { $shortName .= 'Data'; @@ -158,7 +161,7 @@ public function getNodeInterface(): InterfaceType return null; } - $shortName = (new \ReflectionClass($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY]))->getShortName().'Item'; + $shortName = (new \ReflectionClass($value[ItemNormalizer::ITEM_RESOURCE_CLASS_KEY]))->getShortName(); return $this->typesContainer->has($shortName) ? $this->typesContainer->get($shortName) : null; }, diff --git a/tests/Fixtures/TestBundle/Document/DummyDifferentGraphQlSerializationGroup.php b/tests/Fixtures/TestBundle/Document/DummyDifferentGraphQlSerializationGroup.php new file mode 100644 index 00000000000..7d9cdc512f0 --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/DummyDifferentGraphQlSerializationGroup.php @@ -0,0 +1,83 @@ + + * + * 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; +use Symfony\Component\Serializer\Annotation\Groups; + +/** + * Dummy with different serialization groups for item_query and collection_query. + * + * @author Mahmood Bazdar + * + * @ApiResource( + * graphql={ + * "item_query"={"normalization_context"={"groups"={"item_query"}}}, + * "collection_query"={"normalization_context"={"groups"={"collection_query"}}} + * } + * ) + * @ODM\Document + */ +class DummyDifferentGraphQlSerializationGroup +{ + /** + * @var int The id + * + * @ODM\Id(strategy="INCREMENT", type="integer", nullable=true) + * @Groups({"item_query", "collection_query"}) + */ + private $id; + + /** + * @var string The dummy name + * + * @ODM\Field(type="string") + * @Groups({"item_query", "collection_query"}) + */ + private $name; + + /** + * @var string The dummy title + * + * @ODM\Field(nullable=true) + * @Groups({"item_query"}) + */ + private $title; + + public function getId(): int + { + return $this->id; + } + + public function getName(): string + { + return $this->name; + } + + public function setName(string $name): void + { + $this->name = $name; + } + + public function getTitle(): string + { + return $this->title; + } + + public function setTitle(string $title): void + { + $this->title = $title; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/DummyDifferentGraphQlSerializationGroup.php b/tests/Fixtures/TestBundle/Entity/DummyDifferentGraphQlSerializationGroup.php new file mode 100644 index 00000000000..d54b21b3cf0 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DummyDifferentGraphQlSerializationGroup.php @@ -0,0 +1,89 @@ + + * + * 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; +use Symfony\Component\Serializer\Annotation\Groups; + +/** + * Dummy with different serialization groups for item_query and collection_query. + * + * @author Mahmood Bazdar + * + * @ApiResource( + * graphql={ + * "item_query"={"normalization_context"={"groups"={"item_query"}}}, + * "collection_query"={"normalization_context"={"groups"={"collection_query"}}} + * } + * ) + * @ORM\Entity + */ +class DummyDifferentGraphQlSerializationGroup +{ + /** + * @var int The id + * + * @ORM\Column(type="integer", nullable=true) + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + * @Groups({"item_query", "collection_query"}) + */ + private $id; + + /** + * @var string The dummy name + * + * @ORM\Column + * @Groups({"item_query", "collection_query"}) + */ + private $name; + + /** + * @var string The dummy title + * + * @ORM\Column(nullable=true) + * @Groups({"item_query"}) + */ + private $title; + + public function getId(): int + { + return $this->id; + } + + public function getName(): string + { + return $this->name; + } + + public function setName(string $name): self + { + $this->name = $name; + + return $this; + } + + public function getTitle(): string + { + return $this->title; + } + + public function setTitle(string $title): self + { + $this->title = $title; + + return $this; + } +} diff --git a/tests/GraphQl/Type/TypeBuilderTest.php b/tests/GraphQl/Type/TypeBuilderTest.php index 1239be66a36..557686991b8 100644 --- a/tests/GraphQl/Type/TypeBuilderTest.php +++ b/tests/GraphQl/Type/TypeBuilderTest.php @@ -64,14 +64,14 @@ protected function setUp(): void public function testGetResourceObjectType(): void { $resourceMetadata = new ResourceMetadata('shortName', 'description'); - $this->typesContainerProphecy->has('shortNameItem')->shouldBeCalled()->willReturn(false); - $this->typesContainerProphecy->set('shortNameItem', Argument::type(ObjectType::class))->shouldBeCalled(); + $this->typesContainerProphecy->has('shortName')->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->set('shortName', Argument::type(ObjectType::class))->shouldBeCalled(); $this->typesContainerProphecy->has('Node')->shouldBeCalled()->willReturn(false); $this->typesContainerProphecy->set('Node', Argument::type(InterfaceType::class))->shouldBeCalled(); /** @var ObjectType $resourceObjectType */ $resourceObjectType = $this->typeBuilder->getResourceObjectType('resourceClass', $resourceMetadata, false, 'item_query', null); - $this->assertSame('shortNameItem', $resourceObjectType->name); + $this->assertSame('shortName', $resourceObjectType->name); $this->assertSame('description', $resourceObjectType->description); $this->assertSame($this->defaultFieldResolver, $resourceObjectType->resolveFieldFn); $this->assertArrayHasKey('interfaces', $resourceObjectType->config); @@ -87,14 +87,14 @@ public function testGetResourceObjectTypeOutputClass(): void { $resourceMetadata = (new ResourceMetadata('shortName', 'description')) ->withGraphql(['item_query' => ['output' => ['class' => 'outputClass']]]); - $this->typesContainerProphecy->has('shortNameItem')->shouldBeCalled()->willReturn(false); - $this->typesContainerProphecy->set('shortNameItem', Argument::type(ObjectType::class))->shouldBeCalled(); + $this->typesContainerProphecy->has('shortName')->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->set('shortName', Argument::type(ObjectType::class))->shouldBeCalled(); $this->typesContainerProphecy->has('Node')->shouldBeCalled()->willReturn(false); $this->typesContainerProphecy->set('Node', Argument::type(InterfaceType::class))->shouldBeCalled(); /** @var ObjectType $resourceObjectType */ $resourceObjectType = $this->typeBuilder->getResourceObjectType('resourceClass', $resourceMetadata, false, 'item_query', null); - $this->assertSame('shortNameItem', $resourceObjectType->name); + $this->assertSame('shortName', $resourceObjectType->name); $this->assertSame('description', $resourceObjectType->description); $this->assertSame($this->defaultFieldResolver, $resourceObjectType->resolveFieldFn); $this->assertArrayHasKey('interfaces', $resourceObjectType->config); @@ -106,6 +106,50 @@ public function testGetResourceObjectTypeOutputClass(): void $resourceObjectType->config['fields'](); } + /** + * @dataProvider resourceObjectTypeQuerySerializationGroupsProvider + */ + public function testGetResourceObjectTypeQuerySerializationGroups(string $itemSerializationGroup, string $collectionSerializationGroup, string $shortName, string $queryName) + { + $resourceMetadata = (new ResourceMetadata('shortName', 'description')) + ->withGraphql([ + 'item_query' => ['normalization_context' => ['groups' => [$itemSerializationGroup]]], + 'collection_query' => ['normalization_context' => ['groups' => [$collectionSerializationGroup]]], + ]); + $this->typesContainerProphecy->has($shortName)->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->set($shortName, Argument::type(ObjectType::class))->shouldBeCalled(); + $this->typesContainerProphecy->has('Node')->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->set('Node', Argument::type(InterfaceType::class))->shouldBeCalled(); + + /** @var ObjectType $resourceObjectType */ + $resourceObjectType = $this->typeBuilder->getResourceObjectType('resourceClass', $resourceMetadata, false, $queryName, null); + $this->assertSame($shortName, $resourceObjectType->name); + } + + public function resourceObjectTypeQuerySerializationGroupsProvider(): array + { + return [ + 'same serialization groups for item_query and collection_query' => [ + 'group', + 'group', + 'shortName', + 'item_query', + ], + 'different serialization groups for item_query and collection_query when using item_query' => [ + 'item_group', + 'collection_group', + 'shortNameItem', + 'item_query', + ], + 'different serialization groups for item_query and collection_query when using collection_query' => [ + 'item_group', + 'collection_group', + 'shortNameCollection', + 'collection_query', + ], + ]; + } + public function testGetResourceObjectTypeInput(): void { $resourceMetadata = new ResourceMetadata('shortName', 'description'); @@ -175,8 +219,8 @@ public function testGetResourceObjectTypeMutation(): void $this->assertArrayHasKey('fields', $resourceObjectType->config); // Recursive call (not using wrapped type) - $this->typesContainerProphecy->has('shortNameItem')->shouldBeCalled()->willReturn(false); - $this->typesContainerProphecy->set('shortNameItem', Argument::type(ObjectType::class))->shouldBeCalled(); + $this->typesContainerProphecy->has('shortName')->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->set('shortName', Argument::type(ObjectType::class))->shouldBeCalled(); $fieldsType = $resourceObjectType->config['fields'](); $this->assertArrayHasKey('shortName', $fieldsType); @@ -262,11 +306,11 @@ public function testGetNodeInterface(): void $this->assertNull($nodeInterface->resolveType([], [], $this->prophesize(ResolveInfo::class)->reveal())); - $this->typesContainerProphecy->has('DummyItem')->shouldBeCalled()->willReturn(false); + $this->typesContainerProphecy->has('Dummy')->shouldBeCalled()->willReturn(false); $this->assertNull($nodeInterface->resolveType([ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => Dummy::class], [], $this->prophesize(ResolveInfo::class)->reveal())); - $this->typesContainerProphecy->has('DummyItem')->shouldBeCalled()->willReturn(true); - $this->typesContainerProphecy->get('DummyItem')->shouldBeCalled()->willReturn(GraphQLType::string()); + $this->typesContainerProphecy->has('Dummy')->shouldBeCalled()->willReturn(true); + $this->typesContainerProphecy->get('Dummy')->shouldBeCalled()->willReturn(GraphQLType::string()); /** @var GraphQLType $resolvedType */ $resolvedType = $nodeInterface->resolveType([ItemNormalizer::ITEM_RESOURCE_CLASS_KEY => Dummy::class], [], $this->prophesize(ResolveInfo::class)->reveal()); $this->assertSame(GraphQLType::string(), $resolvedType); From 3306949840dec58b10477f15895ffdc8f9827b56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 30 Sep 2019 13:53:51 +0200 Subject: [PATCH 7/8] [JSON Schema] Fix command --- src/JsonSchema/SchemaFactory.php | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/JsonSchema/SchemaFactory.php b/src/JsonSchema/SchemaFactory.php index 427738cf77b..1c5e300e87a 100644 --- a/src/JsonSchema/SchemaFactory.php +++ b/src/JsonSchema/SchemaFactory.php @@ -73,7 +73,11 @@ public function buildSchema(string $resourceClass, string $format = 'json', stri $version = $schema->getVersion(); $definitionName = $this->buildDefinitionName($resourceClass, $format, $type, $operationType, $operationName, $serializerContext); - $method = (null !== $operationType && null !== $operationName) ? $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'method') : 'GET'; + if (null === $operationType || null === $operationName) { + $method = Schema::TYPE_INPUT === $type ? 'POST' : 'GET'; + } else { + $method = $resourceMetadata->getTypedOperationAttribute($operationType, $operationName, 'method'); + } if (Schema::TYPE_OUTPUT !== $type && !\in_array($method, ['POST', 'PATCH', 'PUT'], true)) { return $schema; From 97d765b19a19c232bc0224adee552e85e20174ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?K=C3=A9vin=20Dunglas?= Date: Mon, 30 Sep 2019 15:47:40 +0200 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 83c6c4d9959..2a1fa17ab58 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,12 @@ # Changelog +## 2.5.0 + +* Fix BC-break when using short-syntax notation for `access_control` +* Fix BC-break when no item operations are declared +* GraphQL: Adding serialization group difference condition for `item_query` and `collection_query` types +* JSON Schema: Fix command + ## 2.5.0 beta 3 * GraphQL: Use different types (`MyTypeItem` and `MyTypeCollection`) only if serialization groups are different for `item_query` and `collection_query` (#3083)