feat: improve iri converter interface (#4734)

* feat: improve iri converter interface

* fix bc layer

* fix new metadata iris

* item non nullable

* fix phpstan

* self review

* fix features

* iterable

* review interface

* phpstan

Author: soyuka

.github/patch/v3_features.patch

@@ -0,0 +1,141 @@
+diff --git a/features/jsonld/input_output.feature b/features/jsonld/input_output.feature
+index ad7ee0344..f05fef8bf 100644
+--- a/features/jsonld/input_output.feature
++++ b/features/jsonld/input_output.feature
+@@ -65,7 +65,7 @@ Feature: JSON-LD DTO input and output
+     """
+     {
+       "@context": "/contexts/DummyDtoCustom",
+-      "@id": "/dummy_dto_customs",
++      "@id": "/dummy_dto_custom_output",
+       "@type": "hydra:Collection",
+       "hydra:member": [
+         {
+diff --git a/features/main/custom_identifier_with_subresource.feature b/features/main/custom_identifier_with_subresource.feature
+index cf18a15a3..773dd8728 100644
+--- a/features/main/custom_identifier_with_subresource.feature
++++ b/features/main/custom_identifier_with_subresource.feature
+@@ -84,7 +84,7 @@ Feature: Using custom parent identifier for subresources
+     """
+     {
+       "@context": "/contexts/SlugParentDummy",
+-      "@id": "/slug_parent_dummies/parent-dummy",
++      "@id": "/slug_child_dummies/child-dummy/parent_dummy",
+       "@type": "SlugParentDummy",
+       "id": 1,
+       "slug": "parent-dummy",
+diff --git a/features/main/custom_operation.feature b/features/main/custom_operation.feature
+index 6b8e4a7bc..9fa829ec5 100644
+--- a/features/main/custom_operation.feature
++++ b/features/main/custom_operation.feature
+@@ -64,7 +64,7 @@ Feature: Custom operation
+     """
+     {
+         "@context": "/contexts/CustomActionDummy",
+-        "@id": "/custom_action_dummies/1",
++        "@id": "/custom_action_collection_dummies/1",
+         "@type": "CustomActionDummy",
+         "id": 1,
+         "foo": "custom!"
+diff --git a/features/main/default_order.feature b/features/main/default_order.feature
+index 1bc97ab37..912110143 100644
+--- a/features/main/default_order.feature
++++ b/features/main/default_order.feature
+@@ -127,7 +127,7 @@ Feature: Default order
+     """
+     {
+       "@context": "/contexts/Foo",
+-      "@id": "/foos",
++      "@id": "/custom_collection_asc_foos",
+       "@type": "hydra:Collection",
+       "hydra:member": [
+         {
+@@ -183,7 +183,7 @@ Feature: Default order
+     """
+     {
+       "@context": "/contexts/Foo",
+-      "@id": "/foos",
++      "@id": "/custom_collection_desc_foos",
+       "@type": "hydra:Collection",
+       "hydra:member": [
+         {
+diff --git a/features/main/operation.feature b/features/main/operation.feature
+index 24dda9b87..8786f6b04 100644
+--- a/features/main/operation.feature
++++ b/features/main/operation.feature
+@@ -47,7 +47,7 @@ Feature: Operation support
+     """
+     {
+       "@context": "/contexts/EmbeddedDummy",
+-      "@id": "/embedded_dummies/1",
++      "@id": "/embedded_dummies_groups/1",
+       "@type": "EmbeddedDummy",
+       "name": "Dummy #1",
+       "embeddedDummy": {
+@@ -76,7 +76,7 @@ Feature: Operation support
+     """
+     {
+         "@context": "/contexts/Book",
+-        "@id": "/books/1",
++        "@id": "/books/by_isbn/9780451524935",
+         "@type": "Book",
+         "name": "1984",
+         "isbn": "9780451524935",
+diff --git a/features/main/subresource.feature b/features/main/subresource.feature
+index ae4969a5a..2fd278ea0 100644
+--- a/features/main/subresource.feature
++++ b/features/main/subresource.feature
+@@ -13,7 +13,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/Answer",
+-      "@id": "/answers/1",
++      "@id": "/questions/1/answer",
+       "@type": "Answer",
+       "id": 1,
+       "content": "42",
+@@ -234,7 +234,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/RelatedDummy",
+-      "@id": "/related_dummies/2",
++      "@id": "/dummies/1/related_dummies/2",
+       "@type": "https://schema.org/Product",
+       "id": 2,
+       "name": null,
+@@ -274,7 +274,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/ThirdLevel",
+-      "@id": "/third_levels/1",
++      "@id": "/dummies/1/related_dummies/1/third_level",
+       "@type": "ThirdLevel",
+       "fourthLevel": "/fourth_levels/1",
+       "badFourthLevel": null,
+@@ -293,7 +293,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/FourthLevel",
+-      "@id": "/fourth_levels/1",
++      "@id": "/dummies/1/related_dummies/1/third_level/fourth_level",
+       "@type": "FourthLevel",
+       "badThirdLevel": [],
+       "id": 1,
+@@ -411,7 +411,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/Dummy",
+-      "@id": "/dummies/1",
++      "@id": "/related_owned_dummies/1/owning_dummy",
+       "@type": "Dummy",
+       "description": null,
+       "dummy": null,
+@@ -444,7 +444,7 @@ Feature: Subresource support
+     """
+     {
+       "@context": "/contexts/Dummy",
+-      "@id": "/dummies/1",
++      "@id": "/related_owning_dummies/1/owned_dummy",
+       "@type": "Dummy",
+       "description": null,
+       "dummy": null,

.github/workflows/ci.yml

@@ -818,8 +818,11 @@ jobs:
       - name: Convert metadata to API Platform 3
         run: |
             tests/Fixtures/app/console api:upgrade-resource -f
-      - name: Clear test app cache
-        run: rm -Rf tests/Fixtures/app/var/cache/*
+      - name: Apply behat features patch (IRIs update)
+        run: |
+            git apply .github/patch/v3_features.patch
+      - name: clear test app cache
+        run: rm -rf tests/Fixtures/app/var/cache/*
       - name: Run Behat tests
         run: |
           mkdir -p build/logs/behat
@@ -901,6 +904,9 @@ jobs:
       - name: Convert metadata to API Platform 3
         run: |
             tests/Fixtures/app/console api:upgrade-resource -f
+      - name: Apply behat features patch (IRIs update)
+        run: |
+            git apply .github/patch/v3_features.patch
       - name: Clear test app cache
         run: rm -Rf tests/Fixtures/app/var/cache/*
       - name: Run Behat tests

features/main/crud_uri_variables.feature

@@ -169,7 +169,7 @@ Feature: Uri Variables
     """
     {
         "@context": "/contexts/Company",
-        "@id": "/companies/1",
+        "@id": "/employees/1/company",
         "@type": "Company",
         "id": 1,
         "name": "Foo Company 1",

src/Api/IdentifiersExtractor.php

@@ -17,10 +17,12 @@
 use ApiPlatform\Exception\RuntimeException;
 use ApiPlatform\Metadata\GraphQl\Operation as GraphQlOperation;
 use ApiPlatform\Metadata\HttpOperation;
+use ApiPlatform\Metadata\Operation;
 use ApiPlatform\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
 use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;
 use ApiPlatform\Util\ResourceClassInfoTrait;
+use Symfony\Component\PropertyAccess\Exception\NoSuchPropertyException;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 
@@ -51,11 +53,11 @@ public function __construct(ResourceMetadataCollectionFactoryInterface $resource
      *
      * TODO: 3.0 identifiers should be stringable?
      */
-    public function getIdentifiersFromItem($item, string $operationName = null, array $context = []): array
+    public function getIdentifiersFromItem($item, Operation $operation = null, array $context = []): array
     {
         $identifiers = [];
         $resourceClass = $this->getResourceClass($item, true);
-        $operation = $context['operation'] ?? $this->resourceMetadataFactory->create($resourceClass)->getOperation($operationName, false, true);
+        $operation = $operation ?? $this->resourceMetadataFactory->create($resourceClass)->getOperation(null, false, true);
 
         if ($operation instanceof HttpOperation) {
             $links = $operation->getUriVariables();
@@ -100,16 +102,20 @@ private function getIdentifierValue($item, string $class, string $property, stri
                 continue;
             }
 
-            if ($type->isCollection()) {
-                $collectionValueType = $type->getCollectionValueTypes()[0] ?? null;
+            try {
+                if ($type->isCollection()) {
+                    $collectionValueType = $type->getCollectionValueTypes()[0] ?? null;
 
-                if (null !== $collectionValueType && $collectionValueType->getClassName() === $class) {
-                    return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, sprintf('%s[0].%s', $propertyName, $property)), $parameterName);
+                    if (null !== $collectionValueType && $collectionValueType->getClassName() === $class) {
+                        return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, sprintf('%s[0].%s', $propertyName, $property)), $parameterName);
+                    }
                 }
-            }
 
-            if ($type->getClassName() === $class) {
-                return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$propertyName.$property"), $parameterName);
+                if ($type->getClassName() === $class) {
+                    return $this->resolveIdentifierValue($this->propertyAccessor->getValue($item, "$propertyName.$property"), $parameterName);
+                }
+            } catch (NoSuchPropertyException $e) {
+                throw new RuntimeException('Not able to retrieve identifiers.', $e->getCode(), $e);
             }
         }
 

src/Api/IdentifiersExtractorInterface.php

@@ -14,6 +14,7 @@
 namespace ApiPlatform\Api;
 
 use ApiPlatform\Exception\RuntimeException;
+use ApiPlatform\Metadata\Operation;
 
 /**
  * Extracts identifiers for a given Resource according to the retrieved Metadata.
@@ -29,5 +30,5 @@ interface IdentifiersExtractorInterface
      *
      * @throws RuntimeException
      */
-    public function getIdentifiersFromItem($item, ?string $operationName = null, array $context = []): array;
+    public function getIdentifiersFromItem($item, ?Operation $operation = null, array $context = []): array;
 }

src/Api/IriConverterInterface.php

@@ -16,6 +16,7 @@
 use ApiPlatform\Exception\InvalidArgumentException;
 use ApiPlatform\Exception\ItemNotFoundException;
 use ApiPlatform\Exception\RuntimeException;
+use ApiPlatform\Metadata\Operation;
 
 /**
  * Converts item and resources to IRI and vice versa.
@@ -32,22 +33,15 @@ interface IriConverterInterface
      *
      * @return object
      */
-    public function getItemFromIri(string $iri, array $context = []);
+    public function getResourceFromIri(string $iri, array $context = [], ?Operation $operation = null);
 
     /**
      * Gets the IRI associated with the given item.
      *
-     * @param object $item
+     * @param object|class-string $resource
      *
      * @throws InvalidArgumentException
      * @throws RuntimeException
      */
-    public function getIriFromItem($item, string $operationName = null, int $referenceType = UrlGeneratorInterface::ABS_PATH, array $context = []): string;
-
-    /**
-     * Gets the IRI associated with the given resource collection.
-     *
-     * @throws InvalidArgumentException
-     */
-    public function getIriFromResourceClass(string $resourceClass, string $operationName = null, int $referenceType = UrlGeneratorInterface::ABS_PATH, array $context = []): string;
+    public function getIriFromResource($resource, int $referenceType = UrlGeneratorInterface::ABS_PATH, ?Operation $operation = null, array $context = []): ?string;
 }

src/Core/EventListener/WriteListener.php

@@ -14,7 +14,7 @@
 namespace ApiPlatform\Core\EventListener;
 
 use ApiPlatform\Api\IriConverterInterface;
-use ApiPlatform\Core\Api\IriConverterInterface as IriConverterLegacyInterface;
+use ApiPlatform\Core\Api\IriConverterInterface as LegacyIriConverterInterface;
 use ApiPlatform\Core\Api\ResourceClassResolverInterface;
 use ApiPlatform\Core\DataPersister\DataPersisterInterface;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
@@ -43,7 +43,7 @@ final class WriteListener
     public const OPERATION_ATTRIBUTE_KEY = 'write';
 
     private $dataPersister;
-    /** @var IriConverterLegacyInterface|IriConverterInterface|null */
+    /** @var LegacyIriConverterInterface|IriConverterInterface|null */
     private $iriConverter;
     private $metadataBackwardCompatibilityLayer;
 
@@ -121,7 +121,7 @@ public function onKernelView(ViewEvent $event): void
                 }
 
                 if ($this->isResourceClass($this->getObjectClass($controllerResult))) {
-                    $request->attributes->set('_api_write_item_iri', $this->iriConverter->getIriFromItem($controllerResult));
+                    $request->attributes->set('_api_write_item_iri', $this->iriConverter instanceof LegacyIriConverterInterface ? $this->iriConverter->getIriFromItem($controllerResult) : $this->iriConverter->getIriFromResource($controllerResult));
                 }
 
                 break;

src/Core/Swagger/Serializer/DocumentationNormalizer.php

@@ -395,7 +395,6 @@ private function updateGetOperation(bool $v3, \ArrayObject $pathOperation, array
 
             if (
                 ($resourceMetadata->getAttributes()['extra_properties']['is_legacy_subresource'] ?? false) ||
-                ($resourceMetadata->getAttributes()['extra_properties']['legacy_subresource_behavior'] ?? false) ||
                 ($resourceMetadata->getAttributes()['extra_properties']['is_alternate_resource_metadata'] ?? false)) {
                 // Avoid duplicates parameters when there is a filter on a subresource identifier
                 $parametersMemory = [];

src/Doctrine/Common/Filter/SearchFilterTrait.php

@@ -30,6 +30,9 @@ trait SearchFilterTrait
 {
     use PropertyHelperTrait;
 
+    /**
+     * @var LegacyIriConverterInterface|IriConverterInterface
+     */
     protected $iriConverter;
     protected $propertyAccessor;
     protected $identifiersExtractor;
@@ -124,7 +127,8 @@ abstract protected function normalizePropertyName($property);
     protected function getIdFromValue(string $value)
     {
         try {
-            $item = $this->getIriConverter()->getItemFromIri($value, ['fetch_data' => false]);
+            $iriConverter = $this->getIriConverter();
+            $item = $iriConverter instanceof LegacyIriConverterInterface ? $iriConverter->getItemFromIri($value, ['fetch_data' => false]) : $iriConverter->getResourceFromIri($value, ['fetch_data' => false]); // @phpstan-ignore-line bc-compatibility inside a trait
 
             return $this->getPropertyAccessor()->getValue($item, 'id');
         } catch (InvalidArgumentException $e) {

src/Doctrine/EventListener/PublishMercureUpdatesListener.php

@@ -100,7 +100,7 @@ public function __construct(ResourceClassResolverInterface $resourceClassResolve
                 new ExpressionFunction('iri', static function (string $apiResource, int $referenceType = UrlGeneratorInterface::ABS_URL): string {
                     return sprintf('iri(%s, %d)', $apiResource, $referenceType);
                 }, static function (array $arguments, $apiResource, int $referenceType = UrlGeneratorInterface::ABS_URL) use ($iriConverter): string {
-                    return $iriConverter->getIriFromItem($apiResource, null, $referenceType);
+                    return $iriConverter->getIriFromResource($apiResource, $referenceType);
                 })
             );
         }
@@ -245,8 +245,8 @@ private function storeObjectToPublish($object, string $property): void
 
         if ('deletedObjects' === $property) {
             $this->deletedObjects[(object) [
-                'id' => $this->iriConverter->getIriFromItem($object),
-                'iri' => $this->iriConverter->getIriFromItem($object, null, UrlGeneratorInterface::ABS_URL),
+                'id' => $this->iriConverter->getIriFromResource($object),
+                'iri' => $this->iriConverter->getIriFromResource($object, UrlGeneratorInterface::ABS_URL),
             ]] = $options;
 
             return;
@@ -271,7 +271,7 @@ private function publishUpdate($object, array $options, string $type): void
             $resourceClass = $this->getObjectClass($object);
             $context = $options['normalization_context'] ?? $this->resourceMetadataFactory->create($resourceClass)->getOperation()->getNormalizationContext() ?? [];
 
-            $iri = $options['topics'] ?? $this->iriConverter->getIriFromItem($object, null, UrlGeneratorInterface::ABS_URL);
+            $iri = $options['topics'] ?? $this->iriConverter->getIriFromResource($object, UrlGeneratorInterface::ABS_URL);
             $data = $options['data'] ?? $this->serializer->serialize($object, key($this->formats), $context);
         }