Skip to content

Commit

Permalink
Fix ResourceClassResolver handling of inheritance
Browse files Browse the repository at this point in the history
  • Loading branch information
teohhanhui committed May 20, 2019
1 parent cb4f3cd commit a05a3e8
Show file tree
Hide file tree
Showing 48 changed files with 1,288 additions and 1,058 deletions.
1 change: 0 additions & 1 deletion features/main/operation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
36 changes: 31 additions & 5 deletions features/main/relation.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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:
"""
Expand Down Expand Up @@ -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:
Expand All @@ -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"
Expand All @@ -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"
]
}
"""
2 changes: 1 addition & 1 deletion features/security/strong_typing.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
68 changes: 38 additions & 30 deletions features/serializer/vo_relations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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"
}
"""

Expand All @@ -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
Expand Down
9 changes: 3 additions & 6 deletions src/Api/CachedIdentifiersExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = [];

Expand Down Expand Up @@ -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;
}

Expand Down
12 changes: 5 additions & 7 deletions src/Api/IdentifiersExtractor.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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)
{
Expand Down Expand Up @@ -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();
Expand All @@ -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;
}

Expand Down
48 changes: 30 additions & 18 deletions src/Api/ResourceClassResolver.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

/**
Expand All @@ -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;
}
}
Expand Down
3 changes: 3 additions & 0 deletions src/Api/ResourceClassResolverInterface.php
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -35,9 +35,8 @@
*/
final class PublishMercureUpdatesListener
{
use ClassInfoTrait;
use ResourceClassInfoTrait;

private $resourceClassResolver;
private $iriConverter;
private $resourceMetadataFactory;
private $serializer;
Expand Down Expand Up @@ -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;
}

Expand Down
Loading

0 comments on commit a05a3e8

Please sign in to comment.