Merge pull request #1478 from soyuka/fix/identifier-normalizer-1309

Improve ":id" param normalization

Author: dunglas

.travis.yml

@@ -41,7 +41,7 @@ before_install:
   - phpenv config-rm xdebug.ini || echo "xdebug not available"
   - echo "memory_limit=-1" >> ~/.phpenv/versions/$(phpenv version-name)/etc/conf.d/travis.ini
   - npm install -g swagger-cli
-  - if [[ $lint = 1 ]]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.8.2/php-cs-fixer.phar; fi
+  - if [[ $lint = 1 ]]; then wget https://github.com/FriendsOfPHP/PHP-CS-Fixer/releases/download/v2.8.4/php-cs-fixer.phar; fi
   - if [[ $lint = 1 ]]; then composer global require --dev 'phpstan/phpstan:^0.8'; fi
   - export PATH="$PATH:$HOME/.composer/vendor/bin"
 

composer.json

@@ -44,6 +44,8 @@
         "phpdocumentor/type-resolver": "^0.2.1 || ^0.3 || 0.4",
         "phpunit/phpunit": "^6.1",
         "psr/log": "^1.0",
+        "ramsey/uuid": "^3.7",
+        "ramsey/uuid-doctrine": "^1.4",
         "sensio/framework-extra-bundle": "^3.0.11 || ^4.0",
         "symfony/asset": "^3.3 || ^4.0",
         "symfony/cache": "^3.3 || ^4.0",
@@ -75,6 +77,7 @@
         "guzzlehttp/guzzle": "To use the HTTP cache invalidation system.",
         "phpdocumentor/reflection-docblock": "To support extracting metadata from PHPDoc.",
         "psr/cache-implementation": "To use metadata caching.",
+        "ramsey/uuid": "To support Ramsey's UUID identifiers.",
         "symfony/cache": "To have metadata caching when using Symfony integration.",
         "symfony/config": "To load XML configuration files.",
         "symfony/expression-language": "To use authorization features.",

features/bootstrap/FeatureContext.php

@@ -38,6 +38,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\PersonToPet;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Pet;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Question;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RamseyUuidDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedToDummyFriend;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelationEmbedder;
@@ -870,4 +871,16 @@ public function thereAreDummyImmutableDateObjectsWithDummyDate(int $nb)
 
         $this->manager->flush();
     }
+
+    /**
+     * @Given there is a ramsey identified resource with uuid :uuid
+     */
+    public function thereIsARamseyIdentifiedResource(string $uuid)
+    {
+        $dummy = new RamseyUuidDummy();
+        $dummy->setId($uuid);
+
+        $this->manager->persist($dummy);
+        $this->manager->flush();
+    }
 }

features/main/uuid.feature

@@ -101,3 +101,24 @@ Feature: Using uuid identifier on resource
     When I send a "DELETE" request to "/uuid_identifier_dummies/41b29566-144b-11e6-a148-3e1d05defe78"
     Then the response status code should be 204
     And the response should be empty
+
+  @createSchema
+  Scenario: Retrieve a resource identified by Ramsey\Uuid\Uuid
+    Given there is a ramsey identified resource with uuid "41B29566-144B-11E6-A148-3E1D05DEFE78"
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "GET" request to "/ramsey_uuid_dummies/41B29566-144B-11E6-A148-3E1D05DEFE78"
+    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/ld+json; charset=utf-8"
+
+  Scenario: Delete a resource identified by a Ramsey\Uuid\Uuid
+    When I send a "DELETE" request to "/ramsey_uuid_dummies/41B29566-144B-11E6-A148-3E1D05DEFE78"
+    Then the response status code should be 204
+    And the response should be empty
+
+  Scenario: Retrieve a resource identified by a bad Ramsey\Uuid\Uuid
+    When I add "Content-Type" header equal to "application/ld+json"
+    And I send a "GET" request to "/ramsey_uuid_dummies/41B29566-144B-E1D05DEFE78"
+    Then the response status code should be 404
+    And the response should be in JSON
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"

src/Bridge/Doctrine/Orm/ItemDataProvider.php

@@ -20,6 +20,7 @@
 use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
 use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
 use ApiPlatform\Core\Exception\RuntimeException;
+use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use Doctrine\Common\Persistence\ManagerRegistry;
@@ -71,11 +72,13 @@ public function getItem(string $resourceClass, $id, string $operationName = null
     {
         $manager = $this->managerRegistry->getManagerForClass($resourceClass);
 
-        $identifiers = $this->normalizeIdentifiers($id, $manager, $resourceClass);
+        if (!($context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] ?? false)) {
+            $id = $this->normalizeIdentifiers($id, $manager, $resourceClass);
+        }
 
         $fetchData = $context['fetch_data'] ?? true;
         if (!$fetchData && $manager instanceof EntityManagerInterface) {
-            return $manager->getReference($resourceClass, $identifiers);
+            return $manager->getReference($resourceClass, $id);
         }
 
         $repository = $manager->getRepository($resourceClass);
@@ -87,10 +90,10 @@ public function getItem(string $resourceClass, $id, string $operationName = null
         $queryNameGenerator = new QueryNameGenerator();
         $doctrineClassMetadata = $manager->getClassMetadata($resourceClass);
 
-        $this->addWhereForIdentifiers($identifiers, $queryBuilder, $doctrineClassMetadata);
+        $this->addWhereForIdentifiers($id, $queryBuilder, $doctrineClassMetadata);
 
         foreach ($this->itemExtensions as $extension) {
-            $extension->applyToItem($queryBuilder, $queryNameGenerator, $resourceClass, $identifiers, $operationName, $context);
+            $extension->applyToItem($queryBuilder, $queryNameGenerator, $resourceClass, $id, $operationName, $context);
 
             if ($extension instanceof QueryResultItemExtensionInterface && $extension->supportsResult($resourceClass, $operationName, $context)) {
                 return $extension->getResult($queryBuilder, $resourceClass, $operationName, $context);

src/Bridge/Doctrine/Orm/SubresourceDataProvider.php

@@ -23,6 +23,7 @@
 use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface;
 use ApiPlatform\Core\Exception\ResourceClassNotSupportedException;
 use ApiPlatform\Core\Exception\RuntimeException;
+use ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use Doctrine\Common\Persistence\ManagerRegistry;
@@ -152,11 +153,16 @@ private function buildQuery(array $identifiers, array $context, QueryNameGenerat
         $qb = $manager->createQueryBuilder();
         $alias = $queryNameGenerator->generateJoinAlias($identifier);
         $relationType = $classMetadata->getAssociationMapping($previousAssociationProperty)['type'];
-        $normalizedIdentifiers = isset($identifiers[$identifier]) ? $this->normalizeIdentifiers(
-            $identifiers[$identifier],
-            $manager,
-            $identifierResourceClass
-        ) : [];
+        $normalizedIdentifiers = [];
+
+        if (isset($identifiers[$identifier])) {
+            // if it's an array it's already normalized, the IdentifierManagerTrait is deprecated
+            if ($context[ChainIdentifierNormalizer::HAS_IDENTIFIER_NORMALIZER] ?? false) {
+                $normalizedIdentifiers = $identifiers[$identifier];
+            } else {
+                $normalizedIdentifiers = $this->normalizeIdentifiers($identifiers[$identifier], $manager, $identifierResourceClass);
+            }
+        }
 
         switch ($relationType) {
             // MANY_TO_MANY relations need an explicit join so that the identifier part can be retrieved

src/Bridge/Doctrine/Orm/Util/IdentifierManagerTrait.php

@@ -72,7 +72,7 @@ private function normalizeIdentifiers($id, ObjectManager $manager, string $resou
 
             $identifier = null === $identifiersMap ? $identifierValues[$i] ?? null : $identifiersMap[$propertyName] ?? null;
             if (null === $identifier) {
-                throw new PropertyNotFoundException(sprintf('Invalid identifier "%s", "%s" has not been found.', $id, $propertyName));
+                throw new PropertyNotFoundException(sprintf('Invalid identifier "%s", "%s" was not found.', $id, $propertyName));
             }
 
             $doctrineTypeName = $doctrineClassMetadata->getTypeOfField($propertyName);

src/Bridge/RamseyUuid/Identifier/Normalizer/UuidNormalizer.php

@@ -0,0 +1,48 @@
+<?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\Bridge\RamseyUuid\Identifier\Normalizer;
+
+use ApiPlatform\Core\Exception\InvalidIdentifierException;
+use Ramsey\Uuid\Exception\InvalidUuidStringException;
+use Ramsey\Uuid\Uuid;
+use Ramsey\Uuid\UuidInterface;
+use Symfony\Component\Serializer\Normalizer\DenormalizerInterface;
+
+/**
+ * Denormalizes an UUID string to an instance of Ramsey\Uuid.
+ *
+ * @author Antoine Bluchet <soyuka@gmail.com>
+ */
+final class UuidNormalizer implements DenormalizerInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function denormalize($data, $class, $format = null, array $context = [])
+    {
+        try {
+            return Uuid::fromString($data);
+        } catch (InvalidUuidStringException $e) {
+            throw new InvalidIdentifierException($e->getMessage());
+        }
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function supportsDenormalization($data, $type, $format = null)
+    {
+        return is_a($type, UuidInterface::class, true);
+    }
+}

src/Bridge/Symfony/Bundle/DependencyInjection/ApiPlatformExtension.php

@@ -26,6 +26,7 @@
 use Doctrine\Common\Annotations\Annotation;
 use Doctrine\ORM\Version;
 use phpDocumentor\Reflection\DocBlockFactoryInterface;
+use Ramsey\Uuid\Uuid;
 use Symfony\Component\Cache\Adapter\ArrayAdapter;
 use Symfony\Component\Config\FileLocator;
 use Symfony\Component\Config\Resource\DirectoryResource;
@@ -122,6 +123,10 @@ public function load(array $configs, ContainerBuilder $container)
             $loader->load('security.xml');
         }
 
+        if (!class_exists(Uuid::class)) {
+            $container->removeDefinition('api_platform.identifier.uuid_normalizer');
+        }
+
         $useDoctrine = isset($bundles['DoctrineBundle']) && class_exists(Version::class);
 
         $this->registerMetadataConfiguration($container, $config, $loader);

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

@@ -58,6 +58,7 @@
             <argument type="service" id="api_platform.router" />
             <argument type="service" id="api_platform.property_accessor" />
             <argument type="service" id="api_platform.identifiers_extractor.cached" />
+            <argument type="service" id="api_platform.identifier.normalizer" />
         </service>
         <service id="ApiPlatform\Core\Api\IriConverterInterface" alias="api_platform.iri_converter" />
 
@@ -138,6 +139,7 @@
             <argument type="service" id="api_platform.item_data_provider" />
             <argument type="service" id="api_platform.subresource_data_provider" />
             <argument type="service" id="api_platform.serializer.context_builder" />
+            <argument type="service" id="api_platform.identifier.normalizer" />
 
             <tag name="kernel.event_listener" event="kernel.request" method="onKernelRequest" priority="4" />
         </service>
@@ -211,7 +213,7 @@
             <argument>%api_platform.exception_to_status%</argument>
         </service>
 
-        <!-- Identifiers extractor -->
+        <!-- Identifiers -->
 
         <service id="api_platform.identifiers_extractor" class="ApiPlatform\Core\Api\IdentifiersExtractor" public="false">
             <argument type="service" id="api_platform.metadata.property.name_collection_factory" />
@@ -227,6 +229,22 @@
             <argument type="service" id="api_platform.resource_class_resolver" />
         </service>
 
+        <service id="api_platform.identifier.normalizer" class="ApiPlatform\Core\Identifier\Normalizer\ChainIdentifierNormalizer" public="false">
+            <argument type="service" id="api_platform.identifiers_extractor.cached" />
+            <argument type="service" id="api_platform.metadata.property.metadata_factory" />
+            <argument type="tagged" tag="api_platform.identifier.normalizer" />
+        </service>
+
+        <service id="api_platform.identifier.composite_identifier.normalizer" class="ApiPlatform\Core\Identifier\Normalizer\CompositeIdentifierNormalizer" public="false" />
+
+        <service id="api_platform.identifier.date_normalizer" class="ApiPlatform\Core\Identifier\Normalizer\DateTimeIdentifierNormalizer" public="false">
+            <tag name="api_platform.identifier.normalizer" />
+        </service>
+
+        <service id="api_platform.identifier.uuid_normalizer" class="ApiPlatform\Core\Bridge\RamseyUuid\Identifier\Normalizer\UuidNormalizer" public="false">
+            <tag name="api_platform.identifier.normalizer" />
+        </service>
+
         <!-- Subresources -->
 
         <service id="api_platform.subresource_operation_factory" class="ApiPlatform\Core\Operation\Factory\SubresourceOperationFactory" public="false">