diff --git a/CHANGELOG.md b/CHANGELOG.md index ea73b393bdc..fafd4311f6c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ * Swagger UI: Add `swagger_ui_extra_configuration` to Swagger / OpenAPI configuration (#3731) * GraphQL: Support `ApiProperty` security (#4143) * GraphQL: **BC** Fix security on association collection properties. The collection resource `item_query` security is no longer used. `ApiProperty` security can now be used to secure collection (or any other) properties. (#4143) +* Deprecate `allow_plain_identifiers` option (#4167) ## 2.6.4 diff --git a/features/json/relation.feature b/features/json/relation.feature index 7c39dc75f71..f76b9096b36 100644 --- a/features/json/relation.feature +++ b/features/json/relation.feature @@ -120,7 +120,7 @@ Feature: JSON relations support } """ - Scenario: Update an embedded relation using plain identifiers + Scenario: Update an embedded relation When I add "Content-Type" header equal to "application/json" And I send a "PUT" request to "/relation_embedders/1" with body: """ @@ -151,7 +151,8 @@ Feature: JSON relations support } """ - Scenario: Create a related dummy with a relation + # TODO: to remove in 3.0 + Scenario: Create a related dummy with a relation using plain identifiers When I add "Content-Type" header equal to "application/json" And I send a "POST" request to "/related_dummies" with body: """ @@ -184,6 +185,7 @@ Feature: JSON relations support } """ + # TODO: to remove in 3.0 Scenario: Passing a (valid) plain identifier on a relation When I add "Content-Type" header equal to "application/json" And I send a "POST" request to "/dummies" with body: diff --git a/features/main/relation.feature b/features/main/relation.feature index 8c268ed8417..dd4a65c0a99 100644 --- a/features/main/relation.feature +++ b/features/main/relation.feature @@ -497,13 +497,13 @@ Feature: Relations support And I send a "POST" request to "/relation_embedders" with body: """ { - "related": "certainly not an iri and not a plain identifier" + "related": "certainly not an IRI" } """ Then the response status code should be 400 And the response should be in JSON And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8" - And the JSON node "hydra:description" should contain 'Invalid IRI "certainly not an iri and not a plain identifier".' + And the JSON node "hydra:description" should contain 'Invalid IRI "certainly not an IRI".' Scenario: Passing an invalid type to a relation When I add "Content-Type" header equal to "application/ld+json" diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php index 09af447af4c..5d7100bafec 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php @@ -177,6 +177,7 @@ private function registerCommonConfiguration(ContainerBuilder $container, array $container->setParameter('api_platform.formats', $formats); $container->setParameter('api_platform.patch_formats', $patchFormats); $container->setParameter('api_platform.error_formats', $errorFormats); + // TODO: to remove in 3.0 $container->setParameter('api_platform.allow_plain_identifiers', $config['allow_plain_identifiers']); $container->setParameter('api_platform.eager_loading.enabled', $this->isConfigEnabled($container, $config['eager_loading'])); $container->setParameter('api_platform.eager_loading.max_joins', $config['eager_loading']['max_joins']); diff --git a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php index 6380d02c74a..962cc16beca 100644 --- a/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php +++ b/src/Bridge/Symfony/Bundle/DependencyInjection/Configuration.php @@ -95,7 +95,11 @@ public function getConfigTreeBuilder() ->scalarNode('name_converter')->defaultNull()->info('Specify a name converter to use.')->end() ->scalarNode('asset_package')->defaultNull()->info('Specify an asset package name to use.')->end() ->scalarNode('path_segment_name_generator')->defaultValue('api_platform.path_segment_name_generator.underscore')->info('Specify a path name generator to use.')->end() - ->booleanNode('allow_plain_identifiers')->defaultFalse()->info('Allow plain identifiers, for example "id" instead of "@id" when denormalizing a relation.')->end() + ->booleanNode('allow_plain_identifiers') + ->defaultFalse() + ->info('Allow plain identifiers, for example "id" instead of "@id" when denormalizing a relation.') + ->setDeprecated(...$this->buildDeprecationArgs('2.7', 'The use of `allow_plain_identifiers` has been deprecated in 2.7 and will be removed in 3.0.')) + ->end() ->arrayNode('validator') ->addDefaultsIfNotSet() ->children() diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml index 1f4cf3a1dcb..fc1bf40e797 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/api.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/api.xml @@ -111,6 +111,7 @@ + %api_platform.allow_plain_identifiers% null diff --git a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml index 8d679d5c6f6..4e23af26195 100644 --- a/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml +++ b/src/Bridge/Symfony/Bundle/Resources/config/graphql.xml @@ -216,6 +216,7 @@ + %api_platform.allow_plain_identifiers% null diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 3e41e9da00d..8ab7b0e116a 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -88,7 +88,12 @@ public function __construct(PropertyNameCollectionFactoryInterface $propertyName $this->resourceClassResolver = $resourceClassResolver; $this->propertyAccessor = $propertyAccessor ?: PropertyAccess::createPropertyAccessor(); $this->itemDataProvider = $itemDataProvider; + + if (true === $allowPlainIdentifiers) { + @trigger_error(sprintf('Allowing plain identifiers as argument of "%s" is deprecated since API Platform 2.7 and will not be possible anymore in API Platform 3.', self::class), \E_USER_DEPRECATED); + } $this->allowPlainIdentifiers = $allowPlainIdentifiers; + $this->dataTransformers = $dataTransformers; $this->resourceMetadataFactory = $resourceMetadataFactory; $this->resourceAccessChecker = $resourceAccessChecker; @@ -851,6 +856,11 @@ private function setValue($object, string $attributeName, $value) } } + /** + * TODO: to remove in 3.0. + * + * @deprecated since 2.7 + */ private function supportsPlainIdentifiers(): bool { return $this->allowPlainIdentifiers && null !== $this->itemDataProvider; diff --git a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php index 1f5b861ee64..6b11bbcce05 100644 --- a/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php +++ b/tests/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtensionTest.php @@ -918,6 +918,7 @@ private function getPartialContainerBuilderProphecy($configuration = null) 'api_platform.title' => 'title', 'api_platform.version' => 'version', 'api_platform.show_webby' => true, + // TODO: to remove in 3.0 'api_platform.allow_plain_identifiers' => false, 'api_platform.eager_loading.enabled' => Argument::type('bool'), 'api_platform.eager_loading.max_joins' => 30, diff --git a/tests/Fixtures/TestBundle/Serializer/Denormalizer/DummyPlainIdentifierDenormalizer.php b/tests/Fixtures/TestBundle/Serializer/Denormalizer/DummyPlainIdentifierDenormalizer.php new file mode 100644 index 00000000000..0f072e9655e --- /dev/null +++ b/tests/Fixtures/TestBundle/Serializer/Denormalizer/DummyPlainIdentifierDenormalizer.php @@ -0,0 +1,71 @@ + + * + * 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\Serializer\Denormalizer; + +use ApiPlatform\Core\Api\IriConverterInterface; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Dummy as DummyDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\RelatedDummy as RelatedDummyDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy as DummyEntity; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy as RelatedDummyEntity; +use Symfony\Component\Serializer\Normalizer\ContextAwareDenormalizerInterface; +use Symfony\Component\Serializer\Normalizer\DenormalizerAwareInterface; +use Symfony\Component\Serializer\Normalizer\DenormalizerAwareTrait; + +/** + * BC to keep allow_plain_identifiers working in 2.7 tests + * TODO: to remove in 3.0. + * + * @author Vincent Chalamon + */ +class DummyPlainIdentifierDenormalizer implements ContextAwareDenormalizerInterface, DenormalizerAwareInterface +{ + use DenormalizerAwareTrait; + + private $iriConverter; + + public function __construct(IriConverterInterface $iriConverter) + { + $this->iriConverter = $iriConverter; + } + + /** + * {@inheritdoc} + */ + public function denormalize($data, $class, $format = null, array $context = []) + { + $relatedDummyClass = DummyEntity::class === $class ? RelatedDummyEntity::class : RelatedDummyDocument::class; + if (!empty($data['relatedDummy'])) { + $data['relatedDummy'] = $this->iriConverter->getItemIriFromResourceClass($relatedDummyClass, ['id' => $data['relatedDummy']]); + } + + if (!empty($data['relatedDummies'])) { + foreach ($data['relatedDummies'] as $k => $v) { + $data['relatedDummies'][$k] = $this->iriConverter->getItemIriFromResourceClass($relatedDummyClass, ['id' => $v]); + } + } + + return $this->denormalizer->denormalize($data, $class, $format, $context + [__CLASS__ => true]); + } + + /** + * {@inheritdoc} + */ + public function supportsDenormalization($data, $type, $format = null, array $context = []): bool + { + return 'json' === $format + && (is_a($type, DummyEntity::class, true) || is_a($type, DummyDocument::class, true)) + && ('1' === ($data['relatedDummy'] ?? null) || ['1'] === ($data['relatedDummies'] ?? null)) + && !isset($context[__CLASS__]); + } +} diff --git a/tests/Fixtures/TestBundle/Serializer/Denormalizer/RelatedDummyPlainIdentifierDenormalizer.php b/tests/Fixtures/TestBundle/Serializer/Denormalizer/RelatedDummyPlainIdentifierDenormalizer.php new file mode 100644 index 00000000000..079a89edb13 --- /dev/null +++ b/tests/Fixtures/TestBundle/Serializer/Denormalizer/RelatedDummyPlainIdentifierDenormalizer.php @@ -0,0 +1,65 @@ + + * + * 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\Serializer\Denormalizer; + +use ApiPlatform\Core\Api\IriConverterInterface; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\RelatedDummy as RelatedDummyDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ThirdLevel as ThirdLevelDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy as RelatedDummyEntity; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ThirdLevel as ThirdLevelEntity; +use Symfony\Component\Serializer\Normalizer\ContextAwareDenormalizerInterface; +use Symfony\Component\Serializer\Normalizer\DenormalizerAwareInterface; +use Symfony\Component\Serializer\Normalizer\DenormalizerAwareTrait; + +/** + * BC to keep allow_plain_identifiers working in 2.7 tests + * TODO: to remove in 3.0. + * + * @author Vincent Chalamon + */ +class RelatedDummyPlainIdentifierDenormalizer implements ContextAwareDenormalizerInterface, DenormalizerAwareInterface +{ + use DenormalizerAwareTrait; + + private $iriConverter; + + public function __construct(IriConverterInterface $iriConverter) + { + $this->iriConverter = $iriConverter; + } + + /** + * {@inheritdoc} + */ + public function denormalize($data, $class, $format = null, array $context = []) + { + $data['thirdLevel'] = $this->iriConverter->getItemIriFromResourceClass( + RelatedDummyEntity::class === $class ? ThirdLevelEntity::class : ThirdLevelDocument::class, + ['id' => $data['thirdLevel']] + ); + + return $this->denormalizer->denormalize($data, $class, $format, $context + [__CLASS__ => true]); + } + + /** + * {@inheritdoc} + */ + public function supportsDenormalization($data, $type, $format = null, array $context = []): bool + { + return 'json' === $format + && (is_a($type, RelatedDummyEntity::class, true) || is_a($type, RelatedDummyDocument::class, true)) + && '1' === ($data['thirdLevel'] ?? null) + && !isset($context[__CLASS__]); + } +} diff --git a/tests/Fixtures/app/config/config_common.yml b/tests/Fixtures/app/config/config_common.yml index 20e9dadb1f8..c5b565d06f8 100644 --- a/tests/Fixtures/app/config/config_common.yml +++ b/tests/Fixtures/app/config/config_common.yml @@ -25,7 +25,6 @@ api_platform: description: | This is a test API. Made with love - allow_plain_identifiers: true formats: jsonld: ['application/ld+json'] jsonhal: ['application/hal+json'] @@ -118,6 +117,20 @@ services: tags: - { name: 'serializer.normalizer' } + app.serializer.denormalizer.related_dummy_plain_identifier: + class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Serializer\Denormalizer\RelatedDummyPlainIdentifierDenormalizer' + public: false + arguments: ['@api_platform.iri_converter'] + tags: + - { name: 'serializer.normalizer' } + + app.serializer.denormalizer.dummy_plain_identifier: + class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Serializer\Denormalizer\DummyPlainIdentifierDenormalizer' + public: false + arguments: ['@api_platform.iri_converter'] + tags: + - { name: 'serializer.normalizer' } + app.name_converter: class: 'ApiPlatform\Core\Tests\Fixtures\TestBundle\Serializer\NameConverter\CustomConverter' diff --git a/tests/Serializer/AbstractItemNormalizerTest.php b/tests/Serializer/AbstractItemNormalizerTest.php index 47af10747d4..09592c80316 100644 --- a/tests/Serializer/AbstractItemNormalizerTest.php +++ b/tests/Serializer/AbstractItemNormalizerTest.php @@ -942,6 +942,11 @@ public function testChildInheritedProperty(): void ], $normalizer->normalize($dummy, null, ['resource_class' => DummyTableInheritance::class, 'resources' => []])); } + /** + * TODO: to remove in 3.0. + * + * @group legacy + */ public function testDenormalizeRelationWithPlainId() { $data = [ @@ -995,6 +1000,11 @@ public function testDenormalizeRelationWithPlainId() $propertyAccessorProphecy->setValue($actual, 'relatedDummy', $relatedDummy)->shouldHaveBeenCalled(); } + /** + * TODO: to remove in 3.0. + * + * @group legacy + */ public function testDenormalizeRelationWithPlainIdNotFound() { $this->expectException(ItemNotFoundException::class); @@ -1054,6 +1064,9 @@ public function testDenormalizeRelationWithPlainIdNotFound() $normalizer->denormalize($data, Dummy::class, 'jsonld'); } + /** + * TODO: to remove in 3.0. + */ public function testDoNotDenormalizeRelationWithPlainIdWhenPlainIdentifiersAreNotAllowed() { $this->expectException(UnexpectedValueException::class);