From a5c1919336aba8960a325625b3b732ee8891e9a5 Mon Sep 17 00:00:00 2001 From: "Nek (Maxime Veber)" Date: Sun, 7 Apr 2019 11:26:21 +0200 Subject: [PATCH 1/3] Fix resource resolution for interface as resources Before this patch, when you configure an interface as resource the serializer was not understanding the implementation was a resource. It also fix the behavior of child of ApiResources that were not detected as it. --- features/main/table_inheritance.feature | 20 +++++++++++ src/Api/IdentifiersExtractor.php | 4 ++- src/Api/IriConverterInterface.php | 6 ++++ src/Api/ResourceClassResolver.php | 21 ++++++++---- src/Api/ResourceClassResolverInterface.php | 10 +++++- src/Api/ResourceIriConverterInterface.php | 38 +++++++++++++++++++++ src/Bridge/Symfony/Routing/IriConverter.php | 15 ++++++-- src/JsonLd/Serializer/ItemNormalizer.php | 2 +- tests/Api/ResourceClassResolverTest.php | 31 ++++++++++++++++- 9 files changed, 135 insertions(+), 12 deletions(-) create mode 100644 src/Api/ResourceIriConverterInterface.php diff --git a/features/main/table_inheritance.feature b/features/main/table_inheritance.feature index 3cf9c9e46f9..fbcf9e58a51 100644 --- a/features/main/table_inheritance.feature +++ b/features/main/table_inheritance.feature @@ -307,6 +307,16 @@ Feature: Table inheritance "items": { "type": "object", "properties": { + "@type": { + "type": "string", + "pattern": "^ResourceInterface$", + "required": "true" + }, + "@id": { + "type": "string", + "pattern": "^/resource_interfaces/", + "required": "true" + }, "foo": { "type": "string", "required": "true" @@ -338,6 +348,16 @@ Feature: Table inheritance "type": "string", "pattern": "ResourceInterface$" }, + "@id": { + "type": "string", + "pattern": "^/resource_interfaces", + "required": "true" + }, + "@type": { + "type": "string", + "pattern": "^ResourceInterface$", + "required": "true" + }, "foo": { "type": "string", "required": "true" diff --git a/src/Api/IdentifiersExtractor.php b/src/Api/IdentifiersExtractor.php index a36d3ac4154..31f493f7520 100644 --- a/src/Api/IdentifiersExtractor.php +++ b/src/Api/IdentifiersExtractor.php @@ -67,7 +67,9 @@ public function getIdentifiersFromResourceClass(string $resourceClass): array public function getIdentifiersFromItem($item): array { $identifiers = []; - $resourceClass = $this->getObjectClass($item); + $type = $this->getObjectClass($item); + $resourceClass = $this->resourceClassResolver->isResourceClass($type) ? $this->resourceClassResolver->getResourceClass($item) : $type; + foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) { $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName); $identifier = $propertyMetadata->isIdentifier(); diff --git a/src/Api/IriConverterInterface.php b/src/Api/IriConverterInterface.php index 65f1fbf759a..2a095c19540 100644 --- a/src/Api/IriConverterInterface.php +++ b/src/Api/IriConverterInterface.php @@ -21,6 +21,8 @@ * Converts item and resources to IRI and vice versa. * * @author Kévin Dunglas + * + * @deprecated in favor of ResourceIriConverterInterface to be removed in ApiPlatform 3.0 */ interface IriConverterInterface { @@ -41,6 +43,10 @@ public function getItemFromIri(string $iri, array $context = []); * * @throws InvalidArgumentException * @throws RuntimeException + * + * @return string Class name of resource + * + * @deprecated in favor of `ResourceIriConverterInterface::getIriFromItemWithResource` */ public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::ABS_PATH): string; diff --git a/src/Api/ResourceClassResolver.php b/src/Api/ResourceClassResolver.php index f75f58227d1..98e88fb1e4e 100644 --- a/src/Api/ResourceClassResolver.php +++ b/src/Api/ResourceClassResolver.php @@ -47,18 +47,27 @@ public function getResourceClass($value, string $resourceClass = null, bool $str throw new InvalidArgumentException(sprintf('No resource class found.')); } + $resolvedResourceClass = null; + foreach ($this->resourceNameCollectionFactory->create() as $currentResourceClass) { + if ($resourceClass === $currentResourceClass) { + $resolvedResourceClass = $currentResourceClass; + } + if (null === $resolvedResourceClass && is_subclass_of($type, $currentResourceClass)) { + $resolvedResourceClass = $currentResourceClass; + } + } + if (null !== $resolvedResourceClass) { + $resourceClass = $resolvedResourceClass; + } + if ( null === $type || ((!$strict || $resourceClass === $type) && $isResourceClass = $this->isResourceClass($type)) + || null !== $resolvedResourceClass && interface_exists($resourceClass) ) { 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 ( ($isResourceClass ?? $this->isResourceClass($type)) || (is_subclass_of($type, $resourceClass) && $this->isResourceClass($resourceClass)) @@ -79,7 +88,7 @@ public function isResourceClass(string $type): bool } foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) { - if ($type === $resourceClass) { + if ($type === $resourceClass || is_subclass_of($type, $resourceClass)) { return $this->localIsResourceClassCache[$type] = true; } } diff --git a/src/Api/ResourceClassResolverInterface.php b/src/Api/ResourceClassResolverInterface.php index 41efc5c9be8..1bf900bfec8 100644 --- a/src/Api/ResourceClassResolverInterface.php +++ b/src/Api/ResourceClassResolverInterface.php @@ -25,12 +25,20 @@ interface ResourceClassResolverInterface /** * Guesses the associated resource. * + * @param object $value Object you're playing with + * @param string $resourceClass Resource class it is supposed to be (could be parent class for instance) + * @param bool $strict value must be type of resource class given or it will return type + * * @throws InvalidArgumentException + * + * @return string Resolved resource class */ public function getResourceClass($value, string $resourceClass = null, bool $strict = false): string; /** - * Is the given class a resource class? + * Is the given class name an api platform resource? + * + * @param string $type FQCN of the resource */ public function isResourceClass(string $type): bool; } diff --git a/src/Api/ResourceIriConverterInterface.php b/src/Api/ResourceIriConverterInterface.php new file mode 100644 index 00000000000..ea2ea65e25c --- /dev/null +++ b/src/Api/ResourceIriConverterInterface.php @@ -0,0 +1,38 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Api; + +use ApiPlatform\Core\Exception\InvalidArgumentException; +use ApiPlatform\Core\Exception\RuntimeException; + +/** + * Converts item and resources to IRI and vice versa. + * + * @author Kévin Dunglas + */ +interface ResourceIriConverterInterface extends IriConverterInterface +{ + /** + * Gets the IRI associated with the given item and resource class. + * + * @param object $item + * @param string $resourceClass resource class to use for generation + * + * @throws InvalidArgumentException + * @throws RuntimeException + * + * @return string Class name of resource + */ + public function getIriFromItemWithResource($item, string $resourceClass, int $referenceType = UrlGeneratorInterface::ABS_PATH): string; +} diff --git a/src/Bridge/Symfony/Routing/IriConverter.php b/src/Bridge/Symfony/Routing/IriConverter.php index 83f378cdae0..a3629dea905 100644 --- a/src/Bridge/Symfony/Routing/IriConverter.php +++ b/src/Bridge/Symfony/Routing/IriConverter.php @@ -15,8 +15,8 @@ use ApiPlatform\Core\Api\IdentifiersExtractor; use ApiPlatform\Core\Api\IdentifiersExtractorInterface; -use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\OperationType; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\Api\UrlGeneratorInterface; use ApiPlatform\Core\DataProvider\ItemDataProviderInterface; use ApiPlatform\Core\DataProvider\OperationDataProviderTrait; @@ -40,7 +40,7 @@ * * @author Kévin Dunglas */ -final class IriConverter implements IriConverterInterface +final class IriConverter implements ResourceIriConverterInterface { use ClassInfoTrait; use OperationDataProviderTrait; @@ -112,10 +112,21 @@ public function getItemFromIri(string $iri, array $context = []) /** * {@inheritdoc} + * + * @deprecated use `getIriFromItemWithResource` instead to be removed in ApiPlatform 3.0 */ public function getIriFromItem($item, int $referenceType = UrlGeneratorInterface::ABS_PATH): string { $resourceClass = $this->getObjectClass($item); + + return $this->getIriFromItemWithResource($item, $resourceClass, $referenceType); + } + + /** + * {@inheritdoc} + */ + public function getIriFromItemWithResource($item, string $resourceClass, int $referenceType = UrlGeneratorInterface::ABS_PATH): string + { $routeName = $this->routeNameResolver->getRouteName($resourceClass, OperationType::ITEM); try { diff --git a/src/JsonLd/Serializer/ItemNormalizer.php b/src/JsonLd/Serializer/ItemNormalizer.php index 211789ff286..148c0386eba 100644 --- a/src/JsonLd/Serializer/ItemNormalizer.php +++ b/src/JsonLd/Serializer/ItemNormalizer.php @@ -72,7 +72,7 @@ public function normalize($object, $format = null, array $context = []) // Use resolved resource class instead of given resource class to support multiple inheritance child types $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); $context = $this->initContext($resourceClass, $context); - $iri = $this->iriConverter->getIriFromItem($object); + $iri = $this->iriConverter->getIriFromItemWithResource($object, $resourceClass); $context['iri'] = $iri; $context['api_normalize'] = true; diff --git a/tests/Api/ResourceClassResolverTest.php b/tests/Api/ResourceClassResolverTest.php index 0e8d6c5c492..025a43afa77 100644 --- a/tests/Api/ResourceClassResolverTest.php +++ b/tests/Api/ResourceClassResolverTest.php @@ -170,7 +170,7 @@ public function testGetResourceClassWithChildResource() $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); - $this->assertEquals(DummyTableInheritanceChild::class, $resourceClassResolver->getResourceClass($t, DummyTableInheritance::class)); + $this->assertEquals(DummyTableInheritance::class, $resourceClassResolver->getResourceClass($t, DummyTableInheritance::class)); } public function testGetResourceClassWithInterfaceResource() @@ -183,4 +183,33 @@ public function testGetResourceClassWithInterfaceResource() $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); $resourceClassResolver->getResourceClass($dummy, DummyResourceInterface::class, true); } + + public function testGetResourceClassWithoutSecondParameterIsAccurate() + { + $dummy = new DummyResourceImplementation(); + $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyResourceInterface::class]))->shouldBeCalled(); + + $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); + $this->assertEquals(DummyResourceInterface::class, $resourceClassResolver->getResourceClass($dummy)); + } + + public function testGetResourceInterfaceWithResourceClassParameterReturnTheResouceClass() + { + $dummy = new DummyResourceImplementation(); + $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyResourceInterface::class]))->shouldBeCalled(); + + $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); + $this->assertEquals(DummyResourceInterface::class, $resourceClassResolver->getResourceClass($dummy, DummyResourceInterface::class)); + } + + public function testInterfaceCanBeResourceClass() + { + $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyResourceInterface::class]))->shouldBeCalled(); + + $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); + $this->assertTrue($resourceClassResolver->isResourceClass(DummyResourceImplementation::class)); + } } From 686d35d24380a617db9b43893d84d735fb0d1842 Mon Sep 17 00:00:00 2001 From: "Nek (Maxime Veber)" Date: Sun, 7 Apr 2019 15:25:51 +0200 Subject: [PATCH 2/3] Rework how resource class resolver use inheritance This commits comes with many changes: 1. The resource class resolver now works well with inheritance and is able to resolve resources using the ApiPlatform resource list (more clever) 2. This change adds a new feature: inherited items from api resources now are working as an api, a new behat test proves it 3. some parts of api platform were not compatible because it does not uses the resource resolver but "get class" to get the resource, theses parts are also fixed Notice: the cache from the resource class resolver is removed here. It was introduced in 2015 and was probably a great improvement, but since 2016 resources are not re-genarated on each call to the factory but we call instead a cached factory. This cache was just adding complexity so I removed it. --- features/bootstrap/DoctrineContext.php | 21 ++++ features/main/table_inheritance.feature | 114 +++++++++++++++--- src/Api/IdentifiersExtractor.php | 6 +- src/Api/ResourceClassResolver.php | 36 ++++-- src/Api/ResourceClassResolverInterface.php | 2 +- src/Api/ResourceIriConverterInterface.php | 2 +- .../PublishMercureUpdatesListener.php | 2 + tests/Api/IdentifiersExtractorTest.php | 1 + tests/Api/ResourceClassResolverTest.php | 28 ++++- .../PublishMercureUpdatesListenerTest.php | 3 + .../Document/DummyTableInheritance.php | 7 +- ...mmyTableInheritanceNotApiResourceChild.php | 45 +++++++ .../Entity/DummyTableInheritance.php | 7 +- ...mmyTableInheritanceNotApiResourceChild.php | 45 +++++++ .../JsonLd/Serializer/ItemNormalizerTest.php | 10 +- 15 files changed, 287 insertions(+), 42 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Document/DummyTableInheritanceNotApiResourceChild.php create mode 100644 tests/Fixtures/TestBundle/Entity/DummyTableInheritanceNotApiResourceChild.php diff --git a/features/bootstrap/DoctrineContext.php b/features/bootstrap/DoctrineContext.php index cd03b6024d3..67b7949b7cc 100644 --- a/features/bootstrap/DoctrineContext.php +++ b/features/bootstrap/DoctrineContext.php @@ -31,6 +31,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyOffer as DummyOfferDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyProduct as DummyProductDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyProperty as DummyPropertyDocument; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\DummyTableInheritanceNotApiResourceChild as DummyTableInheritanceNotApiResourceChildDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\EmbeddableDummy as EmbeddableDummyDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\EmbeddedDummy as EmbeddedDummyDocument; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Document\FileConfigDummy as FileConfigDummyDocument; @@ -74,6 +75,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyOffer; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProduct; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProperty; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceNotApiResourceChild; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy; @@ -165,6 +167,17 @@ public function thereAreDummyObjects(int $nb) $this->manager->flush(); } + /** + * @When some dummy table inheritance data but not api resource child are created + */ + public function someDummyTableInheritanceDataButNotApiResourceChildAreCreated() + { + $dummy = $this->buildDummyTableInheritanceNotApiResourceChild(); + $dummy->setName('Foobarbaz inheritance'); + $this->manager->persist($dummy); + $this->manager->flush(); + } + /** * @Given there are :nb foo objects with fake names */ @@ -1272,6 +1285,14 @@ private function buildDummy() return $this->isOrm() ? new Dummy() : new DummyDocument(); } + /** + * @return DummyTableInheritanceNotApiResourceChild|DummyTableInheritanceNotApiResourceChildDocument + */ + private function buildDummyTableInheritanceNotApiResourceChild() + { + return $this->isOrm() ? new DummyTableInheritanceNotApiResourceChild() : new DummyTableInheritanceNotApiResourceChildDocument(); + } + /** * @return DummyAggregateOffer|DummyAggregateOfferDocument */ diff --git a/features/main/table_inheritance.feature b/features/main/table_inheritance.feature index fbcf9e58a51..7a5c568283a 100644 --- a/features/main/table_inheritance.feature +++ b/features/main/table_inheritance.feature @@ -20,15 +20,18 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceChild$" + "pattern": "^DummyTableInheritanceChild$", + "required": "true" }, "@context": { "type": "string", - "pattern": "^/contexts/DummyTableInheritanceChild$" + "pattern": "^/contexts/DummyTableInheritanceChild$", + "required": "true" }, "@id": { "type": "string", - "pattern": "^/dummy_table_inheritance_children/1$" + "pattern": "^/dummy_table_inheritance_children/1$", + "required": "true" }, "name": { "type": "string", @@ -61,7 +64,8 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceChild$" + "pattern": "^DummyTableInheritanceChild$", + "required": "true" }, "name": { "type": "string", @@ -80,7 +84,40 @@ Feature: Table inheritance } """ - @createSchema + Scenario: Some children not api resources are created in the app + When some dummy table inheritance data but not api resource child are created + And I send a "GET" request to "/dummy_table_inheritances" + 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 valid according to this schema: + """ + { + "type": "object", + "properties": { + "hydra:member": { + "type": "array", + "items": { + "type": "object", + "properties": { + "@type": { + "type": "string", + "pattern": "^DummyTableInheritance(Child)?$", + "required": "true" + }, + "name": { + "type": "string", + "required": "true" + } + } + }, + "minItems": 1 + } + }, + "required": ["hydra:member"] + } + """ + Scenario: Create a table inherited resource When I add "Content-Type" header equal to "application/ld+json" And I send a "POST" request to "/dummy_table_inheritance_children" with body: @@ -97,15 +134,18 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceChild$" + "pattern": "^DummyTableInheritanceChild$", + "required": "true" }, "@context": { "type": "string", - "pattern": "^/contexts/DummyTableInheritanceChild$" + "pattern": "^/contexts/DummyTableInheritanceChild$", + "required": "true" }, "@id": { "type": "string", - "pattern": "^/dummy_table_inheritance_children/1$" + "pattern": "^/dummy_table_inheritance_children/3$", + "required": "true" }, "name": { "type": "string", @@ -136,15 +176,18 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceDifferentChild$" + "pattern": "^DummyTableInheritanceDifferentChild$", + "required": "true" }, "@context": { "type": "string", - "pattern": "^/contexts/DummyTableInheritanceDifferentChild$" + "pattern": "^/contexts/DummyTableInheritanceDifferentChild$", + "required": "true" }, "@id": { "type": "string", - "pattern": "^/dummy_table_inheritance_different_children/2$" + "pattern": "^/dummy_table_inheritance_different_children/4$", + "required": "true" }, "name": { "type": "string", @@ -167,7 +210,7 @@ Feature: Table inheritance { "children": [ "/dummy_table_inheritance_children/1", - "/dummy_table_inheritance_different_children/2" + "/dummy_table_inheritance_different_children/4" ] } """ @@ -181,15 +224,18 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceRelated$" + "pattern": "^DummyTableInheritanceRelated$", + "required": "true" }, "@context": { "type": "string", - "pattern": "^/contexts/DummyTableInheritanceRelated$" + "pattern": "^/contexts/DummyTableInheritanceRelated$", + "required": "true" }, "@id": { "type": "string", - "pattern": "^/dummy_table_inheritance_relateds/1$" + "pattern": "^/dummy_table_inheritance_relateds/1$", + "required": "true" }, "children": { "items": { @@ -199,7 +245,8 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceChild$" + "pattern": "^DummyTableInheritanceChild$", + "required": "true" }, "name": { "type": "string", @@ -215,7 +262,21 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceDifferentChild$" + "pattern": "^DummyTableInheritance$", + "required": "true" + }, + "name": { + "type": "string", + "required": "true" + } + } + }, + { + "properties": { + "@type": { + "type": "string", + "pattern": "^DummyTableInheritanceDifferentChild$", + "required": "true" }, "name": { "type": "string", @@ -255,7 +316,8 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceChild$" + "pattern": "^DummyTableInheritanceChild$", + "required": "true" }, "name": { "type": "string", @@ -271,7 +333,21 @@ Feature: Table inheritance "properties": { "@type": { "type": "string", - "pattern": "^DummyTableInheritanceDifferentChild$" + "pattern": "^DummyTableInheritance$", + "required": "true" + }, + "name": { + "type": "string", + "required": "true" + } + } + }, + { + "properties": { + "@type": { + "type": "string", + "pattern": "^DummyTableInheritanceDifferentChild$", + "required": "true" }, "name": { "type": "string", diff --git a/src/Api/IdentifiersExtractor.php b/src/Api/IdentifiersExtractor.php index 31f493f7520..981fdb9defd 100644 --- a/src/Api/IdentifiersExtractor.php +++ b/src/Api/IdentifiersExtractor.php @@ -67,8 +67,10 @@ public function getIdentifiersFromResourceClass(string $resourceClass): array public function getIdentifiersFromItem($item): array { $identifiers = []; - $type = $this->getObjectClass($item); - $resourceClass = $this->resourceClassResolver->isResourceClass($type) ? $this->resourceClassResolver->getResourceClass($item) : $type; + $resourceClass = $this->getObjectClass($item); + if (null !== $this->resourceClassResolver) { // BC Layer + $resourceClass = $this->resourceClassResolver->isResourceClass($resourceClass) ? $this->resourceClassResolver->getResourceClass($item) : $resourceClass; + } foreach ($this->propertyNameCollectionFactory->create($resourceClass) as $propertyName) { $propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $propertyName); diff --git a/src/Api/ResourceClassResolver.php b/src/Api/ResourceClassResolver.php index 98e88fb1e4e..c2d961ec718 100644 --- a/src/Api/ResourceClassResolver.php +++ b/src/Api/ResourceClassResolver.php @@ -28,7 +28,6 @@ final class ResourceClassResolver implements ResourceClassResolverInterface use ClassInfoTrait; private $resourceNameCollectionFactory; - private $localIsResourceClassCache = []; public function __construct(ResourceNameCollectionFactoryInterface $resourceNameCollectionFactory) { @@ -56,20 +55,27 @@ public function getResourceClass($value, string $resourceClass = null, bool $str $resolvedResourceClass = $currentResourceClass; } } + if (null !== $resolvedResourceClass) { $resourceClass = $resolvedResourceClass; } + $typeIsStrictResourceClass = $this->isStrictResourceClass($type); + if ($strict && $typeIsStrictResourceClass) { + return $type; + } + + $typeIsResourceClass = $this->isResourceClass($type); if ( null === $type - || ((!$strict || $resourceClass === $type) && $isResourceClass = $this->isResourceClass($type)) - || null !== $resolvedResourceClass && interface_exists($resourceClass) + || (((!$strict || $resourceClass === $type)) && $typeIsResourceClass) + || null !== $resolvedResourceClass && (interface_exists($resourceClass) || !$typeIsStrictResourceClass) ) { return $resourceClass; } if ( - ($isResourceClass ?? $this->isResourceClass($type)) + $typeIsResourceClass || (is_subclass_of($type, $resourceClass) && $this->isResourceClass($resourceClass)) ) { return $type; @@ -83,16 +89,28 @@ public function getResourceClass($value, string $resourceClass = null, bool $str */ public function isResourceClass(string $type): bool { - if (isset($this->localIsResourceClassCache[$type])) { - return $this->localIsResourceClassCache[$type]; + foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) { + if (is_a($type, $resourceClass, true)) { + return true; + } } + return false; + } + + /** + * Same of isResourceClass but stricter: it ignores inheritance. + * + * @param string $type FQCN of an object + */ + private function isStrictResourceClass(string $type): bool + { foreach ($this->resourceNameCollectionFactory->create() as $resourceClass) { - if ($type === $resourceClass || is_subclass_of($type, $resourceClass)) { - return $this->localIsResourceClassCache[$type] = true; + if ($type === $resourceClass) { + return true; } } - return $this->localIsResourceClassCache[$type] = false; + return false; } } diff --git a/src/Api/ResourceClassResolverInterface.php b/src/Api/ResourceClassResolverInterface.php index 1bf900bfec8..1555b4f0086 100644 --- a/src/Api/ResourceClassResolverInterface.php +++ b/src/Api/ResourceClassResolverInterface.php @@ -25,7 +25,7 @@ interface ResourceClassResolverInterface /** * Guesses the associated resource. * - * @param object $value Object you're playing with + * @param mixed $value Object you're playing with * @param string $resourceClass Resource class it is supposed to be (could be parent class for instance) * @param bool $strict value must be type of resource class given or it will return type * diff --git a/src/Api/ResourceIriConverterInterface.php b/src/Api/ResourceIriConverterInterface.php index ea2ea65e25c..ec1a1f5dac3 100644 --- a/src/Api/ResourceIriConverterInterface.php +++ b/src/Api/ResourceIriConverterInterface.php @@ -19,7 +19,7 @@ /** * Converts item and resources to IRI and vice versa. * - * @author Kévin Dunglas + * @author Maxime Veber */ interface ResourceIriConverterInterface extends IriConverterInterface { diff --git a/src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php b/src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php index 805e2585baa..289cb6b4ef9 100644 --- a/src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php +++ b/src/Bridge/Doctrine/EventListener/PublishMercureUpdatesListener.php @@ -121,9 +121,11 @@ private function reset(): void private function storeEntityToPublish($entity, string $property): void { $resourceClass = $this->getObjectClass($entity); + if (!$this->resourceClassResolver->isResourceClass($resourceClass)) { return; } + $resourceClass = $this->resourceClassResolver->getResourceClass($entity); $value = $this->resourceMetadataFactory->create($resourceClass)->getAttribute('mercure', false); if (false === $value) { diff --git a/tests/Api/IdentifiersExtractorTest.php b/tests/Api/IdentifiersExtractorTest.php index 878f881f196..de5462afcc5 100644 --- a/tests/Api/IdentifiersExtractorTest.php +++ b/tests/Api/IdentifiersExtractorTest.php @@ -179,6 +179,7 @@ private function getResourceClassResolver() $resourceClassResolver->isResourceClass(Argument::type('string'))->will(function ($args) { return !(Uuid::class === $args[0]); }); + $resourceClassResolver->getResourceClass(Argument::any())->will(function ($args) {return \get_class($args[0]); }); return $resourceClassResolver->reveal(); } diff --git a/tests/Api/ResourceClassResolverTest.php b/tests/Api/ResourceClassResolverTest.php index 025a43afa77..a60fc6a2d3c 100644 --- a/tests/Api/ResourceClassResolverTest.php +++ b/tests/Api/ResourceClassResolverTest.php @@ -24,6 +24,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyCar; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyTableInheritance; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceChild; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyTableInheritanceNotApiResourceChild; use PHPUnit\Framework\TestCase; /** @@ -175,13 +176,12 @@ public function testGetResourceClassWithChildResource() public function testGetResourceClassWithInterfaceResource() { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage("The given object's resource is the interface \"ApiPlatform\Core\Tests\Fixtures\DummyResourceInterface\", finding a class is not possible."); $dummy = new DummyResourceImplementation(); $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyResourceInterface::class]))->shouldBeCalled(); $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); - $resourceClassResolver->getResourceClass($dummy, DummyResourceInterface::class, true); + $this->assertEquals(DummyResourceInterface::class, $resourceClassResolver->getResourceClass($dummy, DummyResourceInterface::class, true)); } public function testGetResourceClassWithoutSecondParameterIsAccurate() @@ -212,4 +212,26 @@ public function testInterfaceCanBeResourceClass() $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); $this->assertTrue($resourceClassResolver->isResourceClass(DummyResourceImplementation::class)); } + + public function testItResolveParentResourceClassOfChildrenOfResourceClasses() + { + $dummy = new DummyTableInheritanceNotApiResourceChild(); + $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyTableInheritance::class]))->shouldBeCalled(); + + $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); + $this->assertTrue($resourceClassResolver->isResourceClass(DummyTableInheritanceNotApiResourceChild::class)); + $this->assertEquals(DummyTableInheritance::class, $resourceClassResolver->getResourceClass($dummy)); + $this->assertEquals(DummyTableInheritance::class, $resourceClassResolver->getResourceClass($dummy, DummyTableInheritance::class, true)); + } + + public function testItResolveChildWhenParentAndChildrenAreResourcesAndIsStrict() + { + $dummy = new DummyTableInheritanceChild(); + $resourceNameCollectionFactoryProphecy = $this->prophesize(ResourceNameCollectionFactoryInterface::class); + $resourceNameCollectionFactoryProphecy->create()->willReturn(new ResourceNameCollection([DummyTableInheritance::class, DummyTableInheritanceChild::class]))->shouldBeCalled(); + + $resourceClassResolver = new ResourceClassResolver($resourceNameCollectionFactoryProphecy->reveal()); + $this->assertEquals(DummyTableInheritanceChild::class, $resourceClassResolver->getResourceClass($dummy, DummyTableInheritance::class, true)); + } } diff --git a/tests/Bridge/Doctrine/EventListener/PublishMercureUpdatesListenerTest.php b/tests/Bridge/Doctrine/EventListener/PublishMercureUpdatesListenerTest.php index 632338a845e..82d84d8735f 100644 --- a/tests/Bridge/Doctrine/EventListener/PublishMercureUpdatesListenerTest.php +++ b/tests/Bridge/Doctrine/EventListener/PublishMercureUpdatesListenerTest.php @@ -28,6 +28,7 @@ use Doctrine\ORM\Event\OnFlushEventArgs; use Doctrine\ORM\UnitOfWork; use PHPUnit\Framework\TestCase; +use Prophecy\Argument; use Symfony\Component\Mercure\Update; use Symfony\Component\Serializer\SerializerInterface; @@ -56,6 +57,7 @@ public function testPublishUpdate() $resourceClassResolverProphecy->isResourceClass(NotAResource::class)->willReturn(false); $resourceClassResolverProphecy->isResourceClass(DummyCar::class)->willReturn(true); $resourceClassResolverProphecy->isResourceClass(DummyFriend::class)->willReturn(true); + $resourceClassResolverProphecy->getResourceClass(Argument::type('object'))->will(function ($args) { return \get_class($args[0]); }); $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); $iriConverterProphecy->getIriFromItem($toInsert, UrlGeneratorInterface::ABS_URL)->willReturn('http://example.com/dummies/1')->shouldBeCalled(); @@ -136,6 +138,7 @@ public function testInvalidMercureAttribute() $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true); + $resourceClassResolverProphecy->getResourceClass(Argument::type('object'))->will(function ($args) { return \get_class($args[0]); }); $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); diff --git a/tests/Fixtures/TestBundle/Document/DummyTableInheritance.php b/tests/Fixtures/TestBundle/Document/DummyTableInheritance.php index 05b6b8f70c2..5ecbf5fe870 100644 --- a/tests/Fixtures/TestBundle/Document/DummyTableInheritance.php +++ b/tests/Fixtures/TestBundle/Document/DummyTableInheritance.php @@ -21,7 +21,12 @@ * @ODM\Document * @ODM\InheritanceType("SINGLE_COLLECTION") * @ODM\DiscriminatorField(value="discr") - * @ODM\DiscriminatorMap({"dummyTableInheritance"=DummyTableInheritance::class, "dummyTableInheritanceChild"=DummyTableInheritanceChild::class, "dummyTableInheritanceDifferentChild"=DummyTableInheritanceDifferentChild::class}) + * @ODM\DiscriminatorMap({ + * "dummyTableInheritance"=DummyTableInheritance::class, + * "dummyTableInheritanceChild"=DummyTableInheritanceChild::class, + * "dummyTableInheritanceDifferentChild"=DummyTableInheritanceDifferentChild::class, + * "dummyTableInheritanceNotApiResourceChild"=DummyTableInheritanceNotApiResourceChild::class + * }) * @ApiResource */ class DummyTableInheritance diff --git a/tests/Fixtures/TestBundle/Document/DummyTableInheritanceNotApiResourceChild.php b/tests/Fixtures/TestBundle/Document/DummyTableInheritanceNotApiResourceChild.php new file mode 100644 index 00000000000..7eb951f8e07 --- /dev/null +++ b/tests/Fixtures/TestBundle/Document/DummyTableInheritanceNotApiResourceChild.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Document; + +use Doctrine\ODM\MongoDB\Mapping\Annotations as ODM; + +/** + * @ODM\Document + */ +class DummyTableInheritanceNotApiResourceChild extends DummyTableInheritance +{ + /** + * @var bool The dummy swagg + * + * @ODM\Field(type="boolean") + */ + private $swaggerThanParent; + + public function __construct() + { + // Definitely always swagger than parents + $this->swaggerThanParent = true; + } + + public function isSwaggerThanParent(): bool + { + return $this->swaggerThanParent; + } + + public function setSwaggerThanParent(bool $swaggerThanParent) + { + $this->swaggerThanParent = $swaggerThanParent; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/DummyTableInheritance.php b/tests/Fixtures/TestBundle/Entity/DummyTableInheritance.php index 4a208281425..23b629d541d 100644 --- a/tests/Fixtures/TestBundle/Entity/DummyTableInheritance.php +++ b/tests/Fixtures/TestBundle/Entity/DummyTableInheritance.php @@ -21,7 +21,12 @@ * @ORM\Entity * @ORM\InheritanceType("JOINED") * @ORM\DiscriminatorColumn(name="discr", type="string") - * @ORM\DiscriminatorMap({"dummyTableInheritance"="DummyTableInheritance", "dummyTableInheritanceChild"="DummyTableInheritanceChild", "dummyTableInheritanceDifferentChild"="DummyTableInheritanceDifferentChild"}) + * @ORM\DiscriminatorMap({ + * "dummyTableInheritance"="DummyTableInheritance", + * "dummyTableInheritanceChild"="DummyTableInheritanceChild", + * "dummyTableInheritanceDifferentChild"="DummyTableInheritanceDifferentChild", + * "dummyTableInheritanceNotApiResourceChild"="DummyTableInheritanceNotApiResourceChild" + * }) * @ApiResource */ class DummyTableInheritance diff --git a/tests/Fixtures/TestBundle/Entity/DummyTableInheritanceNotApiResourceChild.php b/tests/Fixtures/TestBundle/Entity/DummyTableInheritanceNotApiResourceChild.php new file mode 100644 index 00000000000..0d6912e8a2b --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/DummyTableInheritanceNotApiResourceChild.php @@ -0,0 +1,45 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class DummyTableInheritanceNotApiResourceChild extends DummyTableInheritance +{ + /** + * @var bool The dummy swagg + * + * @ORM\Column(type="boolean") + */ + private $swaggerThanParent; + + public function __construct() + { + // Definitely always swagger than parents + $this->swaggerThanParent = true; + } + + public function isSwaggerThanParent(): bool + { + return $this->swaggerThanParent; + } + + public function setSwaggerThanParent(bool $swaggerThanParent) + { + $this->swaggerThanParent = $swaggerThanParent; + } +} diff --git a/tests/JsonLd/Serializer/ItemNormalizerTest.php b/tests/JsonLd/Serializer/ItemNormalizerTest.php index fb5b1bf984f..7ebf2de6644 100644 --- a/tests/JsonLd/Serializer/ItemNormalizerTest.php +++ b/tests/JsonLd/Serializer/ItemNormalizerTest.php @@ -13,8 +13,8 @@ namespace ApiPlatform\Core\Tests\JsonLd\Serializer; -use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\JsonLd\ContextBuilderInterface; use ApiPlatform\Core\JsonLd\Serializer\ItemNormalizer; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; @@ -42,7 +42,7 @@ public function testDontSupportDenormalization() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); $resourceClassResolverProphecy->getResourceClass(['dummy'], 'Dummy')->willReturn(Dummy::class); @@ -66,7 +66,7 @@ public function testSupportNormalization() $resourceMetadataFactoryProphecy = $this->prophesize(ResourceMetadataFactoryInterface::class); $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $contextBuilderProphecy = $this->prophesize(ContextBuilderInterface::class); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); @@ -103,8 +103,8 @@ public function testNormalize() $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn($propertyMetadata)->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1988')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, Argument::type('string'))->willReturn('/dummies/1988')->shouldBeCalled(); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); From 96d0556c9014fb8a49b89c24bead2ff2f7145d1f Mon Sep 17 00:00:00 2001 From: Maxime Veber Date: Fri, 26 Apr 2019 19:13:55 +0200 Subject: [PATCH 3/3] Add and fix tests about relation inheritance Previous commits fix the behavior with non api resources inherited from resource but relation management was still not ok since the update was not done on the abstract item normalizer. The resource class resolver is now used as it should in every normalizers. Most of this commit (tests) comes from @vincentchalamon and its PR #2760 --- features/bootstrap/DoctrineContext.php | 40 +++++++ features/main/table_inheritance.feature | 111 ++++++++++++++++++ src/Serializer/AbstractItemNormalizer.php | 11 +- .../TestBundle/Entity/AbstractUser.php | 92 +++++++++++++++ .../TestBundle/Entity/ExternalUser.php | 41 +++++++ .../TestBundle/Entity/InternalUser.php | 39 ++++++ tests/Fixtures/TestBundle/Entity/Site.php | 85 ++++++++++++++ .../GraphQl/Serializer/ItemNormalizerTest.php | 7 +- tests/Hal/Serializer/ItemNormalizerTest.php | 14 ++- .../Serializer/AbstractItemNormalizerTest.php | 17 +-- tests/Serializer/ItemNormalizerTest.php | 5 +- 11 files changed, 441 insertions(+), 21 deletions(-) create mode 100644 tests/Fixtures/TestBundle/Entity/AbstractUser.php create mode 100644 tests/Fixtures/TestBundle/Entity/ExternalUser.php create mode 100644 tests/Fixtures/TestBundle/Entity/InternalUser.php create mode 100644 tests/Fixtures/TestBundle/Entity/Site.php diff --git a/features/bootstrap/DoctrineContext.php b/features/bootstrap/DoctrineContext.php index 67b7949b7cc..bc9a83e1ec2 100644 --- a/features/bootstrap/DoctrineContext.php +++ b/features/bootstrap/DoctrineContext.php @@ -1561,4 +1561,44 @@ public function testEagerLoadingNotDuplicateRelation() $this->manager->flush(); $this->manager->clear(); } + + /** + * @Given there are :nb sites with internal owner + */ + public function thereAreSitesWithInternalOwner(int $nb) + { + for ($i = 1; $i <= $nb; ++$i) { + $internalUser = new \ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\InternalUser(); + $internalUser->setFirstname('Internal'); + $internalUser->setLastname('User'); + $internalUser->setEmail('john.doe@example.com'); + $internalUser->setInternalId('INT'); + $site = new \ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Site(); + $site->setTitle('title'); + $site->setDescription('description'); + $site->setOwner($internalUser); + $this->manager->persist($site); + } + $this->manager->flush(); + } + + /** + * @Given there are :nb sites with external owner + */ + public function thereAreSitesWithExternalOwner(int $nb) + { + for ($i = 1; $i <= $nb; ++$i) { + $externalUser = new \ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ExternalUser(); + $externalUser->setFirstname('External'); + $externalUser->setLastname('User'); + $externalUser->setEmail('john.doe@example.com'); + $externalUser->setExternalId('EXT'); + $site = new \ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Site(); + $site->setTitle('title'); + $site->setDescription('description'); + $site->setOwner($externalUser); + $this->manager->persist($site); + } + $this->manager->flush(); + } } diff --git a/features/main/table_inheritance.feature b/features/main/table_inheritance.feature index 7a5c568283a..0e7d573bc42 100644 --- a/features/main/table_inheritance.feature +++ b/features/main/table_inheritance.feature @@ -446,3 +446,114 @@ Feature: Table inheritance } } """ + + @!mongodb + Scenario: Generate iri from parent resource + Given there are 3 sites with internal owner + When I add "Content-Type" header equal to "application/ld+json" + And I send a "GET" request to "/sites" + 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 valid according to this schema: + """ + { + "type": "object", + "properties": { + "hydra:member": { + "type": "array", + "items": { + "type": "object", + "properties": { + "@type": { + "type": "string", + "pattern": "^Site$", + "required": "true" + }, + "@id": { + "type": "string", + "pattern": "^/sites/\\d+$", + "required": "true" + }, + "id": { + "type": "integer", + "required": "true" + }, + "title": { + "type": "string", + "required": "true" + }, + "description": { + "type": "string", + "required": "true" + }, + "owner": { + "type": "string", + "pattern": "^/custom_users/\\d+$", + "required": "true" + } + } + }, + "minItems": 3, + "maxItems": 3, + "required": "true" + } + } + } + """ + + @!mongodb + @createSchema + Scenario: Generate iri from current resource even if parent class is a resource + Given there are 3 sites with external owner + When I add "Content-Type" header equal to "application/ld+json" + And I send a "GET" request to "/sites" + 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 valid according to this schema: + """ + { + "type": "object", + "properties": { + "hydra:member": { + "type": "array", + "items": { + "type": "object", + "properties": { + "@type": { + "type": "string", + "pattern": "^Site$", + "required": "true" + }, + "@id": { + "type": "string", + "pattern": "^/sites/\\d+$", + "required": "true" + }, + "id": { + "type": "integer", + "required": "true" + }, + "title": { + "type": "string", + "required": "true" + }, + "description": { + "type": "string", + "required": "true" + }, + "owner": { + "type": "string", + "pattern": "^/external_users/\\d+$", + "required": "true" + } + } + }, + "minItems": 3, + "maxItems": 3, + "required": "true" + } + } + } + """ diff --git a/src/Serializer/AbstractItemNormalizer.php b/src/Serializer/AbstractItemNormalizer.php index 3fde229d47d..99b08ae4b00 100644 --- a/src/Serializer/AbstractItemNormalizer.php +++ b/src/Serializer/AbstractItemNormalizer.php @@ -129,7 +129,7 @@ public function normalize($object, $format = null, array $context = []) // Use resolved resource class instead of given resource class to support multiple inheritance child types $resourceClass = $this->resourceClassResolver->getResourceClass($object, $context['resource_class'] ?? null, true); $context = $this->initContext($resourceClass, $context); - $iri = $context['iri'] ?? $this->iriConverter->getIriFromItem($object); + $iri = $context['iri'] ?? $this->iriConverter->getIriFromItemWithResource($object, $resourceClass); $context['iri'] = $iri; $context['api_normalize'] = true; @@ -546,7 +546,8 @@ protected function getAttributeValue($object, $attribute, $format = null, array $type->isCollection() && ($collectionValueType = $type->getCollectionValueType()) && ($className = $collectionValueType->getClassName()) && - $this->resourceClassResolver->isResourceClass($className) + $this->resourceClassResolver->isResourceClass($className) && + ($className = $this->resourceClassResolver->getResourceClass($attributeValue, $className, true)) ) { return $this->normalizeCollectionOfRelations($propertyMetadata, $attributeValue, $className, $format, $this->createChildContext($context, $attribute)); } @@ -554,7 +555,8 @@ protected function getAttributeValue($object, $attribute, $format = null, array if ( $type && ($className = $type->getClassName()) && - $this->resourceClassResolver->isResourceClass($className) + $this->resourceClassResolver->isResourceClass($className) && + ($className = $this->resourceClassResolver->getResourceClass($attributeValue, $className, true)) ) { return $this->normalizeRelation($propertyMetadata, $attributeValue, $className, $format, $this->createChildContext($context, $attribute)); } @@ -606,7 +608,8 @@ protected function normalizeRelation(PropertyMetadata $propertyMetadata, $relate return $this->serializer->normalize($relatedObject, $format, $context); } - $iri = $this->iriConverter->getIriFromItem($relatedObject); + $iri = $this->iriConverter->getIriFromItemWithResource($relatedObject, $resourceClass); + if (isset($context['resources'])) { $context['resources'][$iri] = $iri; } diff --git a/tests/Fixtures/TestBundle/Entity/AbstractUser.php b/tests/Fixtures/TestBundle/Entity/AbstractUser.php new file mode 100644 index 00000000000..bcea76900ec --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/AbstractUser.php @@ -0,0 +1,92 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + * @ORM\InheritanceType("JOINED") + * @ApiResource( + * collectionOperations={ + * "get"={"path"="/custom_users"} + * }, + * itemOperations={ + * "get"={"path"="/custom_users/{id}"} + * } + * ) + */ +abstract class AbstractUser +{ + /** + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @ORM\Column + */ + private $firstname; + /** + * @ORM\Column + */ + private $lastname; + /** + * @ORM\Column + */ + private $email; + + public function getId(): ?int + { + return $this->id; + } + + public function getFirstname(): ?string + { + return $this->firstname; + } + + public function setFirstname(string $firstname): self + { + $this->firstname = $firstname; + + return $this; + } + + public function getLastname(): ?string + { + return $this->lastname; + } + + public function setLastname(string $lastname): self + { + $this->lastname = $lastname; + + return $this; + } + + public function getEmail(): ?string + { + return $this->email; + } + + public function setEmail(string $email): self + { + $this->email = $email; + + return $this; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/ExternalUser.php b/tests/Fixtures/TestBundle/Entity/ExternalUser.php new file mode 100644 index 00000000000..e98284e664c --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/ExternalUser.php @@ -0,0 +1,41 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + * @ApiResource + */ +class ExternalUser extends AbstractUser +{ + /** + * @ORM\Column + */ + private $externalId; + + public function getExternalId(): ?string + { + return $this->externalId; + } + + public function setExternalId(string $externalId): self + { + $this->externalId = $externalId; + + return $this; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/InternalUser.php b/tests/Fixtures/TestBundle/Entity/InternalUser.php new file mode 100644 index 00000000000..fd89fdb0ac4 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/InternalUser.php @@ -0,0 +1,39 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use Doctrine\ORM\Mapping as ORM; + +/** + * @ORM\Entity + */ +class InternalUser extends AbstractUser +{ + /** + * @ORM\Column + */ + private $internalId; + + public function getInternalId(): ?string + { + return $this->internalId; + } + + public function setInternalId(string $internalId): self + { + $this->internalId = $internalId; + + return $this; + } +} diff --git a/tests/Fixtures/TestBundle/Entity/Site.php b/tests/Fixtures/TestBundle/Entity/Site.php new file mode 100644 index 00000000000..4899d7a8efa --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Site.php @@ -0,0 +1,85 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +declare(strict_types=1); + +namespace ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity; + +use ApiPlatform\Core\Annotation\ApiResource; +use Doctrine\ORM\Mapping as ORM; + +/** + * @ApiResource + * @ORM\Entity + */ +class Site +{ + /** + * @ORM\Column(type="integer") + * @ORM\Id + * @ORM\GeneratedValue(strategy="AUTO") + */ + private $id; + /** + * @ORM\Column + */ + private $title; + /** + * @ORM\Column + */ + private $description; + /** + * @ORM\OneToOne(targetEntity="AbstractUser", cascade={"persist", "remove"}) + * @ORM\JoinColumn(nullable=false) + */ + private $owner; + + public function getId(): ?int + { + return $this->id; + } + + public function getTitle(): ?string + { + return $this->title; + } + + public function setTitle(string $title): self + { + $this->title = $title; + + return $this; + } + + public function getDescription(): ?string + { + return $this->description; + } + + public function setDescription(string $description): self + { + $this->description = $description; + + return $this; + } + + public function getOwner(): ?AbstractUser + { + return $this->owner; + } + + public function setOwner(AbstractUser $owner): self + { + $this->owner = $owner; + + return $this; + } +} diff --git a/tests/GraphQl/Serializer/ItemNormalizerTest.php b/tests/GraphQl/Serializer/ItemNormalizerTest.php index d8d1c9e9293..4e3c02da8f8 100644 --- a/tests/GraphQl/Serializer/ItemNormalizerTest.php +++ b/tests/GraphQl/Serializer/ItemNormalizerTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\GraphQl\Serializer\ItemNormalizer; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; @@ -43,7 +44,7 @@ public function testSupportNormalization() $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true)->shouldBeCalled(); @@ -78,8 +79,8 @@ public function testNormalize() $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn($propertyMetadata)->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, Dummy::class)->willReturn('/dummies/1')->shouldBeCalled(); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); diff --git a/tests/Hal/Serializer/ItemNormalizerTest.php b/tests/Hal/Serializer/ItemNormalizerTest.php index 3ac060292f0..7b8cb227806 100644 --- a/tests/Hal/Serializer/ItemNormalizerTest.php +++ b/tests/Hal/Serializer/ItemNormalizerTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\Hal\Serializer\ItemNormalizer; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface; @@ -79,7 +80,7 @@ public function testSupportsNormalization() $propertyNameCollectionFactoryProphecy = $this->prophesize(PropertyNameCollectionFactoryInterface::class); $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->isResourceClass(Dummy::class)->willReturn(true)->shouldBeCalled(); @@ -121,13 +122,14 @@ public function testNormalize() new PropertyMetadata(new Type(Type::BUILTIN_TYPE_OBJECT, false, RelatedDummy::class), '', true, false, false) )->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); - $iriConverterProphecy->getIriFromItem($relatedDummy)->willReturn('/related-dummies/2')->shouldBeCalled(); + $iriConverterProphecy->getIriFromItemWithResource($relatedDummy, RelatedDummy::class)->willReturn('/related-dummies/2')->shouldBeCalled(); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->getResourceClass($dummy, Dummy::class, true)->willReturn(Dummy::class)->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass($relatedDummy, RelatedDummy::class, true)->willReturn(RelatedDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(true)->shouldBeCalled(); $serializerProphecy = $this->prophesize(SerializerInterface::class); @@ -187,13 +189,14 @@ public function testNormalizeWithoutCache() new PropertyMetadata(new Type(Type::BUILTIN_TYPE_OBJECT, false, RelatedDummy::class), '', true, false, false) )->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); - $iriConverterProphecy->getIriFromItem($relatedDummy)->willReturn('/related-dummies/2')->shouldBeCalled(); + $iriConverterProphecy->getIriFromItemWithResource($relatedDummy, RelatedDummy::class)->willReturn('/related-dummies/2')->shouldBeCalled(); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->getResourceClass($dummy, Dummy::class, true)->willReturn(Dummy::class)->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass($relatedDummy, RelatedDummy::class, true)->willReturn(RelatedDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(true)->shouldBeCalled(); $serializerProphecy = $this->prophesize(SerializerInterface::class); @@ -281,6 +284,7 @@ public function testMaxDepth() $resourceClassResolverProphecy->getResourceClass($level1, MaxDepthDummy::class, true)->willReturn(MaxDepthDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->getResourceClass($level2, MaxDepthDummy::class, true)->willReturn(MaxDepthDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->getResourceClass($level3, MaxDepthDummy::class, true)->willReturn(MaxDepthDummy::class)->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass(null, MaxDepthDummy::class, true)->willReturn(MaxDepthDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->isResourceClass(MaxDepthDummy::class)->willReturn(true)->shouldBeCalled(); $normalizer = new ItemNormalizer( diff --git a/tests/Serializer/AbstractItemNormalizerTest.php b/tests/Serializer/AbstractItemNormalizerTest.php index f38d04bdd18..db4a2c2f740 100644 --- a/tests/Serializer/AbstractItemNormalizerTest.php +++ b/tests/Serializer/AbstractItemNormalizerTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\DataProvider\ItemDataProviderInterface; use ApiPlatform\Core\DataTransformer\DataTransformerInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; @@ -161,9 +162,9 @@ public function testNormalize() ) )->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); - $iriConverterProphecy->getIriFromItem($relatedDummy)->willReturn('/dummies/2')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, Dummy::class)->willReturn('/dummies/1')->shouldBeCalled(); + $iriConverterProphecy->getIriFromItemWithResource($relatedDummy, RelatedDummy::class)->willReturn('/dummies/2')->shouldBeCalled(); $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('foo')->shouldBeCalled(); @@ -174,6 +175,7 @@ public function testNormalize() $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass(Argument::any(), RelatedDummy::class, true)->willReturn(RelatedDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(RelatedDummy::class)->shouldBeCalled(); $serializerProphecy = $this->prophesize(SerializerInterface::class); @@ -243,8 +245,8 @@ public function testNormalizeReadableLinks() ) )->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, Dummy::class)->willReturn('/dummies/1')->shouldBeCalled(); $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); $propertyAccessorProphecy->getValue($dummy, 'relatedDummy')->willReturn($relatedDummy)->shouldBeCalled(); @@ -252,6 +254,7 @@ public function testNormalizeReadableLinks() $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled(); + $resourceClassResolverProphecy->getResourceClass(Argument::any(), RelatedDummy::class, true)->willReturn(RelatedDummy::class)->shouldBeCalled(); $resourceClassResolverProphecy->isResourceClass(RelatedDummy::class)->willReturn(RelatedDummy::class)->shouldBeCalled(); $serializerProphecy = $this->prophesize(SerializerInterface::class); @@ -825,8 +828,8 @@ public function testChildInheritedProperty() new PropertyMetadata(new Type(Type::BUILTIN_TYPE_STRING, true), '', true, true, false, false, false, false, null, DummyTableInheritanceChild::class) )->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, DummyTableInheritance::class)->willReturn('/dummies/1')->shouldBeCalled(); $propertyAccessorProphecy = $this->prophesize(PropertyAccessorInterface::class); $propertyAccessorProphecy->getValue($dummy, 'name')->willReturn('foo')->shouldBeCalled(); diff --git a/tests/Serializer/ItemNormalizerTest.php b/tests/Serializer/ItemNormalizerTest.php index 9aa8b4e3efe..ec767a6a5db 100644 --- a/tests/Serializer/ItemNormalizerTest.php +++ b/tests/Serializer/ItemNormalizerTest.php @@ -15,6 +15,7 @@ use ApiPlatform\Core\Api\IriConverterInterface; use ApiPlatform\Core\Api\ResourceClassResolverInterface; +use ApiPlatform\Core\Api\ResourceIriConverterInterface; use ApiPlatform\Core\DataTransformer\DataTransformerInterface; use ApiPlatform\Core\Exception\InvalidArgumentException; use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface; @@ -83,8 +84,8 @@ public function testNormalize() $propertyMetadataFactoryProphecy = $this->prophesize(PropertyMetadataFactoryInterface::class); $propertyMetadataFactoryProphecy->create(Dummy::class, 'name', [])->willReturn($propertyMetadata)->shouldBeCalled(); - $iriConverterProphecy = $this->prophesize(IriConverterInterface::class); - $iriConverterProphecy->getIriFromItem($dummy)->willReturn('/dummies/1')->shouldBeCalled(); + $iriConverterProphecy = $this->prophesize(ResourceIriConverterInterface::class); + $iriConverterProphecy->getIriFromItemWithResource($dummy, Dummy::class)->willReturn('/dummies/1')->shouldBeCalled(); $resourceClassResolverProphecy = $this->prophesize(ResourceClassResolverInterface::class); $resourceClassResolverProphecy->getResourceClass($dummy, null, true)->willReturn(Dummy::class)->shouldBeCalled();