Adding query duplicate detection in multiple classes

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<int, array<string, FieldDefinition>> $list
+     *
+     * @return array<string, FieldDefinition>
+     */
+    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);
     }
 }

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<string, FieldDefinition>
      *
      * @throws ReflectionException
      */
@@ -91,7 +90,7 @@ public function getQueries(object $controller): array
     }
 
     /**
-     * @return FieldDefinition[]
+     * @return array<string, FieldDefinition>
      *
      * @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<object> $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<string, FieldDefinition>
      *
      * @throws ReflectionException
      */
@@ -335,7 +328,7 @@ public function handle(QueryFieldDescriptor $fieldDescriptor): ?FieldDefinition
             }*/
         }
 
-        return array_values($queryList);
+        return $queryList;
     }
 
     /**

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));
     }
 }

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());

tests/Fixtures/DuplicateQueriesInTwoControllers/TestControllerWithDuplicateQuery1.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Fixtures\DuplicateQueriesInTwoControllers;
+
+use ArrayObject;
+use TheCodingMachine\GraphQLite\Annotations\Query;
+
+class TestControllerWithDuplicateQuery1
+{
+    /**
+     * @Query()
+     */
+    public function duplicateQuery(): string
+    {
+        return 'string1';
+    }
+}

tests/Fixtures/DuplicateQueriesInTwoControllers/TestControllerWithDuplicateQuery2.php

@@ -0,0 +1,17 @@
+<?php
+
+namespace TheCodingMachine\GraphQLite\Fixtures\DuplicateQueriesInTwoControllers;
+
+use ArrayObject;
+use TheCodingMachine\GraphQLite\Annotations\Query;
+
+class TestControllerWithDuplicateQuery2
+{
+    /**
+     * @Query()
+     */
+    public function duplicateQuery(): string
+    {
+        return 'string2';
+    }
+}

tests/SchemaFactoryTest.php

@@ -194,6 +194,7 @@ public function testDuplicateQueryException(): void
                 ->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
+        );
+    }
 }