api-platform · soyuka · Jul 27, 2020 · Aug 5, 2020
diff --git a/features/bootstrap/DoctrineContext.php b/features/bootstrap/DoctrineContext.php
@@ -15,6 +15,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\AbsoluteUrlRelationDummy as AbsoluteUrlRelationDummyDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Address as AddressDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Answer as AnswerDocument;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Book as BookDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositeItem as CompositeItemDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositeLabel as CompositeLabelDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CompositePrimitiveItem as CompositePrimitiveItemDocument;
@@ -77,6 +78,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\AbsoluteUrlRelationDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Address;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Answer;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Book;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeItem;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositeLabel;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CompositePrimitiveItem;
@@ -1589,6 +1591,18 @@ public function thereAreNetworkPathDummies(int $nb)
         $this->manager->flush();
     }
 
+    /**
+     * @Given there is a book
+     */
+    public function thereIsABook()
+    {
+        $book = $this->buildBook();
+        $book->name = '1984';
+        $book->isbn = '9780451524935';
+        $this->manager->persist($book);
+        $this->manager->flush();
+    }
+
     private function isOrm(): bool
     {
         return null !== $this->schemaTool;
@@ -2006,4 +2020,12 @@ private function buildNetworkPathRelationDummy()
     {
         return $this->isOrm() ? new NetworkPathRelationDummy() : new NetworkPathRelationDummyDocument();
     }
+
+    /**
+     * @return BookDocument | Book
+     */
+    private function buildBook()
+    {
+        return $this->isOrm() ? new Book() : new BookDocument();
+    }
 }
diff --git a/features/main/composite.feature b/features/main/composite.feature
@@ -119,10 +119,11 @@ Feature: Retrieve data with Composite identifiers
     }
     """
 
-  Scenario: Get the first composite relation with a missing identifier
-    Given there are Composite identifier objects
-    When I send a "GET" request to "/composite_relations/compositeLabel=1;"
-    Then the response status code should be 404
+  # Deprecate ?
+  # Scenario: Get the first composite relation with a missing identifier
+  #   Given there are Composite identifier objects
+  #   When I send a "GET" request to "/composite_relations/compositeLabel=1;"
+  #   Then the response status code should be 404
 
   Scenario: Get first composite item
     Given there are Composite identifier objects

diff --git a/features/main/operation.feature b/features/main/operation.feature
@@ -63,3 +63,23 @@ Feature: Operation support
   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
+
+  @createSchema
+  Scenario: Get a book by it's ISBN
+    Given there is a book
+    When I send a "GET" request to "books/by_isbn/9780451524935"
+    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"
+    And the JSON should be equal to:
+    """
+    {
+        "@context": "/contexts/Book",
+        "@id": "/books/1",
+        "@type": "Book",
+        "name": "1984",
+        "isbn": "9780451524935",
+        "id": 1
+    }
+    """
+
diff --git a/src/Bridge/Doctrine/Orm/ItemDataProvider.php b/src/Bridge/Doctrine/Orm/ItemDataProvider.php
@@ -20,7 +20,6 @@
 use ApiPlatform\Core\DataProvider\DenormalizedIdentifiersAwareItemDataProviderInterface;
 use ApiPlatform\Core\DataProvider\RestrictedDataProviderInterface;
 use ApiPlatform\Core\Exception\RuntimeException;
-use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use Doctrine\Common\Persistence\ManagerRegistry;
@@ -65,19 +64,11 @@ public function supports(string $resourceClass, string $operationName = null, ar
      *
      * @throws RuntimeException
      */
-    public function getItem(string $resourceClass, $id, string $operationName = null, array $context = [])
+    public function getItem(string $resourceClass, $identifiers, string $operationName = null, array $context = [])
     {
         /** @var EntityManagerInterface $manager */
         $manager = $this->managerRegistry->getManagerForClass($resourceClass);
 
-        if ((\is_int($id) || \is_string($id)) && !($context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] ?? false)) {
-            $id = $this->normalizeIdentifiers($id, $manager, $resourceClass);
-        }
-        if (!\is_array($id)) {
-            throw new \InvalidArgumentException(sprintf('$id must be array when "%s" key is set to true in the $context', IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER));
-        }
-        $identifiers = $id;
-
         $fetchData = $context['fetch_data'] ?? true;
         if (!$fetchData) {
             return $manager->getReference($resourceClass, $identifiers);

diff --git a/src/Bridge/Symfony/Bundle/Resources/config/api.xml b/src/Bridge/Symfony/Bundle/Resources/config/api.xml
@@ -43,6 +43,7 @@
             <argument>%api_platform.enable_docs%</argument>
             <argument>%api_platform.graphql.graphiql.enabled%</argument>
             <argument>%api_platform.graphql.graphql_playground.enabled%</argument>
+            <argument type="service" id="api_platform.identifiers_extractor.cached" />
 
             <tag name="routing.loader" />
         </service>

diff --git a/src/Bridge/Symfony/Routing/ApiLoader.php b/src/Bridge/Symfony/Routing/ApiLoader.php
@@ -13,6 +13,7 @@
 
 namespace ApiPlatform\Core\Bridge\Symfony\Routing;
 
+use ApiPlatform\Core\Api\IdentifiersExtractorInterface;
 use ApiPlatform\Core\Api\OperationType;
 use ApiPlatform\Core\Exception\InvalidResourceException;
 use ApiPlatform\Core\Exception\RuntimeException;
@@ -56,8 +57,9 @@ final class ApiLoader extends Loader
     private $graphQlPlaygroundEnabled;
     private $entrypointEnabled;
     private $docsEnabled;
+    private $identifiersExtractor;
 
-    public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $graphiQlEnabled = false, bool $graphQlPlaygroundEnabled = false)
+    public function __construct(KernelInterface $kernel, ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory, ResourceMetadataFactoryInterface $resourceMetadataFactory, OperationPathResolverInterface $operationPathResolver, ContainerInterface $container, array $formats, array $resourceClassDirectories = [], SubresourceOperationFactoryInterface $subresourceOperationFactory = null, bool $graphqlEnabled = false, bool $entrypointEnabled = true, bool $docsEnabled = true, bool $graphiQlEnabled = false, bool $graphQlPlaygroundEnabled = false, IdentifiersExtractorInterface $identifiersExtractor = null)
     {
         /** @var string[]|string $paths */
         $paths = $kernel->locateResource('@ApiPlatformBundle/Resources/config/routing');
@@ -74,6 +76,7 @@ public function __construct(KernelInterface $kernel, ResourceNameCollectionFacto
         $this->graphQlPlaygroundEnabled = $graphQlPlaygroundEnabled;
         $this->entrypointEnabled = $entrypointEnabled;
         $this->docsEnabled = $docsEnabled;
+        $this->identifiersExtractor = $identifiersExtractor;
     }
 
     /**
@@ -221,6 +224,10 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
             }
         }
 
+        if (!isset($operation['identifiedBy'])) {
+            $operation['identifiedBy'] = null !== $this->identifiersExtractor ? $this->identifiersExtractor->getIdentifiersFromResourceClass($resourceClass) : ['id'];
+        }
+
         $path = trim(trim($resourceMetadata->getAttribute('route_prefix', '')), '/');
         $path .= $this->operationPathResolver->resolveOperationPath($resourceShortName, $operation, $operationType, $operationName);
 
@@ -230,6 +237,7 @@ private function addRoute(RouteCollection $routeCollection, string $resourceClas
                 '_controller' => $controller,
                 '_format' => null,
                 '_api_resource_class' => $resourceClass,
+                '_api_identified_by' => \is_array($operation['identifiedBy']) ? $operation['identifiedBy'] : [$operation['identifiedBy']],
                 sprintf('_api_%s_operation_name', $operationType) => $operationName,
             ] + ($operation['defaults'] ?? []),
             $operation['requirements'] ?? [],

diff --git a/src/Bridge/Symfony/Routing/IriConverter.php b/src/Bridge/Symfony/Routing/IriConverter.php
@@ -25,6 +25,7 @@
 use ApiPlatform\Core\Exception\InvalidIdentifierException;
 use ApiPlatform\Core\Exception\ItemNotFoundException;
 use ApiPlatform\Core\Exception\RuntimeException;
+use ApiPlatform\Core\Identifier\CompositeIdentifierParser;
 use ApiPlatform\Core\Identifier\IdentifierConverterInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
@@ -94,10 +95,6 @@ public function getItemFromIri(string $iri, array $context = [])
             throw new InvalidArgumentException($e->getMessage(), $e->getCode(), $e);
         }
 
-        if ($this->identifierConverter) {
-            $context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] = true;
-        }
-
         if (isset($attributes['subresource_operation_name'])) {
             if (($item = $this->getSubresourceData($identifiers, $attributes, $context)) && !\is_array($item)) {
                 return $item;
@@ -148,10 +145,16 @@ public function getItemIriFromResourceClass(string $resourceClass, array $identi
     {
         $routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM);
 
-        try {
-            $identifiers = $this->generateIdentifiersUrl($identifiers, $resourceClass);
+        // Make composite identifiers work, there's only one identifier but the Resources has several:
+        if (1 < \count($identifiers)) {
+            $route = $this->router->getRouteCollection()->get($routeName);
+            if (1 === \count($identifiedBy = $route->getDefault('_api_identified_by'))) {
+                $identifiers = [$identifiedBy[0] => CompositeIdentifierParser::stringify($identifiers)];
+            }
+        }
 
-            return $this->router->generate($routeName, ['id' => implode(';', $identifiers)], $this->getReferenceType($resourceClass, $referenceType));
+        try {
+            return $this->router->generate($routeName, $identifiers, $this->getReferenceType($resourceClass, $referenceType));
         } catch (RoutingExceptionInterface $e) {
             throw new InvalidArgumentException(sprintf('Unable to generate an IRI for "%s".', $resourceClass), $e->getCode(), $e);
         }
@@ -169,30 +172,6 @@ public function getSubresourceIriFromResourceClass(string $resourceClass, array
         }
     }
 
-    /**
-     * Generate the identifier url.
-     *
-     * @throws InvalidArgumentException
-     *
-     * @return string[]
-     */
-    private function generateIdentifiersUrl(array $identifiers, string $resourceClass): array
-    {
-        if (0 === \count($identifiers)) {
-            throw new InvalidArgumentException(sprintf('No identifiers defined for resource of type "%s"', $resourceClass));
-        }
-
-        if (1 === \count($identifiers)) {
-            return [(string) reset($identifiers)];
-        }
-
-        foreach ($identifiers as $name => $value) {
-            $identifiers[$name] = sprintf('%s=%s', $name, $value);
-        }
-
-        return array_values($identifiers);
-    }
-
     private function getReferenceType(string $resourceClass, ?int $referenceType): ?int
     {
         if (null === $referenceType && null !== $this->resourceMetadataFactory) {

diff --git a/src/DataProvider/OperationDataProviderTrait.php b/src/DataProvider/OperationDataProviderTrait.php
@@ -85,38 +85,16 @@ private function getSubresourceData($identifiers, array $attributes, array $cont
      */
     private function extractIdentifiers(array $parameters, array $attributes)
     {
-        if (isset($attributes['item_operation_name'])) {
-            if (!isset($parameters['id'])) {
-                throw new InvalidIdentifierException('Parameter "id" not found');
-            }
-
-            $id = $parameters['id'];
-
-            if (null !== $this->identifierConverter) {
-                return $this->identifierConverter->convert((string) $id, $attributes['resource_class']);
-            }
-
-            return $id;
-        }
-
-        if (!isset($attributes['subresource_context'])) {
-            throw new RuntimeException('Either "item_operation_name" or "collection_operation_name" must be defined, unless the "_api_receive" request attribute is set to false.');
-        }
-
+        $identifiersKeys = $attributes['identified_by'] ?? $attributes['subresource_context']['identifiers'];
         $identifiers = [];
-
-        foreach ($attributes['subresource_context']['identifiers'] as $key => [$id, $resourceClass, $hasIdentifier]) {
-            if (false === $hasIdentifier) {
-                continue;
+        foreach ($identifiersKeys as $identifier) {
+            if (!isset($parameters[$identifier])) {
+                throw new InvalidIdentifierException(sprintf('Parameter "%s" not found', $identifier));
             }
 
-            $identifiers[$id] = $parameters[$id];
-
-            if (null !== $this->identifierConverter) {
-                $identifiers[$id] = $this->identifierConverter->convert((string) $identifiers[$id], $resourceClass);
-            }
+            $identifiers[$identifier] = $parameters[$identifier];
         }
 
-        return $identifiers;
+        return $this->identifierConverter->denormalize($identifiers, $attributes['resource_class']);
     }
 }
diff --git a/src/Identifier/CompositeIdentifierParser.php b/src/Identifier/CompositeIdentifierParser.php
@@ -20,6 +20,8 @@
  */
 final class CompositeIdentifierParser
 {
+    public const COMPOSITE_IDENTIFIER_REGEXP = '/(\w+)=(?<=\w=)(.*?)(?=;\w+=)|(\w+)=([^;]*);?$/';
+
     private function __construct()
     {
     }
@@ -33,7 +35,7 @@ public static function parse(string $identifier): array
     {
         $matches = [];
         $identifiers = [];
-        $num = preg_match_all('/(\w+)=(?<=\w=)(.*?)(?=;\w+=)|(\w+)=([^;]*);?$/', $identifier, $matches, PREG_SET_ORDER);
+        $num = preg_match_all(self::COMPOSITE_IDENTIFIER_REGEXP, $identifier, $matches, PREG_SET_ORDER);
 
         foreach ($matches as $i => $match) {
             if ($i === $num - 1) {
@@ -45,4 +47,14 @@ public static function parse(string $identifier): array
 
         return $identifiers;
     }
+
+    public static function stringify(array $identifiers): string
+    {
+        $composite = [];
+        foreach ($identifiers as $name => $value) {
+            $composite[] = sprintf('%s=%s', $name, $value);
+        }
+
+        return implode(';', $composite);
+    }
 }
diff --git a/src/Identifier/IdentifierConverter.php b/src/Identifier/IdentifierConverter.php
@@ -25,7 +25,7 @@
  *
  * @author Antoine Bluchet <[email protected]>
  */
-final class IdentifierConverter implements ContextAwareIdentifierConverterInterface
+final class IdentifierConverter implements NormalizeIdentifierConverterInterface
 {
     private $propertyMetadataFactory;
     private $identifiersExtractor;
@@ -56,32 +56,40 @@ public function convert(string $data, string $class, array $context = []): array
         $keys = $this->identifiersExtractor->getIdentifiersFromResourceClass($class);
 
         if (($numIdentifiers = \count($keys)) > 1) {
+            // todo put this in normalizer
             $identifiers = CompositeIdentifierParser::parse($data);
         } elseif (0 === $numIdentifiers) {
             throw new InvalidIdentifierException(sprintf('Resource "%s" has no identifiers.', $class));
         } else {
             $identifiers = [$keys[0] => $data];
         }
 
-        // Normalize every identifier (DateTime, UUID etc.)
-        foreach ($keys as $key) {
-            if (!isset($identifiers[$key])) {
-                throw new InvalidIdentifierException(sprintf('Invalid identifier "%1$s", "%1$s" was not found.', $key));
-            }
+        return $this->denormalize($identifiers, $class);
+    }
 
-            if (null === $type = $this->getIdentifierType($class, $key)) {
+    /**
+     * {@inheritdoc}
+     */
+    public function denormalize($identifiers, $class, $format = null, array $context = []): array
+    {
+        // Normalize every identifier (DateTime, UUID etc.)
+        foreach ($identifiers as $identifier => $value) {
+            if (null === $type = $this->getIdentifierType($class, $identifier)) {
+                if (preg_match_all(CompositeIdentifierParser::COMPOSITE_IDENTIFIER_REGEXP, $value)) {
+                    return CompositeIdentifierParser::parse($value);
+                }
                 continue;
             }
 
             foreach ($this->identifierDenormalizers as $identifierDenormalizer) {
-                if (!$identifierDenormalizer->supportsDenormalization($identifiers[$key], $type)) {
+                if (!$identifierDenormalizer->supportsDenormalization($value, $type)) {
                     continue;
                 }
 
                 try {
-                    $identifiers[$key] = $identifierDenormalizer->denormalize($identifiers[$key], $type);
+                    $identifiers[$identifier] = $identifierDenormalizer->denormalize($value, $type);
                 } catch (InvalidIdentifierException $e) {
-                    throw new InvalidIdentifierException(sprintf('Identifier "%s" could not be denormalized.', $key), $e->getCode(), $e);
+                    throw new InvalidIdentifierException(sprintf('Identifier "%s" could not be denormalized.', $identifier), $e->getCode(), $e);
                 }
             }
         }