Fix #2686: deprecate allow_plain_identifiers option

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
 

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:

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"

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']);

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()

src/Bridge/Symfony/Bundle/Resources/config/api.xml

@@ -111,6 +111,7 @@
             <argument type="service" id="api_platform.name_converter" on-invalid="ignore" />
             <argument type="service" id="serializer.mapping.class_metadata_factory" on-invalid="ignore" />
             <argument type="service" id="api_platform.item_data_provider" on-invalid="ignore" />
+            <!-- TODO: to remove in 3.0 -->
             <argument>%api_platform.allow_plain_identifiers%</argument>
             <argument>null</argument>
             <argument type="tagged" tag="api_platform.data_transformer" on-invalid="ignore" />

src/Bridge/Symfony/Bundle/Resources/config/graphql.xml

@@ -216,6 +216,7 @@
             <argument type="service" id="api_platform.name_converter" on-invalid="ignore" />
             <argument type="service" id="serializer.mapping.class_metadata_factory" on-invalid="ignore" />
             <argument type="service" id="api_platform.item_data_provider" on-invalid="ignore" />
+            <!-- TODO: to remove in 3.0 -->
             <argument>%api_platform.allow_plain_identifiers%</argument>
             <argument>null</argument>
             <argument type="tagged" tag="api_platform.data_transformer" on-invalid="ignore" />

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;

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,

tests/Fixtures/TestBundle/Serializer/Denormalizer/DummyPlainIdentifierDenormalizer.php

@@ -0,0 +1,71 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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 <vincentchalamon@gmail.com>
+ */
+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__]);
+    }
+}

tests/Fixtures/TestBundle/Serializer/Denormalizer/RelatedDummyPlainIdentifierDenormalizer.php

@@ -0,0 +1,65 @@
+<?php
+
+/*
+ * This file is part of the API Platform project.
+ *
+ * (c) Kévin Dunglas <dunglas@gmail.com>
+ *
+ * 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 <vincentchalamon@gmail.com>
+ */
+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__]);
+    }
+}

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'
 

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);