From ba9506242c286452cc38f232c8c1e947d7962421 Mon Sep 17 00:00:00 2001 From: Vincent Chalamon Date: Wed, 24 Apr 2019 10:23:22 +0200 Subject: [PATCH 1/2] Fix #2759 --- features/bootstrap/DoctrineContext.php | 49 ++++++ features/graphql/non_resource.feature | 92 ----------- features/main/table_inheritance.feature | 152 ++++++++++++++++++ .../Symfony/Routing/RouteNameResolver.php | 16 ++ .../Symfony/Routing/RouteNameResolverTest.php | 68 +++++--- .../TestBundle/Entity/AbstractUser.php | 95 +++++++++++ .../TestBundle/Entity/ExternalUser.php | 41 +++++ .../TestBundle/Entity/InternalUser.php | 39 +++++ tests/Fixtures/TestBundle/Entity/Site.php | 88 ++++++++++ 9 files changed, 526 insertions(+), 114 deletions(-) delete mode 100644 features/graphql/non_resource.feature 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 cd03b6024d3..8bec90f52cc 100644 --- a/features/bootstrap/DoctrineContext.php +++ b/features/bootstrap/DoctrineContext.php @@ -76,11 +76,13 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\DummyProperty; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddableDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\EmbeddedDummy; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ExternalUser; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FileConfigDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Foo; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FooDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\FourthLevel; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Greeting; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\InternalUser; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\MaxDepthDummy; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Node; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Order; @@ -95,6 +97,7 @@ use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelatedToDummyFriend; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\RelationEmbedder; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\SecuredDummy; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\Site; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\ThirdLevel; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\User; use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\UuidIdentifierDummy; @@ -165,6 +168,52 @@ public function thereAreDummyObjects(int $nb) $this->manager->flush(); } + /** + * @Given there are :nb sites with internal owner + */ + public function thereAreSitesWithInternalOwner(int $nb) + { + for ($i = 1; $i <= $nb; ++$i) { + $internalUser = new InternalUser(); + $internalUser->setFirstname('Internal'); + $internalUser->setLastname('User'); + $internalUser->setEmail('john.doe@example.com'); + $internalUser->setInternalId('INT'); + + $site = new 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 ExternalUser(); + $externalUser->setFirstname('External'); + $externalUser->setLastname('User'); + $externalUser->setEmail('john.doe@example.com'); + $externalUser->setExternalId('EXT'); + + $site = new Site(); + $site->setTitle('title'); + $site->setDescription('description'); + $site->setOwner($externalUser); + + $this->manager->persist($site); + } + + $this->manager->flush(); + } + /** * @Given there are :nb foo objects with fake names */ diff --git a/features/graphql/non_resource.feature b/features/graphql/non_resource.feature deleted file mode 100644 index 909e61390b6..00000000000 --- a/features/graphql/non_resource.feature +++ /dev/null @@ -1,92 +0,0 @@ -# TODO: FIXME: GraphQL support for non-resources is non-existent - -Feature: GraphQL non-resource handling - In order to use non-resource types - As a developer - I should be able to serialize types not mapped to an API resource. - - Scenario: Get a resource containing a raw object - Then it is not supported -# When I send the following GraphQL request: -# """ -# { -# containNonResource(id: "/contain_non_resources/1") { -# _id -# id -# nested { -# _id -# id -# notAResource { -# foo -# bar -# } -# } -# notAResource { -# foo -# bar -# } -# } -# } -# """ -# 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/json" -# And the JSON should be equal to: -# """ -# { -# "data": { -# "containNonResource": { -# "_id": 1, -# "id": "/contain_non_resources/1", -# "nested": { -# "_id": "1-nested", -# "id": "/contain_non_resources/1-nested", -# "notAResource": { -# "foo": "f2", -# "bar": "b2" -# } -# }, -# "notAResource": { -# "foo": "f1", -# "bar": "b1" -# } -# } -# } -# } -# """ - - @!mongodb - @createSchema - Scenario: Create a resource that has a non-resource relation. - Then it is not supported -# When I send the following GraphQL request: -# """ -# mutation { -# createNonRelationResource(input: {relation: {foo: "test"}}) { -# nonRelationResource { -# _id -# id -# relation { -# foo -# } -# } -# } -# } -# """ -# 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/json" -# And the JSON should be equal to: -# """ -# { -# "data": { -# "nonRelationResource": { -# "_id": 1, -# "id": "/non_relation_resources/1", -# "relation": { -# "foo": "test" -# } -# } -# } -# } -# """ diff --git a/features/main/table_inheritance.feature b/features/main/table_inheritance.feature index 3cf9c9e46f9..873924a83a4 100644 --- a/features/main/table_inheritance.feature +++ b/features/main/table_inheritance.feature @@ -350,3 +350,155 @@ Feature: Table inheritance } } """ + + @createSchema + 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: + """ + {"name": "foo", "nickname": "bar"} + """ + Then the response status code should be 201 + 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": { + "@type": { + "type": "string", + "pattern": "^DummyTableInheritanceChild$" + }, + "@context": { + "type": "string", + "pattern": "^/contexts/DummyTableInheritanceChild$" + }, + "@id": { + "type": "string", + "pattern": "^/dummy_table_inheritance_children/1$" + }, + "name": { + "type": "string", + "pattern": "^foo$", + "required": "true" + }, + "nickname": { + "type": "string", + "pattern": "^bar$", + "required": "true" + } + } + } + """ + + @createSchema + @dropSchema + 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": "^/people/\\d+$", + "required": "true" + } + } + }, + "minItems": 3, + "maxItems": 3, + "required": "true" + } + } + } + """ + + @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/Bridge/Symfony/Routing/RouteNameResolver.php b/src/Bridge/Symfony/Routing/RouteNameResolver.php index eefa3356e12..7f3063d69cb 100644 --- a/src/Bridge/Symfony/Routing/RouteNameResolver.php +++ b/src/Bridge/Symfony/Routing/RouteNameResolver.php @@ -45,6 +45,7 @@ public function getRouteName(string $resourceClass, $operationType /*, array $co $operationType = OperationTypeDeprecationHelper::getOperationType($operationType); + // Try with strict class name foreach ($this->router->getRouteCollection()->all() as $routeName => $route) { $currentResourceClass = $route->getDefault('_api_resource_class'); $operation = $route->getDefault(sprintf('_api_%s_operation_name', $operationType)); @@ -59,6 +60,21 @@ public function getRouteName(string $resourceClass, $operationType /*, array $co } } + // Maybe the parent class is a resource + foreach ($this->router->getRouteCollection()->all() as $routeName => $route) { + $currentResourceClass = $route->getDefault('_api_resource_class'); + $operation = $route->getDefault(sprintf('_api_%s_operation_name', $operationType)); + $methods = $route->getMethods(); + + if (null !== $currentResourceClass && is_a($resourceClass, $currentResourceClass, true) && null !== $operation && (empty($methods) || \in_array('GET', $methods, true))) { + if (OperationType::SUBRESOURCE === $operationType && false === $this->isSameSubresource($context, $route->getDefault('_api_subresource_context'))) { + continue; + } + + return $routeName; + } + } + throw new InvalidArgumentException(sprintf('No %s route associated with the type "%s".', $operationType, $resourceClass)); } diff --git a/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php b/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php index df0cf445964..fbb96b4818a 100644 --- a/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php +++ b/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php @@ -16,6 +16,8 @@ use ApiPlatform\Core\Api\OperationType; use ApiPlatform\Core\Bridge\Symfony\Routing\RouteNameResolver; use ApiPlatform\Core\Bridge\Symfony\Routing\RouteNameResolverInterface; +use ApiPlatform\Core\Tests\Fixtures\TestBundle\Entity\User; +use FOS\UserBundle\Model\User as BaseUser; use PHPUnit\Framework\TestCase; use Symfony\Component\Routing\Route; use Symfony\Component\Routing\RouteCollection; @@ -23,6 +25,7 @@ /** * @author Teoh Han Hui + * @author Vincent Chalamon */ class RouteNameResolverTest extends TestCase { @@ -38,11 +41,11 @@ public function testConstruct() public function testGetRouteNameForItemRouteWithNoMatchingRoute() { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('No item route associated with the type "AppBundle\\Entity\\User".'); + $this->expectExceptionMessage('No item route associated with the type "ApiPlatform\\Core\\Tests\\Fixtures\\TestBundle\\Entity\\User".'); $routeCollection = new RouteCollection(); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); @@ -50,7 +53,7 @@ public function testGetRouteNameForItemRouteWithNoMatchingRoute() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM); + $routeNameResolver->getRouteName(User::class, OperationType::ITEM); } /** @@ -61,11 +64,11 @@ public function testGetRouteNameForItemRouteLegacy() { $routeCollection = new RouteCollection(); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_item_operation_name' => 'some_item_op', ])); @@ -73,7 +76,7 @@ public function testGetRouteNameForItemRouteLegacy() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', false); + $actual = $routeNameResolver->getRouteName(User::class, false); $this->assertSame('some_item_route', $actual); } @@ -82,11 +85,11 @@ public function testGetRouteNameForItemRoute() { $routeCollection = new RouteCollection(); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_item_operation_name' => 'some_item_op', ])); @@ -94,7 +97,28 @@ public function testGetRouteNameForItemRoute() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::ITEM); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::ITEM); + + $this->assertSame('some_item_route', $actual); + } + + public function testGetRouteNameForItemRouteFromAbstract() + { + $routeCollection = new RouteCollection(); + $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ + '_api_resource_class' => BaseUser::class, + '_api_collection_operation_name' => 'some_collection_op', + ])); + $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ + '_api_resource_class' => BaseUser::class, + '_api_item_operation_name' => 'some_item_op', + ])); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->getRouteCollection()->willReturn($routeCollection); + + $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::ITEM); $this->assertSame('some_item_route', $actual); } @@ -102,11 +126,11 @@ public function testGetRouteNameForItemRoute() public function testGetRouteNameForCollectionRouteWithNoMatchingRoute() { $this->expectException(\InvalidArgumentException::class); - $this->expectExceptionMessage('No collection route associated with the type "AppBundle\\Entity\\User".'); + $this->expectExceptionMessage('No collection route associated with the type "ApiPlatform\\Core\\Tests\\Fixtures\\TestBundle\\Entity\\User".'); $routeCollection = new RouteCollection(); $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_item_operation_name' => 'some_item_op', ])); @@ -114,7 +138,7 @@ public function testGetRouteNameForCollectionRouteWithNoMatchingRoute() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::COLLECTION); + $routeNameResolver->getRouteName(User::class, OperationType::COLLECTION); } /** @@ -125,11 +149,11 @@ public function testGetRouteNameForCollectionRouteLegacy() { $routeCollection = new RouteCollection(); $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_item_operation_name' => 'some_item_op', ])); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); @@ -137,7 +161,7 @@ public function testGetRouteNameForCollectionRouteLegacy() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', true); + $actual = $routeNameResolver->getRouteName(User::class, true); $this->assertSame('some_collection_route', $actual); } @@ -146,11 +170,11 @@ public function testGetRouteNameForCollectionRoute() { $routeCollection = new RouteCollection(); $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_item_operation_name' => 'some_item_op', ])); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); @@ -158,7 +182,7 @@ public function testGetRouteNameForCollectionRoute() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::COLLECTION); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::COLLECTION); $this->assertSame('some_collection_route', $actual); } @@ -167,17 +191,17 @@ public function testGetRouteNameForSubresourceRoute() { $routeCollection = new RouteCollection(); $routeCollection->add('a_some_subresource_route', new Route('/a/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_subresource_operation_name' => 'some_other_item_op', '_api_subresource_context' => ['identifiers' => [[1, 'bar']]], ])); $routeCollection->add('b_some_subresource_route', new Route('/b/some/item/path/{id}', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_subresource_operation_name' => 'some_item_op', '_api_subresource_context' => ['identifiers' => [[1, 'foo']]], ])); $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ - '_api_resource_class' => 'AppBundle\Entity\User', + '_api_resource_class' => User::class, '_api_collection_operation_name' => 'some_collection_op', ])); @@ -185,7 +209,7 @@ public function testGetRouteNameForSubresourceRoute() $routerProphecy->getRouteCollection()->willReturn($routeCollection); $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); - $actual = $routeNameResolver->getRouteName('AppBundle\Entity\User', OperationType::SUBRESOURCE, ['subresource_resources' => ['foo' => 1]]); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::SUBRESOURCE, ['subresource_resources' => ['foo' => 1]]); $this->assertSame('b_some_subresource_route', $actual); } diff --git a/tests/Fixtures/TestBundle/Entity/AbstractUser.php b/tests/Fixtures/TestBundle/Entity/AbstractUser.php new file mode 100644 index 00000000000..348f08d5a45 --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/AbstractUser.php @@ -0,0 +1,95 @@ + + * + * 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"="/people"} + * }, + * itemOperations={ + * "get"={"path"="/people/{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..66ae7a2d20f --- /dev/null +++ b/tests/Fixtures/TestBundle/Entity/Site.php @@ -0,0 +1,88 @@ + + * + * 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; + } +} From 07adbdefcecc63c5314606b6407358fc7a38ee9c Mon Sep 17 00:00:00 2001 From: Vincent Chalamon Date: Wed, 24 Apr 2019 12:20:37 +0200 Subject: [PATCH 2/2] Fix review --- features/graphql/non_resource.feature | 92 +++++++++++++++++++ features/main/table_inheritance.feature | 45 +-------- .../Symfony/Routing/RouteNameResolver.php | 2 +- .../Symfony/Routing/RouteNameResolverTest.php | 48 ++++++++++ .../TestBundle/Entity/AbstractUser.php | 4 +- 5 files changed, 146 insertions(+), 45 deletions(-) create mode 100644 features/graphql/non_resource.feature diff --git a/features/graphql/non_resource.feature b/features/graphql/non_resource.feature new file mode 100644 index 00000000000..909e61390b6 --- /dev/null +++ b/features/graphql/non_resource.feature @@ -0,0 +1,92 @@ +# TODO: FIXME: GraphQL support for non-resources is non-existent + +Feature: GraphQL non-resource handling + In order to use non-resource types + As a developer + I should be able to serialize types not mapped to an API resource. + + Scenario: Get a resource containing a raw object + Then it is not supported +# When I send the following GraphQL request: +# """ +# { +# containNonResource(id: "/contain_non_resources/1") { +# _id +# id +# nested { +# _id +# id +# notAResource { +# foo +# bar +# } +# } +# notAResource { +# foo +# bar +# } +# } +# } +# """ +# 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/json" +# And the JSON should be equal to: +# """ +# { +# "data": { +# "containNonResource": { +# "_id": 1, +# "id": "/contain_non_resources/1", +# "nested": { +# "_id": "1-nested", +# "id": "/contain_non_resources/1-nested", +# "notAResource": { +# "foo": "f2", +# "bar": "b2" +# } +# }, +# "notAResource": { +# "foo": "f1", +# "bar": "b1" +# } +# } +# } +# } +# """ + + @!mongodb + @createSchema + Scenario: Create a resource that has a non-resource relation. + Then it is not supported +# When I send the following GraphQL request: +# """ +# mutation { +# createNonRelationResource(input: {relation: {foo: "test"}}) { +# nonRelationResource { +# _id +# id +# relation { +# foo +# } +# } +# } +# } +# """ +# 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/json" +# And the JSON should be equal to: +# """ +# { +# "data": { +# "nonRelationResource": { +# "_id": 1, +# "id": "/non_relation_resources/1", +# "relation": { +# "foo": "test" +# } +# } +# } +# } +# """ diff --git a/features/main/table_inheritance.feature b/features/main/table_inheritance.feature index 873924a83a4..9ba571f67f5 100644 --- a/features/main/table_inheritance.feature +++ b/features/main/table_inheritance.feature @@ -351,47 +351,7 @@ Feature: Table inheritance } """ - @createSchema - 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: - """ - {"name": "foo", "nickname": "bar"} - """ - Then the response status code should be 201 - 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": { - "@type": { - "type": "string", - "pattern": "^DummyTableInheritanceChild$" - }, - "@context": { - "type": "string", - "pattern": "^/contexts/DummyTableInheritanceChild$" - }, - "@id": { - "type": "string", - "pattern": "^/dummy_table_inheritance_children/1$" - }, - "name": { - "type": "string", - "pattern": "^foo$", - "required": "true" - }, - "nickname": { - "type": "string", - "pattern": "^bar$", - "required": "true" - } - } - } - """ - + @!mongodb @createSchema @dropSchema Scenario: Generate iri from parent resource @@ -435,7 +395,7 @@ Feature: Table inheritance }, "owner": { "type": "string", - "pattern": "^/people/\\d+$", + "pattern": "^/custom_users/\\d+$", "required": "true" } } @@ -448,6 +408,7 @@ Feature: Table inheritance } """ + @!mongodb @createSchema Scenario: Generate iri from current resource even if parent class is a resource Given there are 3 sites with external owner diff --git a/src/Bridge/Symfony/Routing/RouteNameResolver.php b/src/Bridge/Symfony/Routing/RouteNameResolver.php index 7f3063d69cb..ed98970897b 100644 --- a/src/Bridge/Symfony/Routing/RouteNameResolver.php +++ b/src/Bridge/Symfony/Routing/RouteNameResolver.php @@ -66,7 +66,7 @@ public function getRouteName(string $resourceClass, $operationType /*, array $co $operation = $route->getDefault(sprintf('_api_%s_operation_name', $operationType)); $methods = $route->getMethods(); - if (null !== $currentResourceClass && is_a($resourceClass, $currentResourceClass, true) && null !== $operation && (empty($methods) || \in_array('GET', $methods, true))) { + if (null !== $operation && (empty($methods) || \in_array('GET', $methods, true)) && null !== $currentResourceClass && is_a($resourceClass, $currentResourceClass, true)) { if (OperationType::SUBRESOURCE === $operationType && false === $this->isSameSubresource($context, $route->getDefault('_api_subresource_context'))) { continue; } diff --git a/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php b/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php index fbb96b4818a..301c17a2321 100644 --- a/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php +++ b/tests/Bridge/Symfony/Routing/RouteNameResolverTest.php @@ -187,6 +187,27 @@ public function testGetRouteNameForCollectionRoute() $this->assertSame('some_collection_route', $actual); } + public function testGetRouteNameForCollectionRouteFromAbstract() + { + $routeCollection = new RouteCollection(); + $routeCollection->add('some_item_route', new Route('/some/item/path/{id}', [ + '_api_resource_class' => BaseUser::class, + '_api_item_operation_name' => 'some_item_op', + ])); + $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ + '_api_resource_class' => BaseUser::class, + '_api_collection_operation_name' => 'some_collection_op', + ])); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->getRouteCollection()->willReturn($routeCollection); + + $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::COLLECTION); + + $this->assertSame('some_collection_route', $actual); + } + public function testGetRouteNameForSubresourceRoute() { $routeCollection = new RouteCollection(); @@ -213,4 +234,31 @@ public function testGetRouteNameForSubresourceRoute() $this->assertSame('b_some_subresource_route', $actual); } + + public function testGetRouteNameForSubresourceRouteFromAbstract() + { + $routeCollection = new RouteCollection(); + $routeCollection->add('a_some_subresource_route', new Route('/a/some/item/path/{id}', [ + '_api_resource_class' => BaseUser::class, + '_api_subresource_operation_name' => 'some_other_item_op', + '_api_subresource_context' => ['identifiers' => [[1, 'bar']]], + ])); + $routeCollection->add('b_some_subresource_route', new Route('/b/some/item/path/{id}', [ + '_api_resource_class' => BaseUser::class, + '_api_subresource_operation_name' => 'some_item_op', + '_api_subresource_context' => ['identifiers' => [[1, 'foo']]], + ])); + $routeCollection->add('some_collection_route', new Route('/some/collection/path', [ + '_api_resource_class' => BaseUser::class, + '_api_collection_operation_name' => 'some_collection_op', + ])); + + $routerProphecy = $this->prophesize(RouterInterface::class); + $routerProphecy->getRouteCollection()->willReturn($routeCollection); + + $routeNameResolver = new RouteNameResolver($routerProphecy->reveal()); + $actual = $routeNameResolver->getRouteName(User::class, OperationType::SUBRESOURCE, ['subresource_resources' => ['foo' => 1]]); + + $this->assertSame('b_some_subresource_route', $actual); + } } diff --git a/tests/Fixtures/TestBundle/Entity/AbstractUser.php b/tests/Fixtures/TestBundle/Entity/AbstractUser.php index 348f08d5a45..75dcdcdba5b 100644 --- a/tests/Fixtures/TestBundle/Entity/AbstractUser.php +++ b/tests/Fixtures/TestBundle/Entity/AbstractUser.php @@ -21,10 +21,10 @@ * @ORM\InheritanceType("JOINED") * @ApiResource( * collectionOperations={ - * "get"={"path"="/people"} + * "get"={"path"="/custom_users"} * }, * itemOperations={ - * "get"={"path"="/people/{id}"} + * "get"={"path"="/custom_users/{id}"} * } * ) */