Merge pull request #39 from moufmouf/override

Optimization: we are not recreating fields in child class that are part of parent class

Author: moufmouf

src/FieldsBuilder.php

@@ -4,6 +4,7 @@
 namespace TheCodingMachine\GraphQLite;
 
 use function array_merge;
+use function get_parent_class;
 use GraphQL\Type\Definition\InputType;
 use GraphQL\Type\Definition\ListOfType;
 use GraphQL\Type\Definition\NonNull;
@@ -15,6 +16,7 @@
 use phpDocumentor\Reflection\Types\Self_;
 use Psr\Http\Message\UploadedFileInterface;
 use ReflectionMethod;
+use TheCodingMachine\GraphQLite\Annotations\Field;
 use TheCodingMachine\GraphQLite\Annotations\SourceFieldInterface;
 use TheCodingMachine\GraphQLite\Hydrators\HydratorInterface;
 use TheCodingMachine\GraphQLite\Mappers\CannotMapTypeExceptionInterface;
@@ -222,7 +224,21 @@ private function getFieldsByAnnotations($controller, string $annotationName, boo
         $oldDeclaringClass = null;
         $context = null;
 
+        $closestMatchingTypeClass = null;
+        if ($annotationName === Field::class) {
+            $parent = get_parent_class($refClass->getName());
+            if ($parent !== null) {
+                $closestMatchingTypeClass = $this->typeMapper->findClosestMatchingParent($parent);
+            }
+        }
+
         foreach ($refClass->getMethods() as $refMethod) {
+            if ($closestMatchingTypeClass !== null && $closestMatchingTypeClass === $refMethod->getDeclaringClass()->getName()) {
+                // Optimisation: no need to fetch annotations from parent classes that are ALREADY GraphQL types.
+                // We will merge the fields anyway.
+                break;
+            }
+
             // First, let's check the "Query" or "Mutation" or "Field" annotation
             $queryAnnotation = $this->annotationReader->getRequestAnnotation($refMethod, $annotationName);
 

src/Mappers/RecursiveTypeMapper.php

@@ -148,7 +148,7 @@ public function mapClassToType(string $className, ?OutputType $subType): Mutable
      * @param string $className
      * @return string|null
      */
-    private function findClosestMatchingParent(string $className): ?string
+    public function findClosestMatchingParent(string $className): ?string
     {
         do {
             if ($this->typeMapper->canMapClassToType($className)) {

src/Mappers/RecursiveTypeMapperInterface.php

@@ -94,4 +94,12 @@ public function canMapNameToType(string $typeName): bool;
      * @return Type&(InputType|OutputType)
      */
     public function mapNameToType(string $typeName): Type;
+
+    /**
+     * Returns the closest parent that can be mapped, or null if nothing can be matched.
+     *
+     * @param string $className
+     * @return string|null
+     */
+    public function findClosestMatchingParent(string $className): ?string;
 }

src/Types/TypeAnnotatedObjectType.php

@@ -43,6 +43,7 @@ public static function createFromAnnotatedClass(string $typeName, string $classN
                     $fields = $fieldProvider->getSelfFields($className);
                 }
                 if ($parentType !== null) {
+                    // Note: with +=, the keys already present are not overloaded
                     $fields += $parentType->getFields();
                 }
                 return $fields;

tests/Fixtures/Integration/Models/Contact.php

@@ -6,6 +6,7 @@
 
 use DateTimeInterface;
 use Psr\Http\Message\UploadedFileInterface;
+use TheCodingMachine\GraphQLite\Annotations\Field;
 use TheCodingMachine\GraphQLite\Annotations\Type;
 
 /**
@@ -33,6 +34,10 @@ class Contact
      * @var DateTimeInterface
      */
     private $birthDate;
+    /**
+     * @var string
+     */
+    private $company;
 
     public function __construct(string $name)
     {
@@ -107,4 +112,23 @@ public function setBirthDate(DateTimeInterface $birthDate): void
     {
         $this->birthDate = $birthDate;
     }
+
+    /**
+     * This getter will be overridden in the extend class.
+     *
+     * @Field()
+     * @return string
+     */
+    public function getCompany(): string
+    {
+        return $this->company;
+    }
+
+    /**
+     * @param string $company
+     */
+    public function setCompany(string $company): void
+    {
+        $this->company = $company;
+    }
 }

tests/Fixtures/Integration/Types/ExtendedContactType.php

@@ -22,4 +22,15 @@ public function uppercaseName(Contact $contact): string
     {
         return strtoupper($contact->getName());
     }
-}
+
+    /**
+     * Here, we are testing overriding the field in the extend class.
+     *
+     * @Field()
+     * @return string
+     */
+    public function company(Contact $contact): string
+    {
+        return $contact->getName().' Ltd';
+    }
+}

tests/Integration/EndToEndTest.php

@@ -199,6 +199,7 @@ public function testEndToEnd()
         query {
             contacts {
                 name
+                company
                 uppercaseName
                 ... on User {
                     email
@@ -216,10 +217,12 @@ public function testEndToEnd()
             'contacts' => [
                 [
                     'name' => 'Joe',
+                    'company' => 'Joe Ltd',
                     'uppercaseName' => 'JOE'
                 ],
                 [
                     'name' => 'Bill',
+                    'company' => 'Bill Ltd',
                     'uppercaseName' => 'BILL',
                     'email' => 'bill@example.com'
                 ]
@@ -237,10 +240,12 @@ public function testEndToEnd()
             'contacts' => [
                 [
                     'name' => 'Joe',
+                    'company' => 'Joe Ltd',
                     'uppercaseName' => 'JOE'
                 ],
                 [
                     'name' => 'Bill',
+                    'company' => 'Bill Ltd',
                     'uppercaseName' => 'BILL',
                     'email' => 'bill@example.com'
                 ]