From 8e5d5f47fe8887f2a15dbc63dcd0839157d3bf84 Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Fri, 22 Nov 2019 11:14:15 +0100 Subject: [PATCH 1/4] Add test to expect exception in duplicate queries --- .../TestControllerWithDuplicateQuery.php | 25 +++++++++++++++++++ tests/SchemaFactoryTest.php | 18 +++++++++++++ 2 files changed, 43 insertions(+) create mode 100644 tests/Fixtures/DuplicateQueries/TestControllerWithDuplicateQuery.php diff --git a/tests/Fixtures/DuplicateQueries/TestControllerWithDuplicateQuery.php b/tests/Fixtures/DuplicateQueries/TestControllerWithDuplicateQuery.php new file mode 100644 index 0000000000..6fd62a18f0 --- /dev/null +++ b/tests/Fixtures/DuplicateQueries/TestControllerWithDuplicateQuery.php @@ -0,0 +1,25 @@ +toArray(Debug::RETHROW_INTERNAL_EXCEPTIONS)['data']); } + + public function testDuplicateQueryException(): void + { + $factory = new SchemaFactory( + new ArrayCache(), + new BasicAutoWiringContainer( + new EmptyContainer() + ) + ); + $factory->setAuthenticationService(new VoidAuthenticationService()) + ->setAuthorizationService(new VoidAuthorizationService()) + ->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueries') + ->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration'); + + $this->expectException(GraphQLRuntimeException::class); + $schema = $factory->createSchema(); + } + } From f9fcbebec60d54040c23c834ce4eb886b98662fd Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Fri, 22 Nov 2019 11:28:46 +0100 Subject: [PATCH 2/4] Use more specific exception class --- tests/SchemaFactoryTest.php | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/tests/SchemaFactoryTest.php b/tests/SchemaFactoryTest.php index d065285c0f..8d86c790a3 100644 --- a/tests/SchemaFactoryTest.php +++ b/tests/SchemaFactoryTest.php @@ -17,6 +17,7 @@ use TheCodingMachine\GraphQLite\Containers\EmptyContainer; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\CompositeTypeMapper; +use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException; use TheCodingMachine\GraphQLite\Mappers\Parameters\ContainerParameterHandler; use TheCodingMachine\GraphQLite\Mappers\Parameters\TypeHandler; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; @@ -32,6 +33,7 @@ use TheCodingMachine\GraphQLite\Security\VoidAuthorizationService; use TheCodingMachine\GraphQLite\Fixtures\TestSelfType; + class SchemaFactoryTest extends TestCase { @@ -193,8 +195,17 @@ public function testDuplicateQueryException(): void ->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueries') ->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration'); - $this->expectException(GraphQLRuntimeException::class); + $this->expectException(DuplicateMappingException::class); $schema = $factory->createSchema(); + $queryString = ' + query { + duplicateQuery + } + '; + GraphQL::executeQuery( + $schema, + $queryString + ); } } From 08417a4cfe1ba1fc6d195913650320934f82b73d Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Fri, 22 Nov 2019 11:36:56 +0100 Subject: [PATCH 3/4] Throw an exception if duplicate queries are found in controller --- src/FieldsBuilder.php | 9 +++++++-- src/Mappers/DuplicateMappingException.php | 5 +++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php index c1f5afd783..45a05d0cf6 100644 --- a/src/FieldsBuilder.php +++ b/src/FieldsBuilder.php @@ -17,6 +17,7 @@ use TheCodingMachine\GraphQLite\Annotations\SourceFieldInterface; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeException; use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface; +use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException; use TheCodingMachine\GraphQLite\Mappers\Parameters\ParameterMiddlewareInterface; use TheCodingMachine\GraphQLite\Mappers\Parameters\TypeHandler; use TheCodingMachine\GraphQLite\Mappers\RecursiveTypeMapperInterface; @@ -31,6 +32,7 @@ use Webmozart\Assert\Assert; use function array_merge; use function array_shift; +use function array_values; use function get_parent_class; use function ucfirst; @@ -317,7 +319,10 @@ public function handle(QueryFieldDescriptor $fieldDescriptor): ?FieldDefinition }); if ($field !== null) { - $queryList[] = $field; + if (isset($queryList[$fieldDescriptor->getName()])) { + throw DuplicateMappingException::createForQuery($refClass->getName(), $fieldDescriptor->getName()); + } + $queryList[$fieldDescriptor->getName()] = $field; } /*if ($unauthorized) { @@ -332,7 +337,7 @@ public function handle(QueryFieldDescriptor $fieldDescriptor): ?FieldDefinition }*/ } - return $queryList; + return array_values($queryList); } /** diff --git a/src/Mappers/DuplicateMappingException.php b/src/Mappers/DuplicateMappingException.php index 912324aa0f..99270d24a8 100644 --- a/src/Mappers/DuplicateMappingException.php +++ b/src/Mappers/DuplicateMappingException.php @@ -23,4 +23,9 @@ public static function createForTypeName(string $type, string $sourceClass1, str { throw new self(sprintf("The type '%s' is created by 2 different classes: '%s' and '%s'", $type, $sourceClass1, $sourceClass2)); } + + public static function createForQuery(string $sourceClass, string $queryName): self + { + throw new self(sprintf("The query '%s' is duplicate in class '%s'", $queryName, $sourceClass)); + } } From 046d24f0a1cd12af41aa5d677e66406a5ddb0e8d Mon Sep 17 00:00:00 2001 From: Florian Engelhardt Date: Mon, 2 Dec 2019 12:29:29 +0100 Subject: [PATCH 4/4] Fix usage of deprecated symfony array cache --- tests/SchemaFactoryTest.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/SchemaFactoryTest.php b/tests/SchemaFactoryTest.php index 8d86c790a3..ddce2fa338 100644 --- a/tests/SchemaFactoryTest.php +++ b/tests/SchemaFactoryTest.php @@ -10,8 +10,6 @@ use Symfony\Component\Cache\Adapter\ArrayAdapter; use Symfony\Component\Cache\Adapter\Psr16Adapter; use Symfony\Component\Cache\Psr16Cache; -use Symfony\Component\Cache\Simple\ArrayCache; -use Symfony\Component\Cache\Simple\PhpFilesCache; use Symfony\Component\ExpressionLanguage\ExpressionLanguage; use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer; use TheCodingMachine\GraphQLite\Containers\EmptyContainer; @@ -185,7 +183,7 @@ private function doTestSchema(Schema $schema): void public function testDuplicateQueryException(): void { $factory = new SchemaFactory( - new ArrayCache(), + new Psr16Cache(new ArrayAdapter()), new BasicAutoWiringContainer( new EmptyContainer() )