diff --git a/src/Parameters/PrefetchDataParameter.php b/src/Parameters/PrefetchDataParameter.php index 774f3a0bff..1bd8d5d333 100644 --- a/src/Parameters/PrefetchDataParameter.php +++ b/src/Parameters/PrefetchDataParameter.php @@ -53,11 +53,11 @@ public function resolve(object|null $source, array $args, mixed $context, Resolv // needed, GraphQL calls the callback of Deferred below. That's when we call the prefetch method, // already knowing all the requested fields (source-arguments combinations). return new Deferred(function () use ($source, $info, $context, $args, $prefetchBuffer) { - if (! $prefetchBuffer->hasResult($source)) { + if (! $prefetchBuffer->hasResult($source, $args, $info)) { $this->processPrefetch($args, $context, $info, $prefetchBuffer); } - return $prefetchBuffer->getResult($source); + return $prefetchBuffer->getResult($source, $args, $info); }); } @@ -72,7 +72,7 @@ private function processPrefetch(array $args, mixed $context, ResolveInfo $info, foreach ($sources as $source) { // map results to each source to support old prefetch behavior - $prefetchBuffer->storeResult($source, $resolvedValues); + $prefetchBuffer->storeResult($source, $resolvedValues, $args, $info); } } diff --git a/src/PrefetchBuffer.php b/src/PrefetchBuffer.php index 317eb5d920..0f24072e4c 100644 --- a/src/PrefetchBuffer.php +++ b/src/PrefetchBuffer.php @@ -7,6 +7,7 @@ use GraphQL\Type\Definition\ResolveInfo; use WeakMap; +use function array_key_exists; use function md5; use function serialize; @@ -18,7 +19,10 @@ class PrefetchBuffer /** @var array> An array of buffered, indexed by hash of arguments. */ private array $objects = []; - /** @var WeakMap A Storage of prefetch method results, holds source to resolved values. */ + /** + * @var WeakMap> Prefetch method results per source, indexed by hash of arguments, so + * aliases of one field with different arguments don't share a result. + */ private WeakMap $results; public function __construct() @@ -71,28 +75,49 @@ public function purge( unset($this->objects[$this->computeHash($arguments, $info)]); } + /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function storeResult( object $source, mixed $result, + array $arguments = [], + ResolveInfo|null $info = null, ): void { - $this->results->offsetSet($source, $result); + $results = $this->results[$source] ?? []; + $results[$this->computeHash($arguments, $info)] = $result; + $this->results[$source] = $results; } + /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function hasResult( object $source, + array $arguments = [], + ResolveInfo|null $info = null, ): bool { - return $this->results->offsetExists($source); + return array_key_exists($this->computeHash($arguments, $info), $this->results[$source] ?? []); } + /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function getResult( object $source, + array $arguments = [], + ResolveInfo|null $info = null, ): mixed { - return $this->results->offsetGet($source); + return $this->results[$source][$this->computeHash($arguments, $info)]; } + /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function purgeResult( object $source, + array $arguments = [], + ResolveInfo|null $info = null, ): void { - $this->results->offsetUnset($source); + $results = $this->results[$source] ?? []; + unset($results[$this->computeHash($arguments, $info)]); + + if ($results === []) { + unset($this->results[$source]); + } else { + $this->results[$source] = $results; + } } } diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index e494162d4d..af04609b1d 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -194,6 +194,44 @@ public function testBatchPrefetching(): void ); } + public function testPrefetchingSameFieldWithDifferentArguments(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $queryString = ' + query { + contacts { + foo: repeatName(prefix:"foo", suffix:"") + bar: repeatName(prefix:"bar", suffix:"") + again: repeatName(prefix:"foo", suffix:"!") + } + } + '; + + $result = GraphQL::executeQuery( + $schema, + $queryString, + null, + new Context(), + ); + + $this->assertSame([ + 'contacts' => [ + [ + 'foo' => 'fooJoe', + 'bar' => 'barJoe', + 'again' => 'fooJoe!', + ], + [ + 'foo' => 'fooBill', + 'bar' => 'barBill', + 'again' => 'fooBill!', + ], + ], + ], $this->getSuccessResult($result)); + } + public function testDeprecatedField(): void { $schema = $this->mainContainer->get(Schema::class); diff --git a/tests/Parameters/PrefetchDataParameterTest.php b/tests/Parameters/PrefetchDataParameterTest.php index b478880cbe..99f65dc1c8 100644 --- a/tests/Parameters/PrefetchDataParameterTest.php +++ b/tests/Parameters/PrefetchDataParameterTest.php @@ -29,7 +29,7 @@ public function testResolveWithExistingResult(): void ]; $buffer = $context->getPrefetchBuffer($parameter); - $buffer->storeResult($source, $prefetchResult); + $buffer->storeResult($source, $prefetchResult, $args); $resolvedParameterPromise = $parameter->resolve($source, $args, $context, $this->createStub(ResolveInfo::class)); diff --git a/tests/PrefetchBufferTest.php b/tests/PrefetchBufferTest.php index b0e54d7d9f..b52e36abfe 100644 --- a/tests/PrefetchBufferTest.php +++ b/tests/PrefetchBufferTest.php @@ -26,4 +26,28 @@ public function testRegister(): void $buffer->purge(['int'=>42]); $this->assertSame([], $buffer->getObjectsByArguments(['int'=>42])); } + + public function testResultsPerArguments(): void + { + $buffer = new PrefetchBuffer(); + + $source = new stdClass(); + + $buffer->storeResult($source, 'foo', ['prefix' => 'foo']); + $buffer->storeResult($source, null, ['prefix' => 'bar']); + + $this->assertTrue($buffer->hasResult($source, ['prefix' => 'foo'])); + $this->assertTrue($buffer->hasResult($source, ['prefix' => 'bar'])); + $this->assertFalse($buffer->hasResult($source, ['prefix' => 'baz'])); + $this->assertFalse($buffer->hasResult($source)); + $this->assertSame('foo', $buffer->getResult($source, ['prefix' => 'foo'])); + $this->assertNull($buffer->getResult($source, ['prefix' => 'bar'])); + + $buffer->purgeResult($source, ['prefix' => 'foo']); + $this->assertFalse($buffer->hasResult($source, ['prefix' => 'foo'])); + $this->assertTrue($buffer->hasResult($source, ['prefix' => 'bar'])); + + $buffer->purgeResult($source, ['prefix' => 'bar']); + $this->assertFalse($buffer->hasResult($source, ['prefix' => 'bar'])); + } } diff --git a/website/docs/CHANGELOG.md b/website/docs/CHANGELOG.md index 4a656a19ad..24263985e3 100644 --- a/website/docs/CHANGELOG.md +++ b/website/docs/CHANGELOG.md @@ -72,6 +72,9 @@ sidebar_label: Changelog error without locations. GraphQLite now requires `webonyx/graphql-php` 15.38.3, whose `QueryComplexity` rule reports each invalid variable with its own located error. ([#839](https://github.com/thecodingmachine/graphqlite/pull/839)) @oojacoboo +- A `#[Prefetch]` field selected twice on the same object with different arguments (two aliases) no longer + returns the first alias's prefetch result for both. `PrefetchBuffer` stores results per source and argument + set; its result methods take the arguments and `ResolveInfo` as optional trailing parameters. @oojacoboo ## >8.0.0