From 7bb8ae278ee930d9b31c5f4b53357b294054c31d Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Sat, 26 Aug 2023 23:15:27 +0200 Subject: [PATCH 1/3] Allow any type of key and missing values --- .../WebonyxGraphQLSyncPromiseAdapter.php | 2 +- src/DataLoader.php | 11 +---- .../Webonyx/GraphQL/SyncPromiseAdapter.php | 48 +++++++++++++++++++ 3 files changed, 51 insertions(+), 10 deletions(-) diff --git a/lib/promise-adapter/src/Adapter/WebonyxGraphQLSyncPromiseAdapter.php b/lib/promise-adapter/src/Adapter/WebonyxGraphQLSyncPromiseAdapter.php index 7fb5eaa..dd60566 100644 --- a/lib/promise-adapter/src/Adapter/WebonyxGraphQLSyncPromiseAdapter.php +++ b/lib/promise-adapter/src/Adapter/WebonyxGraphQLSyncPromiseAdapter.php @@ -32,7 +32,7 @@ class WebonyxGraphQLSyncPromiseAdapter implements PromiseAdapterInterface public function __construct(?SyncPromiseAdapter $webonyxPromiseAdapter = null) { - $webonyxPromiseAdapter = $webonyxPromiseAdapter?:new SyncPromiseAdapter(); + $webonyxPromiseAdapter = $webonyxPromiseAdapter?:new \Overblog\DataLoader\Promise\Adapter\Webonyx\GraphQL\SyncPromiseAdapter(); $this->setWebonyxPromiseAdapter($webonyxPromiseAdapter); } diff --git a/src/DataLoader.php b/src/DataLoader.php index 7cd10a3..3ff4dd9 100644 --- a/src/DataLoader.php +++ b/src/DataLoader.php @@ -426,18 +426,11 @@ function ($values) use ($keys, $queue) { sprintf('not return a Promise of an Array: %s.', gettype($values)) ); } - if (count($values) !== count($keys)) { - throw new \RuntimeException( - 'DataLoader must be constructed with a function which accepts ' . - 'Array and returns Promise>, but the function did ' . - 'not return a Promise of an Array of the same length as the Array of keys.' - ); - } // Step through the values, resolving or rejecting each Promise in the // loaded queue. - foreach ($queue as $index => $data) { - $value = $values[$index]; + foreach ($queue as $data) { + $value = $values[$data['key']] ?? null; if ($value instanceof \Throwable) { $data['reject']($value); } else { diff --git a/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php b/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php index da42184..314ae51 100644 --- a/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php +++ b/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php @@ -11,9 +11,17 @@ namespace Overblog\DataLoader\Promise\Adapter\Webonyx\GraphQL; +use Countable; +use Ds\Map; +use Exception; +use GraphQL\Error\InvariantViolation; +use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter as BaseSyncPromiseAdapter; use GraphQL\Executor\Promise\Promise; use Overblog\DataLoader\DataLoader; +use Throwable; +use function assert; +use function is_array; class SyncPromiseAdapter extends BaseSyncPromiseAdapter { @@ -26,4 +34,44 @@ protected function onWait(Promise $promise): void { DataLoader::await(); } + + /** @throws InvariantViolation */ + public function all(iterable $promisesOrValues): Promise + { + assert(is_countable($promisesOrValues)); + + $all = new SyncPromise(); + + $total = count($promisesOrValues); + $count = 0; + /** @var Map $result */ + $result = new Map(); + + $resolveAllWhenFinished = function () use (&$count, &$total, $all, &$result, $promisesOrValues): void { + if ($count === $total) { + $all->resolve(is_array($promisesOrValues) ? $result->toArray() : $result); + } + }; + + foreach ($promisesOrValues as $index => $promiseOrValue) { + if ($promiseOrValue instanceof Promise) { + $result->put($index, null); + $promiseOrValue->then( + static function ($value) use ($result, $index, &$count, &$resolveAllWhenFinished): void { + $result->put($index, $value); + ++$count; + $resolveAllWhenFinished(); + }, + static fn (Throwable $reason): SyncPromise => $all->reject($reason) + ); + } else { + $result->put($index, $promiseOrValue); + ++$count; + } + } + + $resolveAllWhenFinished(); + + return new Promise($all, $this); + } } From 39fa28c9399cc909463b0e194e17d731cbbad96d Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Mon, 17 Aug 2026 15:10:16 +0200 Subject: [PATCH 2/3] fix: support positional and keyed batch results --- src/DataLoader.php | 69 +++++++++++++------ .../Webonyx/GraphQL/SyncPromiseAdapter.php | 48 ------------- tests/AbuseTestCase.php | 9 ++- tests/DataLoadTestCase.php | 18 +++++ 4 files changed, 71 insertions(+), 73 deletions(-) diff --git a/src/DataLoader.php b/src/DataLoader.php index 3ff4dd9..4d8a63c 100644 --- a/src/DataLoader.php +++ b/src/DataLoader.php @@ -417,32 +417,61 @@ private function dispatchQueueBatch(array $queue) // Await the resolution of the call to batchLoadFn. $batchPromise->then( - function ($values) use ($keys, $queue) { - // Assert the expected resolution from batchLoadFn. - if (!is_array($values) && !$values instanceof \Traversable) { - throw new \RuntimeException( - 'DataLoader must be constructed with a function which accepts ' . - 'Array and returns Promise>, but the function did ' . - sprintf('not return a Promise of an Array: %s.', gettype($values)) - ); - } - - // Step through the values, resolving or rejecting each Promise in the - // loaded queue. - foreach ($queue as $data) { - $value = $values[$data['key']] ?? null; - if ($value instanceof \Throwable) { - $data['reject']($value); - } else { - $data['resolve']($value); - } - }; + function ($values) use ($queue) { + $this->resolveDispatchedBatch($values, $queue); } )->then(null, function ($error) use ($queue) { $this->failedDispatch($queue, $error); }); } + /** + * Fan a resolved batch result out to the individual queued promises. + * + * @param mixed $values + * @param array $queue + */ + private function resolveDispatchedBatch($values, $queue) + { + // Assert the expected resolution from batchLoadFn. + if (!is_array($values) && !$values instanceof \Traversable) { + throw new \RuntimeException( + 'DataLoader must be constructed with a function which accepts ' . + 'Array and returns Promise>, but the function did ' . + sprintf('not return a Promise of an Array: %s.', gettype($values)) + ); + } + + if ($values instanceof \Traversable && !$values instanceof \ArrayAccess) { + $values = iterator_to_array($values); + } + + // Step through the values, resolving or rejecting each Promise in the + // loaded queue. + foreach ($queue as $index => $data) { + $key = $data['key']; + if (is_array($values)) { + if (array_is_list($values)) { + $value = $values[$index] ?? null; + } elseif (is_int($key) || is_string($key) || is_float($key) || is_bool($key)) { + $value = $values[$key] ?? null; + } else { + $value = null; + } + } elseif ($values instanceof \ArrayAccess) { + $value = $values->offsetExists($key) ? $values->offsetGet($key) : null; + } else { + $value = null; + } + + if ($value instanceof \Throwable) { + $data['reject']($value); + } else { + $data['resolve']($value); + } + } + } + /** * Do not cache individual loads if the entire batch dispatch fails, * but still reject each request so they do not hang. diff --git a/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php b/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php index 314ae51..da42184 100644 --- a/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php +++ b/src/Promise/Adapter/Webonyx/GraphQL/SyncPromiseAdapter.php @@ -11,17 +11,9 @@ namespace Overblog\DataLoader\Promise\Adapter\Webonyx\GraphQL; -use Countable; -use Ds\Map; -use Exception; -use GraphQL\Error\InvariantViolation; -use GraphQL\Executor\Promise\Adapter\SyncPromise; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter as BaseSyncPromiseAdapter; use GraphQL\Executor\Promise\Promise; use Overblog\DataLoader\DataLoader; -use Throwable; -use function assert; -use function is_array; class SyncPromiseAdapter extends BaseSyncPromiseAdapter { @@ -34,44 +26,4 @@ protected function onWait(Promise $promise): void { DataLoader::await(); } - - /** @throws InvariantViolation */ - public function all(iterable $promisesOrValues): Promise - { - assert(is_countable($promisesOrValues)); - - $all = new SyncPromise(); - - $total = count($promisesOrValues); - $count = 0; - /** @var Map $result */ - $result = new Map(); - - $resolveAllWhenFinished = function () use (&$count, &$total, $all, &$result, $promisesOrValues): void { - if ($count === $total) { - $all->resolve(is_array($promisesOrValues) ? $result->toArray() : $result); - } - }; - - foreach ($promisesOrValues as $index => $promiseOrValue) { - if ($promiseOrValue instanceof Promise) { - $result->put($index, null); - $promiseOrValue->then( - static function ($value) use ($result, $index, &$count, &$resolveAllWhenFinished): void { - $result->put($index, $value); - ++$count; - $resolveAllWhenFinished(); - }, - static fn (Throwable $reason): SyncPromise => $all->reject($reason) - ); - } else { - $result->put($index, $promiseOrValue); - ++$count; - } - } - - $resolveAllWhenFinished(); - - return new Promise($all, $this); - } } diff --git a/tests/AbuseTestCase.php b/tests/AbuseTestCase.php index dadd0a0..dfd9c8f 100644 --- a/tests/AbuseTestCase.php +++ b/tests/AbuseTestCase.php @@ -84,14 +84,13 @@ public function testBatchFunctionMustReturnAPromiseOfAnArrayNotNull() /** * @group provides-descriptive-error-messages-for-api-abuse */ - public function testBatchFunctionMustPromiseAnArrayOfCorrectLength() + public function testBatchFunctionMayReturnMissingValues() { - $this->expectException(RuntimeException::class); - $this->expectExceptionMessage('DataLoader must be constructed with a function which accepts Array and returns Promise>, but the function did not return a Promise of an Array of the same length as the Array of keys.'); - - DataLoader::await(self::idLoader(function () { + $value = DataLoader::await(self::idLoader(function () { return self::$promiseAdapter->createFulfilled([]); })->load(1)); + + $this->assertNull($value); } /** diff --git a/tests/DataLoadTestCase.php b/tests/DataLoadTestCase.php index 289ae25..5d8b3cd 100644 --- a/tests/DataLoadTestCase.php +++ b/tests/DataLoadTestCase.php @@ -51,6 +51,24 @@ public function testSupportsLoadingMultipleKeysInOneCall() $this->assertEquals([], DataLoader::await($promiseEmpty)); } + /** + * @group accepts-any-kind-of-key + */ + public function testSupportsKeyedResultsWithMissingValues() + { + $loader = new DataLoader(function () { + return self::$promiseAdapter->createFulfilled(['present' => 'value']); + }, self::$promiseAdapter); + + list($present, $missing) = DataLoader::await(self::$promiseAdapter->createAll([ + $loader->load('present'), + $loader->load('missing'), + ])); + + $this->assertEquals('value', $present); + $this->assertNull($missing); + } + /** * @group primary-api */ From d346e61172279c337b6a6215878c3f82bcafa98d Mon Sep 17 00:00:00 2001 From: Simon Podlipsky Date: Tue, 18 Aug 2026 09:41:05 +0200 Subject: [PATCH 3/3] docs: describe positional and keyed batch results --- README.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 7a03353..a057e7d 100644 --- a/README.md +++ b/README.md @@ -71,10 +71,13 @@ minimal outgoing data requests. #### Batch Function A batch loading function accepts an Array of keys, and returns a Promise which -resolves to an Array of values. There are a few constraints that must be upheld: +resolves to values in one of these forms: - * The Array of values must be the same length as the Array of keys. - * Each index in the Array of values must correspond to the same index in the Array of keys. + * A list with the same length and order as the Array of keys. + * A keyed Array or `ArrayAccess` value. Missing keys resolve to `null`. + +Object keys require positional list results or an `ArrayAccess` result that +supports those keys. For example, if your batch function was provided the Array of keys: `[ 2, 9, 6, 1 ]`, and loading from a back-end service returned the values: