Rework to improve and simplify identifiers management

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;
@@ -26,6 +27,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConvertedRelated as ConvertedRelatedDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\ConvertedString as ConvertedStringDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Customer as CustomerDocument;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\CustomMultipleIdentifierDummy as CustomMultipleIdentifierDummyDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\Dummy as DummyDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyAggregateOffer as DummyAggregateOfferDocument;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyCar as DummyCarDocument;
@@ -78,6 +80,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;
@@ -89,6 +92,7 @@
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConvertedRelated;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ConvertedString;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Customer;
+use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\CustomMultipleIdentifierDummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Dummy;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyAggregateOffer;
 use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar;
@@ -1605,6 +1609,32 @@ public function thereIsAnInitializeInput(int $id)
         $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();
+    }
+
+    /**
+     * @Given there is a custom multiple identifier dummy
+     */
+    public function thereIsACustomMultipleIdentifierDummy()
+    {
+        $dummy = $this->buildCustomMultipleIdentifierDummy();
+        $dummy->setName('Orwell');
+        $dummy->setFirstId(1);
+        $dummy->setSecondId(2);
+
+        $this->manager->persist($dummy);
+        $this->manager->flush();
+    }
+
     private function isOrm(): bool
     {
         return null !== $this->schemaTool;
@@ -2030,4 +2060,20 @@ private function buildInitializeInput()
     {
         return $this->isOrm() ? new InitializeInput() : new InitializeInputDocument();
     }
+
+    /**
+     * @return BookDocument | Book
+     */
+    private function buildBook()
+    {
+        return $this->isOrm() ? new Book() : new BookDocument();
+    }
+
+    /**
+     * @return CustomMultipleIdentifierDummy | CustomMultipleIdentifierDummyDocument
+     */
+    private function buildCustomMultipleIdentifierDummy()
+    {
+        return $this->isOrm() ? new CustomMultipleIdentifierDummy() : new CustomMultipleIdentifierDummyDocument();
+    }
 }

features/json/relation.feature

@@ -156,7 +156,7 @@ Feature: JSON relations support
     And I send a "POST" request to "/related_dummies" with body:
     """
     {
-      "thirdLevel": "1"
+      "thirdLevel": "/third_levels/1"
     }
     """
     Then the response status code should be 201
@@ -184,16 +184,16 @@ Feature: JSON relations support
     }
     """
 
-  Scenario: Passing a (valid) plain identifier on a relation
+  Scenario: Create a Dummy with relations to RelatedDummy
     When I add "Content-Type" header equal to "application/json"
     And I send a "POST" request to "/dummies" with body:
     """
     {
-      "relatedDummy": "1",
+      "relatedDummy": "/related_dummies/1",
       "relatedDummies": [
-        "1"
+        "/related_dummies/1"
       ],
-      "name": "Dummy with plain relations"
+      "name": "Dummy with relations"
     }
     """
     Then the response status code should be 201
@@ -221,7 +221,7 @@ Feature: JSON relations support
       "relatedOwnedDummy": null,
       "relatedOwningDummy": null,
       "id": 1,
-      "name": "Dummy with plain relations",
+      "name": "Dummy with relations",
       "alias": null,
       "foo": null
     }

features/main/custom_identifier.feature

@@ -101,3 +101,21 @@ Feature: Using custom identifier on resource
     When I send a "DELETE" request to "/custom_identifier_dummies/1"
     Then the response status code should be 204
     And the response should be empty
+
+  @createSchema
+  Scenario: Get a resource
+    Given there is a custom multiple identifier dummy
+    When I send a "GET" request to "/custom_multiple_identifier_dummies/1/2"
+    Then the response status code should be 200
+    And the response should be in JSON
+    And the JSON should be equal to:
+    """
+    {
+      "@context": "/contexts/CustomMultipleIdentifierDummy",
+      "@id": "/custom_multiple_identifier_dummies/1/2",
+      "@type": "CustomMultipleIdentifierDummy",
+      "firstId": 1,
+      "secondId": 2,
+      "name": "Orwell"
+    }
+    """

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 its 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
+    }
+    """
+

phpstan.neon.dist

@@ -138,3 +138,9 @@ parameters:
 			paths:
 				- src/Bridge/Elasticsearch/DataProvider/Filter/OrderFilter.php
 				- src/Bridge/Elasticsearch/DataProvider/Filter/AbstractSearchFilter.php
+
+		-
+			message: '#Unreachable statement - code above always terminates.#'
+			paths:
+				- tests/Serializer/AbstractItemNormalizerTest.php 
+				- tests/Bridge/Doctrine/MongoDbOdm/ItemDataProviderTest.php

phpunit_mongodb.xml

@@ -10,7 +10,10 @@
     <php>
         <ini name="error_reporting" value="-1" />
         <ini name="memory_limit" value="-1" />
-        <env name="SYMFONY_DEPRECATIONS_HELPER" value="max[self]=0" />
+        <server name="SYMFONY_DEPRECATIONS_HELPER" value="max[self]=0" />
+        <!-- This is necessary for GitHub Actions to work properly -->
+        <server name="SYMFONY_PHPUNIT_DIR" value="vendor/bin/.phpunit" />
+        <server name="SYMFONY_PHPUNIT_REMOVE" value="symfony/yaml" />
         <server name="KERNEL_DIR" value="tests/Fixtures/app/" />
         <server name="KERNEL_CLASS" value="AppKernel" />
         <server name="APP_ENV" value="mongodb" />

src/Annotation/ApiResource.php

@@ -28,6 +28,7 @@
  *     @Attribute("attributes", type="array"),
  *     @Attribute("cacheHeaders", type="array"),
  *     @Attribute("collectionOperations", type="array"),
+ *     @Attribute("compositeIdentifier", type="bool"),
  *     @Attribute("denormalizationContext", type="array"),
  *     @Attribute("deprecationReason", type="string"),
  *     @Attribute("description", type="string"),
@@ -89,6 +90,7 @@ final class ApiResource
         'securityPostDenormalizeMessage',
         'cacheHeaders',
         'collectionOperations',
+        'compositeIdentifier',
         'denormalizationContext',
         'deprecationReason',
         'description',
@@ -452,6 +454,13 @@ final class ApiResource
      */
     private $urlGenerationStrategy;
 
+    /**
+     * @see https://github.com/Haehnchen/idea-php-annotation-plugin/issues/112
+     *
+     * @var bool
+     */
+    private $compositeIdentifier;
+
     /**
      * @throws InvalidArgumentException
      */

src/Bridge/Doctrine/MongoDbOdm/ItemDataProvider.php

@@ -16,10 +16,9 @@
 use ApiPlatform\Core\Bridge\Doctrine\Common\Util\IdentifierManagerTrait;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationItemExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\Extension\AggregationResultItemExtensionInterface;
-use ApiPlatform\Core\DataProvider\DenormalizedIdentifiersAwareItemDataProviderInterface;
+use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
 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 ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
@@ -34,7 +33,7 @@
  *
  * @author Alan Poulain <contact@alanpoulain.eu>
  */
-final class ItemDataProvider implements DenormalizedIdentifiersAwareItemDataProviderInterface, RestrictedDataProviderInterface
+final class ItemDataProvider implements ItemDataProviderInterface, RestrictedDataProviderInterface
 {
     use IdentifierManagerTrait;
 
@@ -64,19 +63,13 @@ 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, array $identifiers, ?string $operationName = null, array $context = [])
     {
         /** @var DocumentManager $manager */
         $manager = $this->managerRegistry->getManagerForClass($resourceClass);
 
-        if (!\is_array($id) && !($context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] ?? false)) {
-            $id = $this->normalizeIdentifiers($id, $manager, $resourceClass);
-        }
-
-        $id = (array) $id;
-
         if (!($context['fetch_data'] ?? true)) {
-            return $manager->getReference($resourceClass, reset($id));
+            return $manager->getReference($resourceClass, reset($identifiers));
         }
 
         $repository = $manager->getRepository($resourceClass);
@@ -86,12 +79,12 @@ public function getItem(string $resourceClass, $id, string $operationName = null
 
         $aggregationBuilder = $repository->createAggregationBuilder();
 
-        foreach ($id as $propertyName => $value) {
+        foreach ($identifiers as $propertyName => $value) {
             $aggregationBuilder->match()->field($propertyName)->equals($value);
         }
 
         foreach ($this->itemExtensions as $extension) {
-            $extension->applyToItem($aggregationBuilder, $resourceClass, $id, $operationName, $context);
+            $extension->applyToItem($aggregationBuilder, $resourceClass, $identifiers, $operationName, $context);
 
             if ($extension instanceof AggregationResultItemExtensionInterface && $extension->supportsResult($resourceClass, $operationName, $context)) {
                 return $extension->getResult($aggregationBuilder, $resourceClass, $operationName, $context);

src/Bridge/Doctrine/MongoDbOdm/SubresourceDataProvider.php

@@ -21,7 +21,6 @@
 use ApiPlatform\Core\DataProvider\SubresourceDataProviderInterface;
 use ApiPlatform\Core\Exception\ResourceClassNotSupportedException;
 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 ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
@@ -136,24 +135,14 @@ private function buildAggregation(array $identifiers, array $context, array $exe
         }
 
         $aggregation = $manager->createAggregationBuilder($identifierResourceClass);
-        $normalizedIdentifiers = [];
-
-        if (isset($identifiers[$identifier])) {
-            // if it's an array it's already normalized, the IdentifierManagerTrait is deprecated
-            if ($context[IdentifierConverterInterface::HAS_IDENTIFIER_CONVERTER] ?? false) {
-                $normalizedIdentifiers = $identifiers[$identifier];
-            } else {
-                $normalizedIdentifiers = $this->normalizeIdentifiers($identifiers[$identifier], $manager, $identifierResourceClass);
-            }
-        }
 
         if ($classMetadata->hasAssociation($previousAssociationProperty)) {
             $aggregation->lookup($previousAssociationProperty)->alias($previousAssociationProperty);
-            foreach ($normalizedIdentifiers as $key => $value) {
+            foreach (\is_array($identifiers[$identifier]) ? $identifiers[$identifier] : $identifiers as $key => $value) {
                 $aggregation->match()->field($key)->equals($value);
             }
         } elseif ($classMetadata->isIdentifier($previousAssociationProperty)) {
-            foreach ($normalizedIdentifiers as $key => $value) {
+            foreach (\is_array($identifiers[$identifier]) ? $identifiers[$identifier] : $identifiers as $key => $value) {
                 $aggregation->match()->field($key)->equals($value);
             }
 

src/Bridge/Doctrine/Orm/ItemDataProvider.php

@@ -13,20 +13,16 @@
 
 namespace ApiPlatform\Core\Bridge\Doctrine\Orm;
 
-use ApiPlatform\Core\Bridge\Doctrine\Common\Util\IdentifierManagerTrait;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryItemExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Extension\QueryResultItemExtensionInterface;
 use ApiPlatform\Core\Bridge\Doctrine\Orm\Util\QueryNameGenerator;
-use ApiPlatform\Core\DataProvider\DenormalizedIdentifiersAwareItemDataProviderInterface;
+use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
 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;
+use Doctrine\Common\Persistence\Mapping\ClassMetadata;
 use Doctrine\ORM\EntityManagerInterface;
 use Doctrine\ORM\QueryBuilder;
-use Doctrine\Persistence\ManagerRegistry;
-use Doctrine\Persistence\Mapping\ClassMetadata;
 
 /**
  * Item data provider for the Doctrine ORM.
@@ -35,21 +31,17 @@
  * @author Samuel ROZE <samuel.roze@gmail.com>
  * @final
  */
-class ItemDataProvider implements DenormalizedIdentifiersAwareItemDataProviderInterface, RestrictedDataProviderInterface
+class ItemDataProvider implements ItemDataProviderInterface, RestrictedDataProviderInterface
 {
-    use IdentifierManagerTrait;
-
     private $managerRegistry;
     private $itemExtensions;
 
     /**
      * @param QueryItemExtensionInterface[] $itemExtensions
      */
-    public function __construct(ManagerRegistry $managerRegistry, PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, iterable $itemExtensions = [])
+    public function __construct(ManagerRegistry $managerRegistry, iterable $itemExtensions = [])
     {
         $this->managerRegistry = $managerRegistry;
-        $this->propertyNameCollectionFactory = $propertyNameCollectionFactory;
-        $this->propertyMetadataFactory = $propertyMetadataFactory;
         $this->itemExtensions = $itemExtensions;
     }
 
@@ -65,19 +57,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, array $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);
@@ -119,7 +103,6 @@ private function addWhereForIdentifiers(array $identifiers, QueryBuilder $queryB
             );
 
             $queryBuilder->andWhere($expression);
-
             $queryBuilder->setParameter($placeholder, $value, $classMetadata->getTypeOfField($identifier));
         }
     }