Fix ResourceClassResolver handling of inheritance

Author: teohhanhui

features/main/operation.feature

@@ -4,7 +4,6 @@ Feature: Operation support
   I need to be able to add custom operations and remove built-in ones
 
   @createSchema
-  @dropSchema
   Scenario: Can not write readonly property
     When I add "Content-Type" header equal to "application/ld+json"
     And I send a "POST" request to "/readable_only_properties" with body:

features/main/relation.feature

@@ -491,7 +491,7 @@ Feature: Relations support
     Given there are people having pets
     When I add "Content-Type" header equal to "application/ld+json"
     And I send a "GET" request to "/people"
-    And the response status code should be 200
+    Then the response status code should be 200
     And the response should be in JSON
     And the JSON should be equal to:
     """
@@ -621,8 +621,6 @@ Feature: Relations support
     }
     """
 
-
-  @dropSchema
   Scenario: Passing an invalid IRI to a relation
     When I add "Content-Type" header equal to "application/ld+json"
     And I send a "POST" request to "/relation_embedders" with body:
@@ -634,7 +632,7 @@ Feature: Relations support
     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 value provided (invalid IRI?)."
+    And the JSON node "hydra:description" should contain 'Invalid IRI "certainly not an iri and not a plain identifier".'
 
   Scenario: Passing an invalid type to a relation
     When I add "Content-Type" header equal to "application/ld+json"
@@ -647,4 +645,32 @@ Feature: Relations support
     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 value provided (invalid IRI?)."
+    And the JSON should be valid according to this schema:
+    """
+    {
+      "type": "object",
+      "properties": {
+        "@context": {
+          "type": "string",
+          "pattern": "^/contexts/Error$"
+        },
+        "@type": {
+          "type": "string",
+          "pattern": "^hydra:Error$"
+        },
+        "hydra:title": {
+          "type": "string",
+          "pattern": "^An error occurred$"
+        },
+        "hydra:description": {
+          "pattern": "^Expected IRI or document for resource \"ApiPlatform\\\\Core\\\\Tests\\\\Fixtures\\\\TestBundle\\\\(Document|Entity)\\\\RelatedDummy\", \"integer\" given.$"
+        }
+      },
+      "required": [
+        "@context",
+        "@type",
+        "hydra:title",
+        "hydra:description"
+      ]
+    }
+    """

features/security/strong_typing.feature

@@ -73,7 +73,7 @@ Feature: Handle properly invalid data submitted to the API
     And the JSON node "@context" should be equal to "/contexts/Error"
     And the JSON node "@type" should be equal to "hydra:Error"
     And the JSON node "hydra:title" should be equal to "An error occurred"
-    And the JSON node "hydra:description" should be equal to 'Expected IRI or nested document for attribute "relatedDummy", "string" given.'
+    And the JSON node "hydra:description" should be equal to 'Invalid IRI "1".'
     And the JSON node "trace" should exist
 
   Scenario: Ignore invalid dates

features/serializer/vo_relations.feature

@@ -26,28 +26,28 @@ Feature: Value object as ApiResource
     Then the response status code should be 201
     And the JSON should be equal to:
     """
-     {
-         "@context": "/contexts/VoDummyCar",
-         "@id": "/vo_dummy_cars/1",
-         "@type": "VoDummyCar",
-         "mileage": 1500,
-         "bodyType": "suv",
-         "inspections": [],
-         "make": "CustomCar",
-         "insuranceCompany": {
-             "@id": "/vo_dummy_insurance_companies/1",
-             "@type": "VoDummyInsuranceCompany",
-             "name": "Safe Drive Company"
-         },
-         "drivers": [
-             {
-                 "@id": "/vo_dummy_drivers/1",
-                 "@type": "VoDummyDriver",
-                 "firstName": "John",
-                 "lastName": "Doe"
-             }
-         ]
-     }
+    {
+        "@context": "/contexts/VoDummyCar",
+        "@id": "/vo_dummy_cars/1",
+        "@type": "VoDummyCar",
+        "mileage": 1500,
+        "bodyType": "suv",
+        "inspections": [],
+        "make": "CustomCar",
+        "insuranceCompany": {
+            "@id": "/vo_dummy_insurance_companies/1",
+            "@type": "VoDummyInsuranceCompany",
+            "name": "Safe Drive Company"
+        },
+        "drivers": [
+            {
+                "@id": "/vo_dummy_drivers/1",
+                "@type": "VoDummyDriver",
+                "firstName": "John",
+                "lastName": "Doe"
+            }
+        ]
+    }
     """
     And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
 
@@ -98,8 +98,7 @@ Feature: Value object as ApiResource
         "@type": "VoDummyInspection",
         "accepted": true,
         "car": "/vo_dummy_cars/1",
-        "performed": "2018-08-24T00:00:00+00:00",
-        "id": 1
+        "performed": "2018-08-24T00:00:00+00:00"
     }
     """
 
@@ -117,27 +116,36 @@ Feature: Value object as ApiResource
     }
     """
     Then the response status code should be 400
+    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
     And the JSON should be valid according to this schema:
     """
     {
       "type": "object",
       "properties": {
         "@context": {
-          "enum": ["/contexts/Error"]
+          "type": "string",
+          "pattern": "^/contexts/Error$"
         },
-        "type": {
-          "enum": ["hydra:Error"]
+        "@type": {
+          "type": "string",
+          "pattern": "^hydra:Error$"
         },
         "hydra:title": {
-          "enum": ["An error occurred"]
+          "type": "string",
+          "pattern": "^An error occurred$"
         },
         "hydra:description": {
           "pattern": "^Cannot create an instance of ApiPlatform\\\\Core\\\\Tests\\\\Fixtures\\\\TestBundle\\\\(Document|Entity)\\\\VoDummyCar from serialized data because its constructor requires parameter \"drivers\" to be present.$"
         }
-      }
+      },
+      "required": [
+        "@context",
+        "@type",
+        "hydra:title",
+        "hydra:description"
+      ]
     }
     """
-    And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
 
   @createSchema
   Scenario: Create Value object without default param

src/Api/CachedIdentifiersExtractor.php

@@ -13,7 +13,7 @@
 
 namespace ApiPlatform\Core\Api;
 
-use ApiPlatform\Core\Util\ClassInfoTrait;
+use ApiPlatform\Core\Util\ResourceClassInfoTrait;
 use Psr\Cache\CacheException;
 use Psr\Cache\CacheItemPoolInterface;
 use Symfony\Component\PropertyAccess\PropertyAccess;
@@ -26,14 +26,13 @@
  */
 final class CachedIdentifiersExtractor implements IdentifiersExtractorInterface
 {
-    use ClassInfoTrait;
+    use ResourceClassInfoTrait;
 
     public const CACHE_KEY_PREFIX = 'iri_identifiers';
 
     private $cacheItemPool;
     private $propertyAccessor;
     private $decorated;
-    private $resourceClassResolver;
     private $localCache = [];
     private $localResourceCache = [];
 
@@ -82,9 +81,7 @@ public function getIdentifiersFromItem($item): array
                 continue;
             }
 
-            $relatedResourceClass = $this->getObjectClass($identifiers[$propertyName]);
-
-            if (null !== $this->resourceClassResolver && !$this->resourceClassResolver->isResourceClass($relatedResourceClass)) {
+            if (null === $relatedResourceClass = $this->getResourceClass($identifiers[$propertyName])) {
                 continue;
             }
 

src/Api/IdentifiersExtractor.php

@@ -16,7 +16,7 @@
 use ApiPlatform\Core\Exception\RuntimeException;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
 use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
-use ApiPlatform\Core\Util\ClassInfoTrait;
+use ApiPlatform\Core\Util\ResourceClassInfoTrait;
 use Symfony\Component\PropertyAccess\PropertyAccess;
 use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
 
@@ -27,12 +27,11 @@
  */
 final class IdentifiersExtractor implements IdentifiersExtractorInterface
 {
-    use ClassInfoTrait;
+    use ResourceClassInfoTrait;
 
     private $propertyNameCollectionFactory;
     private $propertyMetadataFactory;
     private $propertyAccessor;
-    private $resourceClassResolver;
 
     public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, PropertyAccessorInterface $propertyAccessor = null, ResourceClassResolverInterface $resourceClassResolver = null)
     {
@@ -67,7 +66,8 @@ public function getIdentifiersFromResourceClass(string $resourceClass): array
     public function getIdentifiersFromItem($item): array
     {
         $identifiers = [];
-        $resourceClass = $this->getObjectClass($item);
+        $resourceClass = $this->getResourceClass($item, true);
+
         foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) {
             $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName);
             $identifier = $propertyMetadata->isIdentifier();
@@ -81,9 +81,7 @@ public function getIdentifiersFromItem($item): array
                 continue;
             }
 
-            $relatedResourceClass = $this->getObjectClass($identifier);
-
-            if (null !== $this->resourceClassResolver && !$this->resourceClassResolver->isResourceClass($relatedResourceClass)) {
+            if (null === $relatedResourceClass = $this->getResourceClass($identifier)) {
                 continue;
             }
 

src/Api/ResourceClassResolver.php

@@ -40,33 +40,45 @@ public function __construct(ResourceNameCollectionFactoryInterface $resourceName
      */
     public function getResourceClass($value, string $resourceClass = null, bool $strict = false): string
     {
-        $type = \is_object($value) && !$value instanceof \Traversable ? $this->getObjectClass($value) : $resourceClass;
-        $resourceClass = $resourceClass ?? $type;
+        if ($strict && null === $resourceClass) {
+            throw new InvalidArgumentException('Strict checking is only possible when resource class is specified.');
+        }
+
+        $actualClass = \is_object($value) && !$value instanceof \Traversable ? $this->getObjectClass($value) : null;
 
-        if (null === $resourceClass) {
-            throw new InvalidArgumentException(sprintf('No resource class found.'));
+        if (null === $actualClass && null === $resourceClass) {
+            throw new InvalidArgumentException('Resource type could not be determined. Resource class must be specified.');
         }
 
-        if (
-            null === $type
-            || ((!$strict || $resourceClass === $type) && $isResourceClass = $this->isResourceClass($type))
-        ) {
+        if (null !== $resourceClass && !$this->isResourceClass($resourceClass)) {
+            throw new InvalidArgumentException(sprintf('Specified class "%s" is not a resource class.', $resourceClass));
+        }
+
+        if (null === $actualClass) {
             return $resourceClass;
         }
 
-        // The Resource is an interface
-        if ($value instanceof $resourceClass && $type !== $resourceClass && interface_exists($resourceClass)) {
-            throw new InvalidArgumentException(sprintf('The given object\'s resource is the interface "%s", finding a class is not possible.', $resourceClass));
+        if ($strict && !is_a($actualClass, $resourceClass, true)) {
+            throw new InvalidArgumentException(sprintf('Object of type "%s" does not match "%s" resource class.', $actualClass, $resourceClass));
+        }
+
+        $mostSpecificResourceClass = null;
+
+        foreach ($this->resourceNameCollectionFactory->create() as $resourceClassName) {
+            if (!is_a($actualClass, $resourceClassName, true)) {
+                continue;
+            }
+
+            if (null === $mostSpecificResourceClass || is_subclass_of($resourceClassName, $mostSpecificResourceClass, true)) {
+                $mostSpecificResourceClass = $resourceClassName;
+            }
         }
 
-        if (
-            ($isResourceClass ?? $this->isResourceClass($type))
-            || (is_subclass_of($type, $resourceClass) && $this->isResourceClass($resourceClass))
-        ) {
-            return $type;
+        if (null === $mostSpecificResourceClass) {
+            throw new InvalidArgumentException(sprintf('No resource class found for object of type "%s".', $actualClass));
         }
 
-        throw new InvalidArgumentException(sprintf('No resource class found for object of type "%s".', $type));
+        return $mostSpecificResourceClass;
     }
 
     /**
@@ -79,7 +91,7 @@ public function isResourceClass(string $type): bool
         }
 
         foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) {
-            if ($type === $resourceClass) {
+            if (is_a($type, $resourceClass, true)) {
                 return $this->localIsResourceClassCache[$type] = true;
             }
         }

src/Api/ResourceClassResolverInterface.php

@@ -25,6 +25,9 @@ interface ResourceClassResolverInterface
     /**
      * Guesses the associated resource.
      *
+     * @param string $resourceClass The expected resource class
+     * @param bool   $strict        If true, value must match the expected resource class
+     *
      * @throws InvalidArgumentException
      */
     public function getResourceClass($value, string $resourceClass = null, bool $strict = false): string;

src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php

@@ -19,7 +19,7 @@
 use ApiPlatform\Core\Exception\InvalidArgumentException;
 use ApiPlatform\Core\Exception\RuntimeException;
 use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
-use ApiPlatform\Core\Util\ClassInfoTrait;
+use ApiPlatform\Core\Util\ResourceClassInfoTrait;
 use Doctrine\ORM\Event\OnFlushEventArgs;
 use Symfony\Component\ExpressionLanguage\ExpressionLanguage;
 use Symfony\Component\Mercure\Update;
@@ -35,9 +35,8 @@
  */
 final class PublishMercureUpdatesListener
 {
-    use ClassInfoTrait;
+    use ResourceClassInfoTrait;
 
-    private $resourceClassResolver;
     private $iriConverter;
     private $resourceMetadataFactory;
     private $serializer;
@@ -120,8 +119,7 @@ private function reset(): void
      */
     private function storeEntityToPublish($entity, string $property): void
     {
-        $resourceClass = $this->getObjectClass($entity);
-        if (!$this->resourceClassResolver->isResourceClass($resourceClass)) {
+        if (null === $resourceClass = $this->getResourceClass($entity)) {
             return;
         }