diff --git a/src/Hydrator/DoctrineObjectWithComputed.php b/src/Hydrator/DoctrineObjectWithComputed.php index bcd8a07..af8ccd1 100644 --- a/src/Hydrator/DoctrineObjectWithComputed.php +++ b/src/Hydrator/DoctrineObjectWithComputed.php @@ -235,7 +235,8 @@ protected function extractByReference(object $object): array * field values by calling registered extractors. A computed field with * arguments has a value for each set of them, not one value, so it is * left out. A batched computed field would be a query for each entity - * extracted, so it is left out too. + * extracted, so it is left out too, as is a mapped property of the + * same name which is not exposed. * * @return array */ @@ -245,7 +246,11 @@ public function extract(object $object): array $data = $this->extractFields($object); foreach ($this->getComputedFieldNames() as $fieldName) { + // A mapped property of the same name which is not exposed is not + // the computed field's value if ($this->hasComputedFieldArguments($fieldName) || isset($this->batchExtractors[$fieldName])) { + unset($data[$fieldName]); + continue; } diff --git a/src/Resolve/FieldResolver.php b/src/Resolve/FieldResolver.php index ace4f64..bb8b956 100644 --- a/src/Resolve/FieldResolver.php +++ b/src/Resolve/FieldResolver.php @@ -61,7 +61,8 @@ public function __invoke(mixed $source, mixed $args, mixed $context, ResolveInfo // A hit is decided by the entity, not the field, so a null field is // served from the extract rather than extracting again - $values = $this->extractValues[$source] ?? null; + $values = $this->extractValues[$source] ?? null; + $hydrator = $this->getHydrator($source); if ($values === null) { /** @@ -73,7 +74,6 @@ public function __invoke(mixed $source, mixed $args, mixed $context, ResolveInfo } // Computed fields are not extracted here; each is computed when queried - $hydrator = $this->getHydrator($source); $values = $hydrator instanceof DoctrineObjectWithComputed ? $hydrator->extractFields($source) : $hydrator->extract($source); @@ -82,35 +82,31 @@ public function __invoke(mixed $source, mixed $args, mixed $context, ResolveInfo $key = $info->fieldName; - if (! array_key_exists($key, $values)) { - $hydrator = $this->getHydrator($source); - - if ($hydrator instanceof DoctrineObjectWithComputed && $hydrator->hasComputedField($info->fieldName)) { - // A computed field with arguments has a value for each set of - // them, as aliases of the field may give different arguments - $args = is_array($args) ? $args : []; - if ($hydrator->hasComputedFieldArguments($info->fieldName)) { - $key .= '(' . serialize($args) . ')'; - } - - if (! array_key_exists($key, $values)) { - // A batched field is loaded with the other entities waiting for it - $batchExtractor = $this->config->getBatchAssociations() - ? $hydrator->getBatchExtractor($info->fieldName) - : null; - - /** @psalm-suppress MixedAssignment */ - $values[$key] = $batchExtractor !== null - ? $this->computedFieldBatchLoader->defer( - $source, - $this->entityTypeContainer->getExposedClass($source), - $info->fieldName, - $batchExtractor, - $args, - ) - : $hydrator->extractComputedField($source, $info->fieldName, $args); - $this->extractValues[$source] = $values; - } + // A computed field wins over a mapped property of the same name which + // is not exposed, so its value is kept under a key no property has + if ($hydrator instanceof DoctrineObjectWithComputed && $hydrator->hasComputedField($info->fieldName)) { + // A computed field with arguments has a value for each set of + // them, as aliases of the field may give different arguments + $args = is_array($args) ? $args : []; + $key .= '(' . ($hydrator->hasComputedFieldArguments($info->fieldName) ? serialize($args) : '') . ')'; + + if (! array_key_exists($key, $values)) { + // A batched field is loaded with the other entities waiting for it + $batchExtractor = $this->config->getBatchAssociations() + ? $hydrator->getBatchExtractor($info->fieldName) + : null; + + /** @psalm-suppress MixedAssignment */ + $values[$key] = $batchExtractor !== null + ? $this->computedFieldBatchLoader->defer( + $source, + $this->entityTypeContainer->getExposedClass($source), + $info->fieldName, + $batchExtractor, + $args, + ) + : $hydrator->extractComputedField($source, $info->fieldName, $args); + $this->extractValues[$source] = $values; } } diff --git a/test/Entity/TestComputedShadowsProperty.php b/test/Entity/TestComputedShadowsProperty.php new file mode 100644 index 0000000..95591d8 --- /dev/null +++ b/test/Entity/TestComputedShadowsProperty.php @@ -0,0 +1,56 @@ +id; + } + + public function getTest(): string + { + return $this->test; + } + + #[GraphQL\ComputedField(type: 'string', name: 'test', group: 'computedShadowTest')] + public function computeTest(): string + { + return 'computed'; + } + + public function getLabel(): string + { + return $this->label; + } + + #[GraphQL\ComputedField(type: 'string', name: 'label', group: 'computedShadowTest')] + public function computeLabel(string $prefix): string + { + return $prefix . 'computed'; + } +} diff --git a/test/Feature/ComputedField/ComputedFieldEdgeCasesTest.php b/test/Feature/ComputedField/ComputedFieldEdgeCasesTest.php index 8e8d8b9..5309b28 100644 --- a/test/Feature/ComputedField/ComputedFieldEdgeCasesTest.php +++ b/test/Feature/ComputedField/ComputedFieldEdgeCasesTest.php @@ -7,12 +7,17 @@ use ApiSkeletons\Doctrine\ORM\GraphQL\Config; use ApiSkeletons\Doctrine\ORM\GraphQL\Driver; use ApiSkeletons\Doctrine\ORM\GraphQL\Exception\Metadata as MetadataException; +use ApiSkeletons\Doctrine\ORM\GraphQL\Hydrator\HydratorContainer; use ApiSkeletons\Doctrine\ORM\GraphQL\Metadata\MetadataFactory; +use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestComputedShadowsProperty; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithCollision; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithGetMethod; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithIsMethod; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\Entity\TestEntityWithPlainMethod; use ApiSkeletonsTest\Doctrine\ORM\GraphQL\TestCase; +use GraphQL\GraphQL; +use GraphQL\Type\Definition\ObjectType; +use GraphQL\Type\Schema; use PHPUnit\Framework\Attributes\DataProvider; use ReflectionClass; use ReflectionMethod; @@ -92,4 +97,54 @@ public function testFieldNameIsDerivedFromTheMethodName(string $methodName, stri $this->assertSame($fieldName, (new ReflectionMethod($factory, 'deriveFieldNameFromMethod'))->invoke($factory, $methodName)); } + + /** + * A computed field named as a mapped property which is not exposed is + * resolved by its method, not by the property's getter + * + * @return array + */ + public static function extractByValueProvider(): array + { + return [ + 'by value' => [true], + 'by reference' => [false], + ]; + } + + #[DataProvider('extractByValueProvider')] + public function testComputedFieldWinsOverAnUnexposedProperty(bool $extractByValue): void + { + $entityManager = $this->getEntityManager(); + $entityManager->persist(new TestComputedShadowsProperty()); + $entityManager->flush(); + $entityManager->clear(); + + $driver = new Driver($entityManager, new Config([ + 'group' => 'computedShadowTest', + 'extractByValue' => $extractByValue, + ])); + + $schema = new Schema([ + 'query' => new ObjectType([ + 'name' => 'query', + 'fields' => [ + 'entities' => $driver->completeConnection(TestComputedShadowsProperty::class), + ], + ]), + ]); + + $result = GraphQL::executeQuery($schema, '{ entities { edges { node { id test label(prefix: "a ") } } } }'); + $output = $result->toArray(); + + $this->assertEmpty($output['errors'] ?? []); + $this->assertSame('computed', $output['data']['entities']['edges'][0]['node']['test']); + $this->assertSame('a computed', $output['data']['entities']['edges'][0]['node']['label']); + + // extract() leaves out a computed field with arguments, and the property + $entity = $entityManager->getRepository(TestComputedShadowsProperty::class)->findAll()[0]; + $extracted = $driver->get(HydratorContainer::class)->get(TestComputedShadowsProperty::class)->extract($entity); + $this->assertSame('computed', $extracted['test']); + $this->assertArrayNotHasKey('label', $extracted); + } }