diff --git a/src/AggregateControllerQueryProvider.php b/src/AggregateControllerQueryProvider.php index ff5225b748..32e47091fc 100644 --- a/src/AggregateControllerQueryProvider.php +++ b/src/AggregateControllerQueryProvider.php @@ -6,7 +6,18 @@ use GraphQL\Type\Definition\FieldDefinition; use Psr\Container\ContainerInterface; +use TheCodingMachine\GraphQLite\Mappers\DuplicateMappingException; +use function array_filter; +use function array_intersect_key; +use function array_keys; +use function array_map; use function array_merge; +use function array_sum; +use function array_values; +use function assert; +use function count; +use function reset; +use function sort; /** * A query provider that looks into all controllers of your application to fetch queries. @@ -40,10 +51,10 @@ public function getQueries(): array foreach ($this->controllers as $controllerName) { $controller = $this->controllersContainer->get($controllerName); - $queryList = array_merge($queryList, $this->fieldsBuilder->getQueries($controller)); + $queryList[$controllerName] = $this->fieldsBuilder->getQueries($controller); } - return $queryList; + return $this->flattenList($queryList); } /** @@ -55,9 +66,43 @@ public function getMutations(): array foreach ($this->controllers as $controllerName) { $controller = $this->controllersContainer->get($controllerName); - $mutationList = array_merge($mutationList, $this->fieldsBuilder->getMutations($controller)); + $mutationList[$controllerName] = $this->fieldsBuilder->getMutations($controller); } - return $mutationList; + return $this->flattenList($mutationList); + } + + /** + * @param array> $list + * + * @return array + */ + private function flattenList(array $list): array + { + if (empty($list)) { + return []; + } + + $flattenedList = array_merge(...array_values($list)); + + // Quick check: are there duplicates? If so, the count of the flattenedList is != from the sum of the count of lists. + if (count($flattenedList) === array_sum(array_map('count', $list))) { + return $flattenedList; + } + + // We have an issue, let's detect the duplicate + $duplicates = array_intersect_key(...array_values($list)); + // Let's display an error from the first one. + $firstDuplicate = reset($duplicates); + assert($firstDuplicate instanceof FieldDefinition); + + $duplicateName = $firstDuplicate->name; + + $classes = array_keys(array_filter($list, static function (array $fields) use ($duplicateName) { + return isset($fields[$duplicateName]); + })); + sort($classes); + + throw DuplicateMappingException::createForQueryInTwoControllers($classes[0], $classes[1], $duplicateName); } } diff --git a/src/FieldsBuilder.php b/src/FieldsBuilder.php index a657c923a3..7534f1cf44 100644 --- a/src/FieldsBuilder.php +++ b/src/FieldsBuilder.php @@ -32,7 +32,6 @@ use Webmozart\Assert\Assert; use function array_merge; use function array_shift; -use function array_values; use function get_parent_class; use function is_string; use function ucfirst; @@ -81,7 +80,7 @@ public function __construct( } /** - * @return FieldDefinition[] + * @return array * * @throws ReflectionException */ @@ -91,7 +90,7 @@ public function getQueries(object $controller): array } /** - * @return FieldDefinition[] + * @return array * * @throws ReflectionException */ @@ -118,10 +117,7 @@ public function getFields(object $controller): array $fieldsFromSourceFields = $this->getQueryFieldsFromSourceFields($sourceFields, $refClass); - $fields = []; - foreach ($fieldAnnotations as $field) { - $fields[$field->name] = $field; - } + $fields = $fieldAnnotations; foreach ($fieldsFromSourceFields as $field) { $fields[$field->name] = $field; } @@ -147,10 +143,7 @@ public function getSelfFields(string $className): array $fieldsFromSourceFields = $this->getQueryFieldsFromSourceFields($sourceFields, $refClass); - $fields = []; - foreach ($fieldAnnotations as $field) { - $fields[$field->name] = $field; - } + $fields = $fieldAnnotations; foreach ($fieldsFromSourceFields as $field) { $fields[$field->name] = $field; } @@ -199,7 +192,7 @@ public function getParametersForDecorator(ReflectionMethod $refMethod): array * @param object|class-string $controller The controller instance, or the name of the source class name * @param bool $injectSource Whether to inject the source object or not as the first argument. True for @Field (unless @Type has no class attribute), false for @Query and @Mutation * - * @return FieldDefinition[] + * @return array * * @throws ReflectionException */ @@ -335,7 +328,7 @@ public function handle(QueryFieldDescriptor $fieldDescriptor): ?FieldDefinition }*/ } - return array_values($queryList); + return $queryList; } /** diff --git a/src/Mappers/DuplicateMappingException.php b/src/Mappers/DuplicateMappingException.php index 99270d24a8..714ccd16b1 100644 --- a/src/Mappers/DuplicateMappingException.php +++ b/src/Mappers/DuplicateMappingException.php @@ -26,6 +26,11 @@ public static function createForTypeName(string $type, string $sourceClass1, str public static function createForQuery(string $sourceClass, string $queryName): self { - throw new self(sprintf("The query '%s' is duplicate in class '%s'", $queryName, $sourceClass)); + throw new self(sprintf("The query/mutation '%s' is declared twice in class '%s'", $queryName, $sourceClass)); + } + + public static function createForQueryInTwoControllers(string $sourceClass1, string $sourceClass2, string $queryName): self + { + throw new self(sprintf("The query/mutation '%s' is declared twice: in class '%s' and in class '%s'", $queryName, $sourceClass1, $sourceClass2)); } } diff --git a/tests/FieldsBuilderTest.php b/tests/FieldsBuilderTest.php index b08dfc330e..81976216b1 100644 --- a/tests/FieldsBuilderTest.php +++ b/tests/FieldsBuilderTest.php @@ -86,7 +86,7 @@ public function testQueryProvider(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $usersQuery = $queries[0]; + $usersQuery = $queries['test']; $this->assertSame('test', $usersQuery->name); $this->assertCount(10, $usersQuery->args); @@ -141,7 +141,7 @@ public function testMutations(): void $mutations = $queryProvider->getMutations($controller); $this->assertCount(1, $mutations); - $mutation = $mutations[0]; + $mutation = $mutations['mutation']; $this->assertSame('mutation', $mutation->name); $resolve = $mutation->resolveFn; @@ -190,7 +190,7 @@ public function test($typeHintInDocBlock) $queries = $queryProvider->getQueries($controller); $this->assertCount(1, $queries); - $query = $queries[0]; + $query = $queries['test']; $this->assertInstanceOf(NonNull::class, $query->args[0]->getType()); $this->assertInstanceOf(IntType::class, $query->args[0]->getType()->getWrappedType()); @@ -207,7 +207,7 @@ public function testQueryProviderWithFixedReturnType(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $fixedQuery = $queries[1]; + $fixedQuery = $queries['testFixReturnType']; $this->assertInstanceOf(IDType::class, $fixedQuery->getType()); } @@ -221,7 +221,7 @@ public function testQueryProviderWithComplexFixedReturnType(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $fixedQuery = $queries[6]; + $fixedQuery = $queries['testFixComplexReturnType']; $this->assertInstanceOf(NonNull::class, $fixedQuery->getType()); $this->assertInstanceOf(ListOfType::class, $fixedQuery->getType()->getWrappedType()); @@ -237,7 +237,7 @@ public function testNameFromAnnotation(): void $queries = $queryProvider->getQueries($controller); - $query = $queries[2]; + $query = $queries['nameFromAnnotation']; $this->assertSame('nameFromAnnotation', $query->name); } @@ -410,7 +410,7 @@ public function testQueryProviderWithIterableClass(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $iterableQuery = $queries[3]; + $iterableQuery = $queries['arrayObject']; $this->assertSame('arrayObject', $iterableQuery->name); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); @@ -427,7 +427,7 @@ public function testQueryProviderWithIterable(): void $queries = $queryProvider->getQueries(new TestController()); $this->assertCount(7, $queries); - $iterableQuery = $queries[4]; + $iterableQuery = $queries['iterable']; $this->assertSame('iterable', $iterableQuery->name); $this->assertInstanceOf(NonNull::class, $iterableQuery->getType()); @@ -453,7 +453,7 @@ public function testQueryProviderWithUnion(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $unionQuery = $queries[5]; + $unionQuery = $queries['union']; $this->assertInstanceOf(NonNull::class, $unionQuery->getType()); $this->assertInstanceOf(UnionType::class, $unionQuery->getType()->getWrappedType()); @@ -539,7 +539,7 @@ public function testFailWith(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(1, $queries); - $query = $queries[0]; + $query = $queries['testFailWith']; $this->assertSame('testFailWith', $query->name); $resolve = $query->resolveFn; @@ -621,7 +621,7 @@ public function testMissingArgument(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(7, $queries); - $usersQuery = $queries[0]; + $usersQuery = $queries['test']; $context = []; $resolve = $usersQuery->resolveFn; @@ -686,7 +686,7 @@ public function testSecurityBadQuery(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(1, $queries); - $query = $queries[0]; + $query = $queries['testBadSecurity']; $this->assertSame('testBadSecurity', $query->name); $resolve = $query->resolveFn; @@ -705,7 +705,7 @@ public function testQueryProviderWithNullableArray(): void $queries = $queryProvider->getQueries($controller); $this->assertCount(1, $queries); - $usersQuery = $queries[0]; + $usersQuery = $queries['test']; $this->assertSame('test', $usersQuery->name); $this->assertInstanceOf(NonNull::class, $usersQuery->args[0]->getType()); diff --git a/tests/Fixtures/DuplicateQueriesInTwoControllers/TestControllerWithDuplicateQuery1.php b/tests/Fixtures/DuplicateQueriesInTwoControllers/TestControllerWithDuplicateQuery1.php new file mode 100644 index 0000000000..48a826c994 --- /dev/null +++ b/tests/Fixtures/DuplicateQueriesInTwoControllers/TestControllerWithDuplicateQuery1.php @@ -0,0 +1,17 @@ +addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration'); $this->expectException(DuplicateMappingException::class); + $this->expectExceptionMessage("The query/mutation 'duplicateQuery' is declared twice in class 'TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueries\\TestControllerWithDuplicateQuery"); $schema = $factory->createSchema(); $queryString = ' query { @@ -206,4 +207,30 @@ public function testDuplicateQueryException(): void ); } + public function testDuplicateQueryInTwoControllersException(): void + { + $factory = new SchemaFactory( + new Psr16Cache(new ArrayAdapter()), + new BasicAutoWiringContainer( + new EmptyContainer() + ) + ); + $factory->setAuthenticationService(new VoidAuthenticationService()) + ->setAuthorizationService(new VoidAuthorizationService()) + ->addControllerNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueriesInTwoControllers') + ->addTypeNamespace('TheCodingMachine\\GraphQLite\\Fixtures\\Integration'); + + $this->expectException(DuplicateMappingException::class); + $this->expectExceptionMessage("The query/mutation 'duplicateQuery' is declared twice: in class 'TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueriesInTwoControllers\\TestControllerWithDuplicateQuery1' and in class 'TheCodingMachine\\GraphQLite\\Fixtures\\DuplicateQueriesInTwoControllers\\TestControllerWithDuplicateQuery2"); + $schema = $factory->createSchema(); + $queryString = ' + query { + duplicateQuery + } + '; + GraphQL::executeQuery( + $schema, + $queryString + ); + } }