Reject a getter which requires a parameter when extracting by value

docs/attributes.rst

@@ -82,7 +82,10 @@ data from Doctrine entities.  The hydrator library is
   ``getField()`` or ``isField()`` method (or the entity a ``__call`` method);
   a field without one throws
   ``ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Hydrator`` when the type is
-  built, as it would otherwise always be ``null``.
+  built, as it would otherwise always be ``null``.  The hydrator calls the
+  getter of every mapped field and association, exposed or not, without
+  arguments, so a getter which requires a parameter throws the same
+  exception; give its parameters default values.
   More information here:
   `By Value and By Reference <https://www.doctrine-project.org/projects/doctrine-laminas-hydrator/en/3.0/by-value-by-reference.html#by-value-and-by-reference>`_
 

docs/upgrade.rst

@@ -131,6 +131,12 @@ which has no ``getField()`` or ``isField()`` method, and no ``__call``, now
 throws ``ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Hydrator``.  Add the
 getter, or extract the entity by reference with ``extractByValue: false``.
 
+The hydrator calls the getter of every mapped field and association, exposed
+or not, without arguments.  A getter which requires a parameter failed every
+query of its entity's type, even one which did not ask for that field, as an
+``Internal server error``.  It now throws the same exception when the type is
+built.  Give its parameters default values, or extract by reference.
+
 Date-times are converted to the default timezone
 ------------------------------------------------
 

docs/versions.rst

@@ -83,9 +83,9 @@ ORM versions do not support ``doctrine/persistence`` 4.
   to that field only
 - Mutations: an input without field lists makes the fields of nullable columns optional
 - Validation: a misplaced attribute, an invalid GraphQL name, a duplicate field name, a
-  negative limit, a field without a getter when extracting by value, and a wrong
-  ``input()`` field list are errors; configuration checks no longer depend on
-  ``zend.assertions``
+  negative limit, a field without a getter, or with one which requires a parameter, when
+  extracting by value, and a wrong ``input()`` field list are errors; configuration
+  checks no longer depend on ``zend.assertions``
 - Errors: an error the developer must fix is shown to a client as
   ``Internal server error``
 - Hydration: custom strategies may implement the new ``Strategy`` interface to receive the

src/Type/Entity/Entity.php

@@ -31,9 +31,9 @@
 use Laminas\Hydrator\HydratorInterface;
 use League\Event\EventDispatcher;
 use ReflectionClass;
+use ReflectionMethod;
 use Throwable;
 
-use function array_keys;
 use function array_merge;
 use function assert;
 use function ctype_upper;
@@ -250,10 +250,14 @@ public function getObjectType(): ObjectType
     }
 
     /**
-     * Extracting by value, the hydrator reads a field with its getter, and
-     * silently leaves out a field without one, which would always be null.
-     * The getter is found as the hydrator finds it: getField(), isField(), a
-     * field named isField() itself, or __call.
+     * Extracting by value, the hydrator reads every mapped field and
+     * association, exposed or not, by calling its getter without arguments.
+     * The getter is found as the hydrator finds it: getField(), isField(), or
+     * a field named isField() itself.
+     *
+     * A getter which requires a parameter fails every extract of the entity,
+     * so every query of its type.  An exposed field without a getter, and
+     * without __call, is silently left out, so would always be null.
      *
      * @throws HydratorException
      */
@@ -263,30 +267,44 @@ private function assertExtractable(): void
             return;
         }
 
-        $methods = get_class_methods($this->getEntityClass());
-        if (in_array('__call', $methods, true)) {
-            return;
-        }
-
-        $inflector = InflectorFactory::create()->build();
+        $entityClass   = $this->getEntityClass();
+        $methods       = get_class_methods($entityClass);
+        $hasCall       = in_array('__call', $methods, true);
+        $exposed       = [...$this->entityMetadata->fields, ...$this->entityMetadata->associations];
+        $classMetadata = $this->entityManager->getClassMetadata($entityClass);
+        $inflector     = InflectorFactory::create()->build();
 
-        foreach (array_keys([...$this->entityMetadata->fields, ...$this->entityMetadata->associations]) as $fieldName) {
+        foreach ([...$classMetadata->getFieldNames(), ...$classMetadata->getAssociationNames()] as $fieldName) {
             $getter = 'get' . $inflector->classify($fieldName);
             $isser  = 'is' . $inflector->classify($fieldName);
 
-            if (
-                in_array($getter, $methods, true)
-                || in_array($isser, $methods, true)
-                || (str_starts_with($fieldName, 'is') && ctype_upper(substr($fieldName, 2, 1)) && in_array($fieldName, $methods, true))
-            ) {
-                continue;
+            $method = match (true) {
+                in_array($getter, $methods, true) => $getter,
+                in_array($isser, $methods, true) => $isser,
+                str_starts_with($fieldName, 'is') && ctype_upper(substr($fieldName, 2, 1))
+                    && in_array($fieldName, $methods, true) => $fieldName,
+                default => null,
+            };
+
+            if ($method === null) {
+                if ($hasCall || ! isset($exposed[$fieldName])) {
+                    continue;
+                }
+
+                throw new HydratorException(
+                    'Field ' . $fieldName . ' of entity ' . $entityClass . ' has no ' . $getter . '() or '
+                    . $isser . '() method, which extracting by value reads it with.  Add one, or extract the entity '
+                    . 'by reference with extractByValue: false.',
+                );
             }
 
-            throw new HydratorException(
-                'Field ' . $fieldName . ' of entity ' . $this->getEntityClass() . ' has no ' . $getter . '() or '
-                . $isser . '() method, which extracting by value reads it with.  Add one, or extract the entity '
-                . 'by reference with extractByValue: false.',
-            );
+            if ((new ReflectionMethod($entityClass, $method))->getNumberOfRequiredParameters() > 0) {
+                throw new HydratorException(
+                    'Field ' . $fieldName . ' of entity ' . $entityClass . ' is read by ' . $method . '(), which '
+                    . 'extracting by value calls without arguments, but it requires a parameter.  Give its '
+                    . 'parameters default values, or extract the entity by reference with extractByValue: false.',
+                );
+            }
         }
     }
 

test/Entity/TestGetterOptionalParameter.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity;
+
+use ApiSkeletons\Doctrine\ORM\GraphQL\Attribute as GraphQL;
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * A field whose getter has only an optional parameter, which extracting by
+ * value calls without it
+ */
+#[GraphQL\Entity(group: 'GetterOptional')]
+#[ORM\Entity]
+class TestGetterOptionalParameter
+{
+    #[GraphQL\Field(group: 'GetterOptional')]
+    #[ORM\Id]
+    #[ORM\Column(type: 'integer')]
+    #[ORM\GeneratedValue]
+    private int $id;
+
+    #[GraphQL\Field(group: 'GetterOptional')]
+    #[ORM\Column(type: 'string')]
+    private string $label = 'Label';
+
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    public function getLabel(string $suffix = '!'): string
+    {
+        return $this->label . $suffix;
+    }
+}

test/Entity/TestGetterRequiredParameter.php

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types=1);
+
+namespace ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity;
+
+use ApiSkeletons\Doctrine\ORM\GraphQL\Attribute as GraphQL;
+use Doctrine\ORM\Mapping as ORM;
+
+/**
+ * A field whose getter requires a parameter, which extracting by value
+ * cannot call: exposed in the GetterRequired group and not in the
+ * GetterRequiredUnexposed group.  __call does not change that the getter is
+ * called.
+ */
+#[GraphQL\Entity(group: 'GetterRequired')]
+#[GraphQL\Entity(group: 'GetterRequiredUnexposed')]
+#[ORM\Entity]
+class TestGetterRequiredParameter
+{
+    #[GraphQL\Field(group: 'GetterRequired')]
+    #[GraphQL\Field(group: 'GetterRequiredUnexposed')]
+    #[ORM\Id]
+    #[ORM\Column(type: 'integer')]
+    #[ORM\GeneratedValue]
+    private int $id;
+
+    #[GraphQL\Field(group: 'GetterRequired')]
+    #[ORM\Column(type: 'string')]
+    private string $title = 'Title';
+
+    public function getId(): int
+    {
+        return $this->id;
+    }
+
+    public function getTitle(string $locale): string
+    {
+        return $this->title . ' (' . $locale . ')';
+    }
+
+    /** @param mixed[] $arguments */
+    public function __call(string $name, array $arguments): mixed
+    {
+        return null;
+    }
+}

test/Feature/Hydrator/MissingGetterTest.php

@@ -9,10 +9,13 @@
 use ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Hydrator as HydratorException;
 use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithMagicCall;
 use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithoutGetter;
+use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestGetterOptionalParameter;
+use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestGetterRequiredParameter;
 use ApiSkeletonsTest\Doctrine\ORM\GraphQL\TestCase;
 use GraphQL\GraphQL;
 use GraphQL\Type\Definition\ObjectType;
 use GraphQL\Type\Schema;
+use PHPUnit\Framework\Attributes\DataProvider;
 
 /**
  * Extracting by value, a field without a getter would always be null, so it
@@ -113,4 +116,62 @@ public function testEntityWithCallIsExtractedByValue(): void
             $result['data']['entities']['edges'],
         );
     }
+
+    /** @return array<string, array{string}> */
+    public static function requiredParameterProvider(): array
+    {
+        return [
+            'exposed field' => ['GetterRequired'],
+            'field which is not exposed' => ['GetterRequiredUnexposed'],
+        ];
+    }
+
+    /**
+     * Extracting by value calls the getter of every mapped field without
+     * arguments, so one which requires a parameter is an error, exposed or
+     * not, and with __call or not
+     */
+    #[DataProvider('requiredParameterProvider')]
+    public function testGetterWithARequiredParameterIsAnError(string $group): void
+    {
+        $driver = new Driver($this->getEntityManager(), new Config(['group' => $group]));
+
+        $this->expectException(HydratorException::class);
+        $this->expectExceptionMessage(
+            'Field title of entity ' . TestGetterRequiredParameter::class . ' is read by getTitle(), which '
+            . 'extracting by value calls without arguments, but it requires a parameter.  Give its parameters '
+            . 'default values, or extract the entity by reference with extractByValue: false.',
+        );
+
+        $driver->type(TestGetterRequiredParameter::class);
+    }
+
+    public function testGetterWithARequiredParameterIsExtractedByReference(): void
+    {
+        $driver = new Driver(
+            $this->getEntityManager(),
+            new Config(['group' => 'GetterRequired', 'extractByValue' => false]),
+        );
+
+        $this->assertInstanceOf(ObjectType::class, $driver->type(TestGetterRequiredParameter::class));
+    }
+
+    public function testGetterWithAnOptionalParameterIsCalledWithoutIt(): void
+    {
+        $this->getEntityManager()->persist(new TestGetterOptionalParameter());
+        $this->getEntityManager()->flush();
+        $this->getEntityManager()->clear();
+
+        $driver = new Driver($this->getEntityManager(), new Config(['group' => 'GetterOptional']));
+        $schema = new Schema([
+            'query' => new ObjectType([
+                'name' => 'query',
+                'fields' => ['entities' => $driver->completeConnection(TestGetterOptionalParameter::class)],
+            ]),
+        ]);
+
+        $result = GraphQL::executeQuery($schema, '{ entities { edges { node { label } } } }')->toArray();
+
+        $this->assertSame([['node' => ['label' => 'Label!']]], $result['data']['entities']['edges']);
+    }
 }