From b5f1261d1d35b8ca38e6f4356cf4a71918ab739e Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Tue, 3 Nov 2026 14:13:54 -0500 Subject: [PATCH] fix: keep prefetch results per argument set PrefetchBuffer grouped sources by argument hash, so each alias of a #[Prefetch] field ran its own prefetch, but stored the result by source alone. When one object selected the field twice with different arguments (`a: field(x: 1) b: field(x: 2)`), the second alias found the first alias's result and returned it. Results are now stored per source and argument hash, the same hash that groups the sources. storeResult(), hasResult(), getResult() and purgeResult() take the arguments and ResolveInfo as optional trailing parameters. --- src/Parameters/PrefetchDataParameter.php | 6 +-- src/PrefetchBuffer.php | 35 ++++++++++++++--- tests/Integration/EndToEndTest.php | 38 +++++++++++++++++++ .../Parameters/PrefetchDataParameterTest.php | 2 +- tests/PrefetchBufferTest.php | 24 ++++++++++++ website/docs/CHANGELOG.md | 3 ++ 6 files changed, 99 insertions(+), 9 deletions(-) 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