diff --git a/src/Parameters/PrefetchDataParameter.php b/src/Parameters/PrefetchDataParameter.php index 4b233c5e3..9aaab6c7a 100644 --- a/src/Parameters/PrefetchDataParameter.php +++ b/src/Parameters/PrefetchDataParameter.php @@ -52,24 +52,31 @@ public function resolve(object|null $source, array $args, mixed $context, Resolv // So we record all of these ->resolve() calls, collect them together and when a value is actually // 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 ($info, $context, $args, $prefetchBuffer) { - if (! $prefetchBuffer->hasResult($args, $info)) { - $prefetchResult = $this->computePrefetch($args, $context, $info, $prefetchBuffer); - - $prefetchBuffer->storeResult($prefetchResult, $args, $info); + return new Deferred(function () use ($source, $info, $context, $args, $prefetchBuffer) { + if (! $prefetchBuffer->hasResult($source)) { + $this->processPrefetch($args, $context, $info, $prefetchBuffer); } - return $prefetchResult ?? $prefetchBuffer->getResult($args, $info); + $result = $prefetchBuffer->getResult($source); + // clear internal storage + $prefetchBuffer->purgeResult($source); + return $result; }); } /** @param array $args */ - private function computePrefetch(array $args, mixed $context, ResolveInfo $info, PrefetchBuffer $prefetchBuffer): mixed + private function processPrefetch(array $args, mixed $context, ResolveInfo $info, PrefetchBuffer $prefetchBuffer): void { $sources = $prefetchBuffer->getObjectsByArguments($args, $info); + $prefetchBuffer->purge($args, $info); $toPassPrefetchArgs = QueryField::paramsToArguments($this->fieldName, $this->parameters, null, $args, $context, $info, $this->resolver); - return ($this->resolver)($sources, ...$toPassPrefetchArgs); + $resolvedValues = ($this->resolver)($sources, ...$toPassPrefetchArgs); + + foreach ($sources as $source) { + // map results to each source to support old prefetch behavior + $prefetchBuffer->storeResult($source, $resolvedValues); + } } /** @inheritDoc */ diff --git a/src/PrefetchBuffer.php b/src/PrefetchBuffer.php index 8b1afb02a..69ebcb706 100644 --- a/src/PrefetchBuffer.php +++ b/src/PrefetchBuffer.php @@ -5,8 +5,8 @@ namespace TheCodingMachine\GraphQLite; use GraphQL\Type\Definition\ResolveInfo; +use SplObjectStorage; -use function array_key_exists; use function md5; use function serialize; @@ -18,8 +18,13 @@ class PrefetchBuffer /** @var array> An array of buffered, indexed by hash of arguments. */ private array $objects = []; - /** @var array An array of prefetch method results, indexed by hash of arguments. */ - private array $results = []; + /** @var SplObjectStorage A Storage of prefetch method results, holds source to resolved values. */ + private SplObjectStorage $results; + + public function __construct() + { + $this->results = new SplObjectStorage(); + } /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function register( @@ -66,28 +71,28 @@ 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[$this->computeHash($arguments, $info)] = $result; + $this->results->offsetSet($source, $result); } - /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function hasResult( - array $arguments, - ResolveInfo|null $info = null, + object $source, ): bool { - return array_key_exists($this->computeHash($arguments, $info), $this->results); + return $this->results->offsetExists($source); } - /** @param array $arguments The input arguments passed from GraphQL to the field. */ public function getResult( - array $arguments, - ResolveInfo|null $info = null, + object $source, ): mixed { - return $this->results[$this->computeHash($arguments, $info)]; + return $this->results->offsetGet($source); + } + + public function purgeResult( + object $source, + ): void { + $this->results->offsetUnset($source); } } diff --git a/src/QueryField.php b/src/QueryField.php index b95a9152f..5a224b70a 100644 --- a/src/QueryField.php +++ b/src/QueryField.php @@ -4,11 +4,9 @@ namespace TheCodingMachine\GraphQLite; -use GraphQL\Deferred; +use Closure; use GraphQL\Error\ClientAware; use GraphQL\Executor\Promise\Adapter\SyncPromise; -use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; -use GraphQL\Executor\Promise\Promise; use GraphQL\Language\AST\FieldDefinitionNode; use GraphQL\Type\Definition\FieldDefinition; use GraphQL\Type\Definition\ListOfType; @@ -22,9 +20,6 @@ use TheCodingMachine\GraphQLite\Parameters\ParameterInterface; use TheCodingMachine\GraphQLite\Parameters\SourceParameter; -use function array_filter; -use function array_map; - /** * A GraphQL field that maps to a PHP method automatically. * @@ -79,35 +74,15 @@ public function __construct( $callResolver = function (...$args) use ($originalResolver, $source, $resolver) { $result = $resolver($source, ...$args); - try { - $this->assertReturnType($result); - } catch (TypeMismatchRuntimeException $e) { - $e->addInfo($this->name, $originalResolver->toString()); - - throw $e; - } - - return $result; + return $this->resolveWithPromise($result, $originalResolver); }; - $deferred = (bool) array_filter($toPassArgs, static fn (mixed $value) => $value instanceof SyncPromise); - // GraphQL allows deferring resolving the field's value using promises, i.e. they call the resolve // function ahead of time for all of the fields (allowing us to gather all calls and do something // in batch, like prefetch) and then resolve the promises as needed. To support that for prefetch, // we're checking if any of the resolved parameters returned a promise. If they did, we know // that the value should also be resolved using a promise, so we're wrapping it in one. - return $deferred ? new Deferred(static function () use ($toPassArgs, $callResolver) { - $syncPromiseAdapter = new SyncPromiseAdapter(); - - // Wait for every deferred parameter. - $toPassArgs = array_map( - static fn (mixed $value) => $value instanceof SyncPromise ? $syncPromiseAdapter->wait(new Promise($value, $syncPromiseAdapter)) : $value, - $toPassArgs, - ); - - return $callResolver(...$toPassArgs); - }) : $callResolver(...$toPassArgs); + return $this->deferred($toPassArgs, $callResolver); }; $config += $additionalConfig; @@ -115,6 +90,23 @@ public function __construct( parent::__construct($config); } + private function resolveWithPromise(mixed $result, ResolverInterface $originalResolver): mixed + { + if ($result instanceof SyncPromise) { + return $result->then(fn ($resolvedValue) => $this->resolveWithPromise($resolvedValue, $originalResolver)); + } + + try { + $this->assertReturnType($result); + } catch (TypeMismatchRuntimeException $e) { + $e->addInfo($this->name, $originalResolver->toString()); + + throw $e; + } + + return $result; + } + /** * This method checks the returned value of the resolver to be sure it matches the documented return type. * We are sure the returned value is of the correct type... except if the return type is type-hinted as an array. @@ -204,4 +196,24 @@ public static function paramsToArguments(string $name, array $parameters, object return $toPassArgs; } + + /** + * @param array $toPassArgs + * Create Deferred if any of arguments contain promise + */ + public static function deferred(array $toPassArgs, Closure $callResolver): mixed + { + $deferredArgument = null; + foreach ($toPassArgs as $position => $toPassArg) { + if ($toPassArg instanceof SyncPromise) { + $deferredArgument = $toPassArg->then(static function ($resolvedValue) use ($toPassArgs, $position, $callResolver) { + $toPassArgs[$position] = $resolvedValue; + return self::deferred($toPassArgs, $callResolver); + }); + break; + } + } + + return $deferredArgument ?? $callResolver(...$toPassArgs); + } } diff --git a/tests/Fixtures/Integration/Controllers/BlogController.php b/tests/Fixtures/Integration/Controllers/BlogController.php new file mode 100644 index 000000000..1f358aa69 --- /dev/null +++ b/tests/Fixtures/Integration/Controllers/BlogController.php @@ -0,0 +1,21 @@ +id; + } + + /** + * @param Post[][] $prefetchedPosts + * + * @return Post[] + */ + #[Field] + public function getPosts( + #[Prefetch('prefetchPosts')] + array $prefetchedPosts, + ): array { + return $prefetchedPosts[$this->id]; + } + + /** @param Blog[][] $prefetchedSubBlogs */ + #[Field(outputType: '[Blog!]!')] + public function getSubBlogs( + #[Prefetch('prefetchSubBlogs')] + array $prefetchedSubBlogs, + ): Deferred { + return new Deferred(fn () => $prefetchedSubBlogs[$this->id]); + } + + /** + * @param Blog[] $blogs + * + * @return Post[][] + */ + public static function prefetchPosts(iterable $blogs): array + { + $posts = []; + foreach ($blogs as $blog) { + $blogId = $blog->getId(); + $posts[$blog->getId()] = [ + new Post('post-' . $blogId . '.1'), + new Post('post-' . $blogId . '.2'), + ]; + } + + return $posts; + } + + /** + * @param Blog[] $blogs + * + * @return Blog[][] + */ + public static function prefetchSubBlogs(iterable $blogs): array + { + $subBlogs = []; + foreach ($blogs as $blog) { + $blogId = $blog->getId(); + $subBlogId = $blogId * 10; + $subBlogs[$blog->id] = [new Blog($subBlogId)]; + } + + return $subBlogs; + } +} diff --git a/tests/Fixtures/Integration/Models/Comment.php b/tests/Fixtures/Integration/Models/Comment.php new file mode 100644 index 000000000..094e6e571 --- /dev/null +++ b/tests/Fixtures/Integration/Models/Comment.php @@ -0,0 +1,23 @@ +text; + } +} diff --git a/tests/Fixtures/Integration/Types/CompanyType.php b/tests/Fixtures/Integration/Types/CompanyType.php index 31a042479..fbada7f2a 100644 --- a/tests/Fixtures/Integration/Types/CompanyType.php +++ b/tests/Fixtures/Integration/Types/CompanyType.php @@ -13,22 +13,27 @@ #[ExtendType(class:Company::class)] class CompanyType { - #[Field] public function getName(Company $company): string { return $company->name; } + /** @param Contact[] $contacts */ #[Field] public function getContact( Company $company, #[Prefetch('prefetchContacts')] - array $contacts - ): ?Contact { + array $contacts, + ): Contact|null { return $contacts[$company->name] ?? null; } + /** + * @param Company[] $companies + * + * @return Contact[] + */ public static function prefetchContacts(array $companies): array { $contacts = []; diff --git a/tests/Fixtures/Integration/Types/PostType.php b/tests/Fixtures/Integration/Types/PostType.php index 565c783ec..45e9576bb 100644 --- a/tests/Fixtures/Integration/Types/PostType.php +++ b/tests/Fixtures/Integration/Types/PostType.php @@ -4,8 +4,13 @@ namespace TheCodingMachine\GraphQLite\Fixtures\Integration\Types; +use Exception; +use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; +use GraphQL\Executor\Promise\Promise; use TheCodingMachine\GraphQLite\Annotations\ExtendType; use TheCodingMachine\GraphQLite\Annotations\Field; +use TheCodingMachine\GraphQLite\Annotations\Prefetch; +use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Comment; use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Post; #[ExtendType(class:Post::class)] @@ -22,4 +27,36 @@ public function getTitle(Post $post): string { return $post->title; } + + /** + * @param Comment[][] $prefetchedComments + * + * @return Comment[] + */ + #[Field] + public function getComments( + Post $post, + #[Prefetch('prefetchComments')] + array $prefetchedComments, + ): array { + return $prefetchedComments[$post->title]; + } + + /** + * @param Post[] $posts + * + * @return Promise[] + * + * @throws Exception + */ + public static function prefetchComments(array $posts): Promise + { + $syncPromiseAdapter = new SyncPromiseAdapter(); + $result = []; + foreach ($posts as $post) { + $result[$post->title] = [new Comment('comment for ' . $post->title)]; + } + + return $syncPromiseAdapter->createFulfilled($result); + } } diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 0d9f2086c..24a055dd1 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -2339,4 +2339,119 @@ public function testEndToEndSubscriptionWithInput(): void 'contactAddedWithFilter' => null, ], $this->getSuccessResult($result)); } + + public function testPrefetchingOfSameTypeInDifferentNestingLevels(): void + { + $schema = $this->mainContainer->get(Schema::class); + assert($schema instanceof Schema); + + $schema->assertValid(); + + $queryString = ' + query { + blogs { + id + subBlogs { + id + posts { + title + comments { + text + } + } + } + posts { + title + comments { + text + } + } + } + } + '; + + $result = GraphQL::executeQuery( + $schema, + $queryString, + null, + new Context(), + ); + + $this->assertSame([ + 'blogs' => [ + [ + 'id' => '1', + 'subBlogs' => [ + [ + 'id' => '10', + 'posts' => [ + [ + 'title' => 'post-10.1', + 'comments' => [ + ['text' => 'comment for post-10.1'], + ], + ], + [ + 'title' => 'post-10.2', + 'comments' => [ + ['text' => 'comment for post-10.2'], + ], + ], + ], + ], + ], + 'posts' => [ + [ + 'title' => 'post-1.1', + 'comments' => [ + ['text' => 'comment for post-1.1'], + ], + ], + [ + 'title' => 'post-1.2', + 'comments' => [ + ['text' => 'comment for post-1.2'], + ], + ], + ], + ], + [ + 'id' => '2', + 'subBlogs' => [ + [ + 'id' => '20', + 'posts' => [ + [ + 'title' => 'post-20.1', + 'comments' => [ + ['text' => 'comment for post-20.1'], + ], + ], + [ + 'title' => 'post-20.2', + 'comments' => [ + ['text' => 'comment for post-20.2'], + ], + ], + ], + ], + ], + 'posts' => [ + [ + 'title' => 'post-2.1', + 'comments' => [ + ['text' => 'comment for post-2.1'], + ], + ], + [ + 'title' => 'post-2.2', + 'comments' => [ + ['text' => 'comment for post-2.2'], + ], + ], + ], + ], + ], + ], $this->getSuccessResult($result)); + } } diff --git a/tests/Parameters/PrefetchDataParameterTest.php b/tests/Parameters/PrefetchDataParameterTest.php index 9d241b7cc..a7e875c76 100644 --- a/tests/Parameters/PrefetchDataParameterTest.php +++ b/tests/Parameters/PrefetchDataParameterTest.php @@ -2,7 +2,6 @@ namespace TheCodingMachine\GraphQLite\Parameters; -use Generator; use GraphQL\Deferred; use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter; use GraphQL\Executor\Promise\Promise; @@ -11,8 +10,6 @@ use PHPUnit\Framework\TestCase; use stdClass; use TheCodingMachine\GraphQLite\Context\Context; -use TheCodingMachine\GraphQLite\Middlewares\MissingAuthorizationException; -use TheCodingMachine\GraphQLite\Security\AuthenticationServiceInterface; use TheCodingMachine\GraphQLite\Types\ArgumentResolver; class PrefetchDataParameterTest extends TestCase @@ -32,7 +29,7 @@ public function testResolveWithExistingResult(): void ]; $buffer = $context->getPrefetchBuffer($parameter); - $buffer->storeResult($prefetchResult, $args); + $buffer->storeResult($source, $prefetchResult); $resolvedParameterPromise = $parameter->resolve($source, $args, $context, $this->createStub(ResolveInfo::class)); @@ -71,10 +68,10 @@ public function testResolveWithoutExistingResult(): void $resolvedParameterPromise = $parameter->resolve($source, $args, $context, $this->createStub(ResolveInfo::class)); - self::assertFalse($buffer->hasResult($args)); + self::assertFalse($buffer->hasResult($source)); self::assertSame([$source], $buffer->getObjectsByArguments($args)); self::assertSame($prefetchResult, $this->deferredValue($resolvedParameterPromise)); - self::assertTrue($buffer->hasResult($args)); + self::assertFalse($buffer->hasResult($source)); } private function deferredValue(Deferred $promise): mixed diff --git a/tests/SchemaFactoryTest.php b/tests/SchemaFactoryTest.php index 78defd5b7..03678fa1f 100644 --- a/tests/SchemaFactoryTest.php +++ b/tests/SchemaFactoryTest.php @@ -18,6 +18,7 @@ use TheCodingMachine\GraphQLite\Containers\BasicAutoWiringContainer; use TheCodingMachine\GraphQLite\Containers\EmptyContainer; use TheCodingMachine\GraphQLite\Fixtures\Integration\Controllers\ContactController; +use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Comment; use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Company; use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Contact; use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Post; @@ -118,6 +119,7 @@ public function testCreateSchemaOnlyWithFactories(): void ContactFactory::class, ContactOtherType::class, ContactType::class, + Comment::class, Post::class, PostType::class, Company::class, diff --git a/website/docs/type-mapping.mdx b/website/docs/type-mapping.mdx index 26127719d..d5b6c4dc2 100644 --- a/website/docs/type-mapping.mdx +++ b/website/docs/type-mapping.mdx @@ -330,6 +330,47 @@ query { } ``` +## Promise mapping + +GraphQL includes a native \GraphQL\Deferred type. +You can map the return type by adding a detailed `@return` statement in the PHPDoc. +An alternative to the `@return` statement is using `#[Field(outputType: SomeGQLType)]`. + +All the previously mentioned mappings work with Promises, except when a return type is explicitly declared +in the method signature. + +This allows you to use \Overblog\DataLoader\DataLoader as an alternative +for resolving N+1 query issues and caching intermediate results. + +```php +#[Type] +class Product +{ + // ... + + /** + * @return string + */ + #[Field] + public function getName(): Deferred + { + return new Deferred(fn() => $this->name); + } + + #[Field(outputType: "Float")] + public function getPrice(): Deferred + { + return new Deferred(fn() => $this->price); + } + + #[Field(outputType: "[String!]!")] + public function getCategories(#[Autowire('categoryDataLoader')] DataLoader $categoryDataLoader): SyncPromise + { + return $categoryDataLoader->load($this->id)->adoptedPromise; + } +} +``` + ## More scalar types Available in GraphQLite 4.0+