From cfa9b82eba1f6f3d0f4d090ebd41a39b54640f96 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 12:02:39 +0200 Subject: [PATCH 01/23] split validation rules into two parts: cacheable and non-cacheable --- .../CacheableValidationRulesProvider.php | 36 +++++++++++++++++++ src/GraphQL.php | 33 +++++++++++++++++ src/LighthouseServiceProvider.php | 4 ++- .../ProvidesCacheableValidationRules.php | 27 ++++++++++++++ 4 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 src/Execution/CacheableValidationRulesProvider.php create mode 100644 src/Support/Contracts/ProvidesCacheableValidationRules.php diff --git a/src/Execution/CacheableValidationRulesProvider.php b/src/Execution/CacheableValidationRulesProvider.php new file mode 100644 index 000000000..53e8c23ef --- /dev/null +++ b/src/Execution/CacheableValidationRulesProvider.php @@ -0,0 +1,36 @@ + new QueryDepth($this->configRepository->get('lighthouse.security.max_query_depth', 0)), + DisableIntrospection::class => new DisableIntrospection($this->configRepository->get('lighthouse.security.disable_introspection', 0)), + ] + DocumentValidator::allRules(); + + unset($result[QueryComplexity::class]); + return $result; + } + + public function validationRules(): ?array + { + return [ + QueryComplexity::class => new QueryComplexity($this->configRepository->get('lighthouse.security.max_query_complexity', 0)), + ]; + } +} diff --git a/src/GraphQL.php b/src/GraphQL.php index 91fc506bf..4b006db46 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -12,6 +12,9 @@ use GraphQL\Server\Helper as GraphQLHelper; use GraphQL\Server\OperationParams; use GraphQL\Server\RequestError; +use GraphQL\Type\Schema as SchemaType; +use GraphQL\Validator\DocumentValidator; +use GraphQL\Validator\Rules\QueryComplexity; use Illuminate\Container\Container; use Illuminate\Contracts\Cache\Factory as CacheFactory; use Illuminate\Contracts\Config\Repository as ConfigRepository; @@ -29,6 +32,7 @@ use Nuwave\Lighthouse\Schema\SchemaBuilder; use Nuwave\Lighthouse\Schema\Values\FieldValue; use Nuwave\Lighthouse\Support\Contracts\GraphQLContext; +use Nuwave\Lighthouse\Support\Contracts\ProvidesCacheableValidationRules; use Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules; use Nuwave\Lighthouse\Support\Utils as LighthouseUtils; @@ -128,6 +132,11 @@ public function executeParsedQueryRaw( new StartExecution($schema, $query, $variables, $operationName, $context), ); + $errors = $this->executeAndCacheValidationRules($schema, $query); + if ($errors !== []) { + return new ExecutionResult(null, $errors); + } + $result = GraphQLBase::executeQuery( $schema, $query, @@ -373,4 +382,28 @@ protected function parseQuery(string $query): DocumentNode 'noLocation' => ! $this->configRepository->get('lighthouse.parse_source_location'), ]); } + + + /** + * Execute the validation rules that are cacheable. + * + * @return array + * + * @throws \Exception + */ + protected function executeAndCacheValidationRules(SchemaType $schema, DocumentNode $query): array + { + if (!$this->providesValidationRules instanceof ProvidesCacheableValidationRules) { + return []; + } + + $validationRules = $this->providesValidationRules->cacheableValidationRules(); + foreach ($validationRules as $rule) { + if ($rule instanceof QueryComplexity) { + throw new \InvalidArgumentException("QueryComplexity rule should not be registered in cacheableValidationRules"); + } + } + + return DocumentValidator::validate($schema, $query, $validationRules); + } } diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index 53ae87037..b6ece653f 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -27,6 +27,7 @@ use Nuwave\Lighthouse\Console\ValidateSchemaCommand; use Nuwave\Lighthouse\Console\ValidatorCommand; use Nuwave\Lighthouse\Events\RegisterDirectiveNamespaces; +use Nuwave\Lighthouse\Execution\CacheableValidationRulesProvider; use Nuwave\Lighthouse\Execution\ContextFactory; use Nuwave\Lighthouse\Execution\ContextSerializer; use Nuwave\Lighthouse\Execution\ErrorPool; @@ -100,7 +101,8 @@ public function provideSubscriptionResolver(FieldValue $fieldValue): \Closure } }); - $this->app->bind(ProvidesValidationRules::class, ValidationRulesProvider::class); + //$this->app->bind(ProvidesValidationRules::class, ValidationRulesProvider::class); + $this->app->bind(ProvidesValidationRules::class, CacheableValidationRulesProvider::class); $this->commands(self::COMMANDS); } diff --git a/src/Support/Contracts/ProvidesCacheableValidationRules.php b/src/Support/Contracts/ProvidesCacheableValidationRules.php new file mode 100644 index 000000000..55fe4b415 --- /dev/null +++ b/src/Support/Contracts/ProvidesCacheableValidationRules.php @@ -0,0 +1,27 @@ + + */ + public function cacheableValidationRules(): array; + + /** + * A set of rules for the second query validation step. + * + * These rules would always be executed and not cached. + * + * Returning `null` enables all available rules. + * Empty array skips query validation entirely. + * + * @return array|null + */ + public function validationRules(): ?array; +} From 09ecb9eecfdd2ba0e4dc628272b33ed731ea6b40 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 12:20:14 +0200 Subject: [PATCH 02/23] save schema hash in cache and expose it via SchemaBuilder --- src/GraphQL.php | 2 ++ src/Schema/AST/DocumentAST.php | 10 ++++++++++ src/Schema/SchemaBuilder.php | 5 +++++ tests/Unit/Schema/AST/DocumentASTTest.php | 4 ++++ tests/Unit/Schema/SchemaBuilderTest.php | 3 ++- 5 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index 4b006db46..c11958746 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -404,6 +404,8 @@ protected function executeAndCacheValidationRules(SchemaType $schema, DocumentNo } } + //$schemaHash = $this->schemaBuilder->schemaHash(); + return DocumentValidator::validate($schema, $query, $validationRules); } } diff --git a/src/Schema/AST/DocumentAST.php b/src/Schema/AST/DocumentAST.php index 5cd7cbb71..95b7d8eb0 100644 --- a/src/Schema/AST/DocumentAST.php +++ b/src/Schema/AST/DocumentAST.php @@ -46,6 +46,8 @@ class DocumentAST implements Arrayable public const SCHEMA_EXTENSIONS = 'schemaExtensions'; + public const HASH = 'hash'; + /** * The types within the schema. * @@ -88,6 +90,11 @@ class DocumentAST implements Arrayable /** @var array */ public array $schemaExtensions = []; + /** + * A hash of the schema. + */ + public ?string $hash = null; + /** Create a new DocumentAST instance from a schema. */ public static function fromSource(string $schema): self { @@ -104,6 +111,7 @@ public static function fromSource(string $schema): self } $instance = new static(); + $instance->hash = hash('sha256', $schema); foreach ($documentNode->definitions as $definition) { if ($definition instanceof TypeDefinitionNode) { @@ -195,6 +203,7 @@ public function toArray(): array self::DIRECTIVES => array_map([AST::class, 'toArray'], $this->directives), self::CLASS_NAME_TO_OBJECT_TYPE_NAME => $this->classNameToObjectTypeNames, self::SCHEMA_EXTENSIONS => array_map([AST::class, 'toArray'], $this->schemaExtensions), + self::HASH => $this->hash, ]; } @@ -231,6 +240,7 @@ protected function hydrateFromArray(array $ast): void self::DIRECTIVES => $directives, self::CLASS_NAME_TO_OBJECT_TYPE_NAME => $this->classNameToObjectTypeNames, self::SCHEMA_EXTENSIONS => $schemaExtensions, + self::HASH => $this->hash, ] = $ast; // Utilize the NodeList for lazy unserialization for performance gains. diff --git a/src/Schema/SchemaBuilder.php b/src/Schema/SchemaBuilder.php index 2523c2a41..03e81bb93 100644 --- a/src/Schema/SchemaBuilder.php +++ b/src/Schema/SchemaBuilder.php @@ -28,6 +28,11 @@ public function schema(): Schema ); } + public function schemaHash(): ?string + { + return $this->astBuilder->documentAST()->hash; + } + /** Build an executable schema from an AST. */ protected function build(DocumentAST $documentAST): Schema { diff --git a/tests/Unit/Schema/AST/DocumentASTTest.php b/tests/Unit/Schema/AST/DocumentASTTest.php index 502a3add0..de7477dc9 100644 --- a/tests/Unit/Schema/AST/DocumentASTTest.php +++ b/tests/Unit/Schema/AST/DocumentASTTest.php @@ -28,6 +28,8 @@ public function testParsesSimpleSchema(): void ObjectTypeDefinitionNode::class, $documentAST->types[RootType::QUERY], ); + + $this->assertNotNull($documentAST->hash); } public function testThrowsOnInvalidSchema(): void @@ -111,5 +113,7 @@ public function testBeSerialized(): void $schemaExtension = $reserialized->schemaExtensions[0]; $this->assertInstanceOf(SchemaExtensionNode::class, $schemaExtension); $this->assertInstanceOf(DirectiveNode::class, $schemaExtension->directives[0]); + + $this->assertEquals($documentAST->hash, $reserialized->hash); } } diff --git a/tests/Unit/Schema/SchemaBuilderTest.php b/tests/Unit/Schema/SchemaBuilderTest.php index 82e0d55e8..55d9e353f 100644 --- a/tests/Unit/Schema/SchemaBuilderTest.php +++ b/tests/Unit/Schema/SchemaBuilderTest.php @@ -9,6 +9,7 @@ use GraphQL\Type\Definition\InterfaceType; use GraphQL\Type\Definition\ObjectType; use Nuwave\Lighthouse\Schema\RootType; +use Nuwave\Lighthouse\Schema\SchemaBuilder; use Tests\TestCase; final class SchemaBuilderTest extends TestCase @@ -18,7 +19,7 @@ public function testGeneratesValidSchema(): void $this->buildSchemaWithPlaceholderQuery('') ->assertValid(); - $this->expectNotToPerformAssertions(); + $this->assertNotNull($this->app->make(SchemaBuilder::class)->schemaHash()); } public function testGeneratesWithEmptyQueryType(): void From fe1a8561f7df66167ec9e07e00ea25ef1329d18a Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 13:40:42 +0200 Subject: [PATCH 03/23] query hashing and tests --- src/GraphQL.php | 47 +++-- src/lighthouse.php | 26 +++ tests/Integration/ValidationCachingTest.php | 204 ++++++++++++++++++++ 3 files changed, 266 insertions(+), 11 deletions(-) create mode 100644 tests/Integration/ValidationCachingTest.php diff --git a/src/GraphQL.php b/src/GraphQL.php index c11958746..77e117e81 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -80,14 +80,14 @@ public function executeQueryString( ?string $operationName = null, ): array { try { - $parsedQuery = $this->parse($query); + $parsedQuery = $this->parse($query, $queryHash); } catch (SyntaxError $syntaxError) { return $this->toSerializableArray( new ExecutionResult(null, [$syntaxError]), ); } - return $this->executeParsedQuery($parsedQuery, $context, $variables, $root, $operationName); + return $this->executeParsedQuery($parsedQuery, $context, $variables, $root, $operationName, $queryHash); } /** @@ -105,8 +105,9 @@ public function executeParsedQuery( ?array $variables = [], mixed $root = null, ?string $operationName = null, + ?string $queryHash = null, ): array { - $result = $this->executeParsedQueryRaw($query, $context, $variables, $root, $operationName); + $result = $this->executeParsedQueryRaw($query, $context, $variables, $root, $operationName, $queryHash); return $this->toSerializableArray($result); } @@ -122,6 +123,7 @@ public function executeParsedQueryRaw( ?array $variables = [], mixed $root = null, ?string $operationName = null, + ?string $queryHash = null, ): ExecutionResult { // Building the executable schema might take a while to do, // so we do it before we fire the StartExecution event. @@ -132,7 +134,7 @@ public function executeParsedQueryRaw( new StartExecution($schema, $query, $variables, $operationName, $context), ); - $errors = $this->executeAndCacheValidationRules($schema, $query); + $errors = $this->executeAndCacheValidationRules($schema, $this->schemaBuilder->schemaHash(), $query, $queryHash); if ($errors !== []) { return new ExecutionResult(null, $errors); } @@ -261,9 +263,10 @@ public function executeOperation(OperationParams $params, GraphQLContext $contex * * @api */ - public function parse(string $query): DocumentNode + public function parse(string $query, string &$hash = null): DocumentNode { $cacheConfig = $this->configRepository->get('lighthouse.query_cache'); + $hash = hash('sha256', $query); if (! $cacheConfig['enable']) { return $this->parseQuery($query); @@ -272,10 +275,8 @@ public function parse(string $query): DocumentNode $cacheFactory = Container::getInstance()->make(CacheFactory::class); $store = $cacheFactory->store($cacheConfig['store']); - $sha256 = hash('sha256', $query); - return $store->remember( - "lighthouse:query:{$sha256}", + "lighthouse:query:{$hash}", $cacheConfig['ttl'], fn (): DocumentNode => $this->parseQuery($query), ); @@ -391,7 +392,12 @@ protected function parseQuery(string $query): DocumentNode * * @throws \Exception */ - protected function executeAndCacheValidationRules(SchemaType $schema, DocumentNode $query): array + protected function executeAndCacheValidationRules( + SchemaType $schema, + ?string $schemaHash, + DocumentNode $query, + ?string $queryHash + ): array { if (!$this->providesValidationRules instanceof ProvidesCacheableValidationRules) { return []; @@ -404,8 +410,27 @@ protected function executeAndCacheValidationRules(SchemaType $schema, DocumentNo } } - //$schemaHash = $this->schemaBuilder->schemaHash(); + $cacheConfig = $this->configRepository->get('lighthouse.validation_cache'); + + if ($schemaHash === null || $queryHash === null || ! ($cacheConfig['enable'] ?? false)) { + return DocumentValidator::validate($schema, $query, $validationRules); + } + + $cacheKey = "lighthouse:validation:{$schemaHash}:{$queryHash}"; + + /** @var CacheFactory $cacheFactory */ + $cacheFactory = Container::getInstance()->make(CacheFactory::class); + $store = $cacheFactory->store($cacheConfig['store']); + if ($store->get($cacheKey) !== null){ + return []; + } + + $result = DocumentValidator::validate($schema, $query, $validationRules); + if ($result !== []) { + return $result; + } - return DocumentValidator::validate($schema, $query, $validationRules); + $store->put($cacheKey, true, $cacheConfig['ttl']); + return []; } } diff --git a/src/lighthouse.php b/src/lighthouse.php index 59c34d0eb..b0dd26ccd 100644 --- a/src/lighthouse.php +++ b/src/lighthouse.php @@ -134,6 +134,32 @@ 'ttl' => env('LIGHTHOUSE_QUERY_CACHE_TTL', 24 * 60 * 60), ], + /* + |-------------------------------------------------------------------------- + | Validation Cache + |-------------------------------------------------------------------------- + | + | Caches the result of validating queries to boost performance on subsequent requests. + | + */ + + 'validation_cache' => [ + /* + * Setting to true enables validation caching. + */ + 'enable' => env('LIGHTHOUSE_VALIDATION_CACHE_ENABLE', false), + + /* + * Allows using a specific cache store, uses the app's default if set to null. + */ + 'store' => env('LIGHTHOUSE_VALIDATION_CACHE_STORE', null), + + /* + * Duration in seconds the validation result should remain cached, null means forever. + */ + 'ttl' => env('LIGHTHOUSE_VALIDATION_CACHE_TTL', 24 * 60 * 60), + ], + /* |-------------------------------------------------------------------------- | Parse source location diff --git a/tests/Integration/ValidationCachingTest.php b/tests/Integration/ValidationCachingTest.php new file mode 100644 index 000000000..02b195f07 --- /dev/null +++ b/tests/Integration/ValidationCachingTest.php @@ -0,0 +1,204 @@ +app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 1); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 1); + + // second request should be hit + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 1); + Event::assertDispatchedTimes(CacheHit::class, 1); + Event::assertDispatchedTimes(KeyWritten::class, 1); + } + + public function testErrorsAreNotCached(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + bar + } + ')->assertGraphQLErrorMessage('Cannot query field "bar" on type "Query".'); + + Event::assertDispatchedTimes(CacheMissed::class, 1); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 0); + } + + public function testDifferentQueriesHasDifferentKeys(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 2); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 2); + } + + public function testSameSchemaAndSameQueryHaveSameKeys(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + $event = Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 1); + + // refresh container, but keep the same cache + $cacheFactory = $this->app->make(CacheFactory::class); + $this->refreshApplication(); + $this->app->instance(CacheFactory::class, $cacheFactory); + + $this->setUpTestSchema(); + + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 1); + $event->assertDispatchedTimes(KeyWritten::class, 1); + } + + public function testDifferentSchemasHasDifferentKeys(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 1); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 1); + + // refresh container + $this->refreshApplication(); + + $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' +type Query { + bar: String +} + +GRAPHQL; + $this->setUp(); + + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', true); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertGraphQLErrorMessage('Cannot query field "foo" on type "Query".');; + + Event::assertDispatchedTimes(CacheMissed::class, 1); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 0); + } +} From b7c6347cf46f28d46fb9bcd5806a7ac23d541bf2 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 14:53:23 +0200 Subject: [PATCH 04/23] add disabled test --- .../CacheableValidationRulesProvider.php | 11 +++-- src/GraphQL.php | 3 +- tests/Integration/ValidationCachingTest.php | 47 +++++++++++++++++++ 3 files changed, 56 insertions(+), 5 deletions(-) diff --git a/src/Execution/CacheableValidationRulesProvider.php b/src/Execution/CacheableValidationRulesProvider.php index 53e8c23ef..4472caa91 100644 --- a/src/Execution/CacheableValidationRulesProvider.php +++ b/src/Execution/CacheableValidationRulesProvider.php @@ -8,7 +8,6 @@ use GraphQL\Validator\Rules\QueryDepth; use Illuminate\Contracts\Config\Repository as ConfigRepository; use Nuwave\Lighthouse\Support\Contracts\ProvidesCacheableValidationRules; -use Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules; class CacheableValidationRulesProvider implements ProvidesCacheableValidationRules { @@ -29,8 +28,12 @@ public function cacheableValidationRules(): array public function validationRules(): ?array { - return [ - QueryComplexity::class => new QueryComplexity($this->configRepository->get('lighthouse.security.max_query_complexity', 0)), - ]; + $maxQueryComplexity = $this->configRepository->get('lighthouse.security.max_query_complexity', 0); + return $maxQueryComplexity === 0 + ? [] + : [ + QueryComplexity::class => new QueryComplexity($maxQueryComplexity), + ]; + } } diff --git a/src/GraphQL.php b/src/GraphQL.php index 77e117e81..a8be5ef18 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -248,6 +248,7 @@ public function executeOperation(OperationParams $params, GraphQLContext $contex $params->variables, null, $params->operation, + $params->queryId ); } catch (\Throwable $throwable) { return $this->toSerializableArray( @@ -421,7 +422,7 @@ protected function executeAndCacheValidationRules( /** @var CacheFactory $cacheFactory */ $cacheFactory = Container::getInstance()->make(CacheFactory::class); $store = $cacheFactory->store($cacheConfig['store']); - if ($store->get($cacheKey) !== null){ + if ($store->get($cacheKey) === true){ return []; } diff --git a/tests/Integration/ValidationCachingTest.php b/tests/Integration/ValidationCachingTest.php index 02b195f07..2b66b9773 100644 --- a/tests/Integration/ValidationCachingTest.php +++ b/tests/Integration/ValidationCachingTest.php @@ -49,6 +49,53 @@ public function testEnabled(): void Event::assertDispatchedTimes(KeyWritten::class, 1); } + public function testDisabled(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', false); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 0); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 0); + } + + public function testConfigMissing(): void + { + $config = $this->app->make(ConfigRepository::class); + $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache', null); + + Event::fake(); + + $this->graphQL(/** @lang GraphQL */ ' + { + foo + } + ')->assertExactJson([ + 'data' => [ + 'foo' => Foo::THE_ANSWER, + ], + ]); + + Event::assertDispatchedTimes(CacheMissed::class, 0); + Event::assertDispatchedTimes(CacheHit::class, 0); + Event::assertDispatchedTimes(KeyWritten::class, 0); + } + + public function testErrorsAreNotCached(): void { $config = $this->app->make(ConfigRepository::class); From ec9f70f2217a88c02f25746b19a572afaa825b80 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 16:41:45 +0200 Subject: [PATCH 05/23] Huge Request benchmark --- benchmarks/HugeRequestBench.php | 91 ++++++++++++++++++++++++++++++++ benchmarks/HugeResponseBench.php | 17 ++++-- benchmarks/QueryBench.php | 30 ++++++++--- 3 files changed, 127 insertions(+), 11 deletions(-) create mode 100644 benchmarks/HugeRequestBench.php diff --git a/benchmarks/HugeRequestBench.php b/benchmarks/HugeRequestBench.php new file mode 100644 index 000000000..27f65234a --- /dev/null +++ b/benchmarks/HugeRequestBench.php @@ -0,0 +1,91 @@ +query ??= $this->generateQuery(1); + $this->graphQL($this->query); + } + + /** + * @Warmup(1) + * @Revs(10) + * @Iterations(10) + * + * @ParamProviders({"providePerformanceTuning"}) + * @BeforeMethods("setPerformanceTuning") + */ + public function benchmark10(): void + { + if ($this->query === null) { + $this->query = $this->generateQuery(10); + } + $this->graphQL($this->query); + } + + /** + * @Warmup(1) + * @Revs(10) + * @Iterations(10) + * + * @ParamProviders({"providePerformanceTuning"}) + * @BeforeMethods("setPerformanceTuning") + */ + public function benchmark100(): void + { + if ($this->query === null) { + $this->query = $this->generateQuery(100); + } + $this->graphQL($this->query); + } +} diff --git a/benchmarks/HugeResponseBench.php b/benchmarks/HugeResponseBench.php index dc48a1301..8ac0d1b0a 100644 --- a/benchmarks/HugeResponseBench.php +++ b/benchmarks/HugeResponseBench.php @@ -50,9 +50,12 @@ public function resolve(): array } /** + * @Warmup(1) + * @Revs(10) * @Iterations(10) * - * @OutputTimeUnit("seconds", precision=3) + * @ParamProviders({"providePerformanceTuning"}) + * @BeforeMethods("setPerformanceTuning") */ public function benchmark1(): void { @@ -66,9 +69,12 @@ public function benchmark1(): void } /** - * @Iterations(10) + * @Warmup(1) + * @Revs(10) + * @Iterations(1) * - * @OutputTimeUnit("seconds", precision=3) + * @ParamProviders({"providePerformanceTuning"}) + * @BeforeMethods("setPerformanceTuning") */ public function benchmark100(): void { @@ -84,9 +90,12 @@ public function benchmark100(): void } /** + * @Warmup(1) + * @Revs(10) * @Iterations(10) * - * @OutputTimeUnit("seconds", precision=3) + * @ParamProviders({"providePerformanceTuning"}) + * @BeforeMethods("setPerformanceTuning") */ public function benchmark10k(): void { diff --git a/benchmarks/QueryBench.php b/benchmarks/QueryBench.php index 51b70b2ae..22ccb7056 100644 --- a/benchmarks/QueryBench.php +++ b/benchmarks/QueryBench.php @@ -35,15 +35,31 @@ protected function graphQLEndpointUrl(array $routeParams = []): string } /** - * Define environment setup. - * - * @param \Illuminate\Foundation\Application $app + * Set up function with the performance tuning. */ - protected function getEnvironmentSetUp($app): void + public function setPerformanceTuning(array $params): void { - parent::getEnvironmentSetUp($app); + $this->setUp(); + if ($params[0]){ + $this->app->make(ConfigRepository::class)->set('lighthouse.field_middleware', []); + } + $this->app->make(ConfigRepository::class)->set('lighthouse.query_cache.enable', $params[1]); + $this->app->make(ConfigRepository::class)->set('lighthouse.validation_cache.enable', $params[2]); + } - $config = $app->make(ConfigRepository::class); - $config->set('lighthouse.field_middleware', []); + public function providePerformanceTuning(): array + { + /** + * Indexes: + * 0: Remove all middlewares + * 1: Enable query cache + * 2: Enable validation cache + */ + return [ + 'nothing' => [false, false, false], + 'query cache' => [false, true, false], + 'validation cache' => [false, true, true], + 'everything' => [true, true, true], + ]; } } From 0d8ea1665f5ded77ad7ba6dde2aec8c703e6b368 Mon Sep 17 00:00:00 2001 From: k0ka Date: Thu, 22 Aug 2024 14:43:57 +0000 Subject: [PATCH 06/23] Apply php-cs-fixer changes --- benchmarks/HugeRequestBench.php | 18 ++++++++++++------ benchmarks/HugeResponseBench.php | 9 +++++++++ benchmarks/QueryBench.php | 6 ++---- .../CacheableValidationRulesProvider.php | 9 +++++---- src/GraphQL.php | 19 ++++++++----------- src/LighthouseServiceProvider.php | 2 +- src/Schema/AST/DocumentAST.php | 4 +--- tests/Integration/ValidationCachingTest.php | 3 +-- 8 files changed, 39 insertions(+), 31 deletions(-) diff --git a/benchmarks/HugeRequestBench.php b/benchmarks/HugeRequestBench.php index 27f65234a..a2b1e5e99 100644 --- a/benchmarks/HugeRequestBench.php +++ b/benchmarks/HugeRequestBench.php @@ -10,23 +10,20 @@ final class HugeRequestBench extends QueryBench @field(resolver: "Benchmarks\\HugeRequestBench@resolve") } GRAPHQL; + protected ?string $query = null; /** * Resolves foo. * * @skip - * - * @return string */ public function resolve(): string { - return "foo"; + return 'foo'; } - /** - * Generates query with $count fragments. - */ + /** Generates query with $count fragments. */ private function generateQuery(int $count): string { $query = '{'; @@ -45,10 +42,13 @@ private function generateQuery(int $count): string /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark1(): void @@ -59,10 +59,13 @@ public function benchmark1(): void /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark10(): void @@ -75,10 +78,13 @@ public function benchmark10(): void /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark100(): void diff --git a/benchmarks/HugeResponseBench.php b/benchmarks/HugeResponseBench.php index 8ac0d1b0a..175609654 100644 --- a/benchmarks/HugeResponseBench.php +++ b/benchmarks/HugeResponseBench.php @@ -51,10 +51,13 @@ public function resolve(): array /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark1(): void @@ -70,10 +73,13 @@ public function benchmark1(): void /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(1) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark100(): void @@ -91,10 +97,13 @@ public function benchmark100(): void /** * @Warmup(1) + * * @Revs(10) + * * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) + * * @BeforeMethods("setPerformanceTuning") */ public function benchmark10k(): void diff --git a/benchmarks/QueryBench.php b/benchmarks/QueryBench.php index 22ccb7056..d7255dd4d 100644 --- a/benchmarks/QueryBench.php +++ b/benchmarks/QueryBench.php @@ -34,13 +34,11 @@ protected function graphQLEndpointUrl(array $routeParams = []): string return $this->graphQLEndpoint; } - /** - * Set up function with the performance tuning. - */ + /** Set up function with the performance tuning. */ public function setPerformanceTuning(array $params): void { $this->setUp(); - if ($params[0]){ + if ($params[0]) { $this->app->make(ConfigRepository::class)->set('lighthouse.field_middleware', []); } $this->app->make(ConfigRepository::class)->set('lighthouse.query_cache.enable', $params[1]); diff --git a/src/Execution/CacheableValidationRulesProvider.php b/src/Execution/CacheableValidationRulesProvider.php index 4472caa91..ae4d53012 100644 --- a/src/Execution/CacheableValidationRulesProvider.php +++ b/src/Execution/CacheableValidationRulesProvider.php @@ -18,22 +18,23 @@ public function __construct( public function cacheableValidationRules(): array { $result = [ - QueryDepth::class => new QueryDepth($this->configRepository->get('lighthouse.security.max_query_depth', 0)), - DisableIntrospection::class => new DisableIntrospection($this->configRepository->get('lighthouse.security.disable_introspection', 0)), - ] + DocumentValidator::allRules(); + QueryDepth::class => new QueryDepth($this->configRepository->get('lighthouse.security.max_query_depth', 0)), + DisableIntrospection::class => new DisableIntrospection($this->configRepository->get('lighthouse.security.disable_introspection', 0)), + ] + DocumentValidator::allRules(); unset($result[QueryComplexity::class]); + return $result; } public function validationRules(): ?array { $maxQueryComplexity = $this->configRepository->get('lighthouse.security.max_query_complexity', 0); + return $maxQueryComplexity === 0 ? [] : [ QueryComplexity::class => new QueryComplexity($maxQueryComplexity), ]; - } } diff --git a/src/GraphQL.php b/src/GraphQL.php index a8be5ef18..66548653a 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -248,7 +248,7 @@ public function executeOperation(OperationParams $params, GraphQLContext $contex $params->variables, null, $params->operation, - $params->queryId + $params->queryId, ); } catch (\Throwable $throwable) { return $this->toSerializableArray( @@ -264,7 +264,7 @@ public function executeOperation(OperationParams $params, GraphQLContext $contex * * @api */ - public function parse(string $query, string &$hash = null): DocumentNode + public function parse(string $query, ?string &$hash = null): DocumentNode { $cacheConfig = $this->configRepository->get('lighthouse.query_cache'); $hash = hash('sha256', $query); @@ -385,29 +385,25 @@ protected function parseQuery(string $query): DocumentNode ]); } - /** * Execute the validation rules that are cacheable. * * @return array - * - * @throws \Exception */ protected function executeAndCacheValidationRules( SchemaType $schema, ?string $schemaHash, DocumentNode $query, - ?string $queryHash - ): array - { - if (!$this->providesValidationRules instanceof ProvidesCacheableValidationRules) { + ?string $queryHash, + ): array { + if (! $this->providesValidationRules instanceof ProvidesCacheableValidationRules) { return []; } $validationRules = $this->providesValidationRules->cacheableValidationRules(); foreach ($validationRules as $rule) { if ($rule instanceof QueryComplexity) { - throw new \InvalidArgumentException("QueryComplexity rule should not be registered in cacheableValidationRules"); + throw new \InvalidArgumentException('QueryComplexity rule should not be registered in cacheableValidationRules'); } } @@ -422,7 +418,7 @@ protected function executeAndCacheValidationRules( /** @var CacheFactory $cacheFactory */ $cacheFactory = Container::getInstance()->make(CacheFactory::class); $store = $cacheFactory->store($cacheConfig['store']); - if ($store->get($cacheKey) === true){ + if ($store->get($cacheKey) === true) { return []; } @@ -432,6 +428,7 @@ protected function executeAndCacheValidationRules( } $store->put($cacheKey, true, $cacheConfig['ttl']); + return []; } } diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index b6ece653f..28fc6acbe 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -101,7 +101,7 @@ public function provideSubscriptionResolver(FieldValue $fieldValue): \Closure } }); - //$this->app->bind(ProvidesValidationRules::class, ValidationRulesProvider::class); + // $this->app->bind(ProvidesValidationRules::class, ValidationRulesProvider::class); $this->app->bind(ProvidesValidationRules::class, CacheableValidationRulesProvider::class); $this->commands(self::COMMANDS); diff --git a/src/Schema/AST/DocumentAST.php b/src/Schema/AST/DocumentAST.php index 95b7d8eb0..c3cc7d364 100644 --- a/src/Schema/AST/DocumentAST.php +++ b/src/Schema/AST/DocumentAST.php @@ -90,9 +90,7 @@ class DocumentAST implements Arrayable /** @var array */ public array $schemaExtensions = []; - /** - * A hash of the schema. - */ + /** A hash of the schema. */ public ?string $hash = null; /** Create a new DocumentAST instance from a schema. */ diff --git a/tests/Integration/ValidationCachingTest.php b/tests/Integration/ValidationCachingTest.php index 2b66b9773..d9cd145d1 100644 --- a/tests/Integration/ValidationCachingTest.php +++ b/tests/Integration/ValidationCachingTest.php @@ -95,7 +95,6 @@ public function testConfigMissing(): void Event::assertDispatchedTimes(KeyWritten::class, 0); } - public function testErrorsAreNotCached(): void { $config = $this->app->make(ConfigRepository::class); @@ -242,7 +241,7 @@ public function testDifferentSchemasHasDifferentKeys(): void { foo } - ')->assertGraphQLErrorMessage('Cannot query field "foo" on type "Query".');; + ')->assertGraphQLErrorMessage('Cannot query field "foo" on type "Query".'); Event::assertDispatchedTimes(CacheMissed::class, 1); Event::assertDispatchedTimes(CacheHit::class, 0); From e5c3fcb3da235f59baef82c55d2d98d539f56174 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 16:49:14 +0200 Subject: [PATCH 07/23] fix phpstan errors --- benchmarks/QueryBench.php | 20 +++++++++++++------- src/Schema/AST/DocumentAST.php | 1 + 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/benchmarks/QueryBench.php b/benchmarks/QueryBench.php index d7255dd4d..e14798cc3 100644 --- a/benchmarks/QueryBench.php +++ b/benchmarks/QueryBench.php @@ -34,7 +34,11 @@ protected function graphQLEndpointUrl(array $routeParams = []): string return $this->graphQLEndpoint; } - /** Set up function with the performance tuning. */ + /** + * Set up function with the performance tuning. + * + * @param array{0: bool, 1: bool, 2: bool} $params Performance tuning parameters + */ public function setPerformanceTuning(array $params): void { $this->setUp(); @@ -45,14 +49,16 @@ public function setPerformanceTuning(array $params): void $this->app->make(ConfigRepository::class)->set('lighthouse.validation_cache.enable', $params[2]); } + /** + * Indexes: + * 0: Remove all middlewares + * 1: Enable query cache + * 2: Enable validation cache + * + * @return array + */ public function providePerformanceTuning(): array { - /** - * Indexes: - * 0: Remove all middlewares - * 1: Enable query cache - * 2: Enable validation cache - */ return [ 'nothing' => [false, false, false], 'query cache' => [false, true, false], diff --git a/src/Schema/AST/DocumentAST.php b/src/Schema/AST/DocumentAST.php index c3cc7d364..8f10a029f 100644 --- a/src/Schema/AST/DocumentAST.php +++ b/src/Schema/AST/DocumentAST.php @@ -32,6 +32,7 @@ * directives: array>, * classNameToObjectTypeName: ClassNameToObjectTypeName, * schemaExtensions: array>, + * hash: string|null, * } * * @implements \Illuminate\Contracts\Support\Arrayable From fe03ff09f6c417d7f9bad4d71e1b63ab488e0ec9 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 17:08:03 +0200 Subject: [PATCH 08/23] documentation --- docs/6/security/validation.md | 6 +++--- docs/master/performance/query-caching.md | 8 ++++++++ 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/6/security/validation.md b/docs/6/security/validation.md index c57bac35e..b391e3008 100644 --- a/docs/6/security/validation.md +++ b/docs/6/security/validation.md @@ -314,12 +314,12 @@ By default, Lighthouse enables all default query validation rules from `webonyx/ This covers fundamental checks, e.g. queried fields match the schema, variables have values of the correct type. If you want to add custom rules or change which ones are used, you can bind a custom implementation -of the interface `\Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules` through a service provider. +of the interface `\Nuwave\Lighthouse\Support\Contracts\ProvidesCacheableValidationRules` through a service provider. ```php use Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules; -final class MyCustomRulesProvider implements ProvidesValidationRules {} +final class MyCustomRulesProvider implements ProvidesCacheableValidationRules {} -$this->app->bind(ProvidesValidationRules::class, MyCustomRulesProvider::class); +$this->app->bind(ProvidesCacheableValidationRules::class, MyCustomRulesProvider::class); ``` diff --git a/docs/master/performance/query-caching.md b/docs/master/performance/query-caching.md index 2af2f1715..313822b3f 100644 --- a/docs/master/performance/query-caching.md +++ b/docs/master/performance/query-caching.md @@ -15,6 +15,14 @@ Lighthouse supports Automatic Persisted Queries (APQ), compatible with the APQ is enabled by default, but depends on query caching being enabled. +## Query validation caching + +Lighthouse can cache the result of the query validation process as well. It only caches queries without errors. +`QueryComplexity` validation can not be cached as it is dependent on the query, so it is always executed. + +Query validation caching is disabled by default. You can enable it by setting `validation_cache.enable` to `true` in the +configuration in `config/lighthouse.php`. + ## Testing caveats If you are mocking Laravel cache classes like `\Illuminate\Support\Facades\Cache` or `\Illuminate\Cache\Repository` and asserting expectations in your unit tests, it might be best to disable the query cache in your `phpunit.xml`: From b8451f4b9323a2115af3377c9fc79867e326d89c Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 17:25:36 +0200 Subject: [PATCH 09/23] changelog, fix benchmark a little --- CHANGELOG.md | 4 ++++ benchmarks/HugeResponseBench.php | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9c75b3358..58d54df08 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,10 @@ You can find and compare releases at the [GitHub release page](https://github.co ## Unreleased +### Added + +- Cache query validation results https://github.com/nuwave/lighthouse/pull/2603 + ## v6.43.0 ### Added diff --git a/benchmarks/HugeResponseBench.php b/benchmarks/HugeResponseBench.php index 175609654..d0e0318bd 100644 --- a/benchmarks/HugeResponseBench.php +++ b/benchmarks/HugeResponseBench.php @@ -76,7 +76,7 @@ public function benchmark1(): void * * @Revs(10) * - * @Iterations(1) + * @Iterations(10) * * @ParamProviders({"providePerformanceTuning"}) * From b3b42b009dbba11792b86a7b029c520c6c29b6ed Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 17:35:38 +0200 Subject: [PATCH 10/23] add consistent docs to master and 6th version --- docs/6/performance/query-caching.md | 8 ++++++++ docs/master/security/validation.md | 6 +++--- 2 files changed, 11 insertions(+), 3 deletions(-) diff --git a/docs/6/performance/query-caching.md b/docs/6/performance/query-caching.md index 2af2f1715..313822b3f 100644 --- a/docs/6/performance/query-caching.md +++ b/docs/6/performance/query-caching.md @@ -15,6 +15,14 @@ Lighthouse supports Automatic Persisted Queries (APQ), compatible with the APQ is enabled by default, but depends on query caching being enabled. +## Query validation caching + +Lighthouse can cache the result of the query validation process as well. It only caches queries without errors. +`QueryComplexity` validation can not be cached as it is dependent on the query, so it is always executed. + +Query validation caching is disabled by default. You can enable it by setting `validation_cache.enable` to `true` in the +configuration in `config/lighthouse.php`. + ## Testing caveats If you are mocking Laravel cache classes like `\Illuminate\Support\Facades\Cache` or `\Illuminate\Cache\Repository` and asserting expectations in your unit tests, it might be best to disable the query cache in your `phpunit.xml`: diff --git a/docs/master/security/validation.md b/docs/master/security/validation.md index c57bac35e..b391e3008 100644 --- a/docs/master/security/validation.md +++ b/docs/master/security/validation.md @@ -314,12 +314,12 @@ By default, Lighthouse enables all default query validation rules from `webonyx/ This covers fundamental checks, e.g. queried fields match the schema, variables have values of the correct type. If you want to add custom rules or change which ones are used, you can bind a custom implementation -of the interface `\Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules` through a service provider. +of the interface `\Nuwave\Lighthouse\Support\Contracts\ProvidesCacheableValidationRules` through a service provider. ```php use Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules; -final class MyCustomRulesProvider implements ProvidesValidationRules {} +final class MyCustomRulesProvider implements ProvidesCacheableValidationRules {} -$this->app->bind(ProvidesValidationRules::class, MyCustomRulesProvider::class); +$this->app->bind(ProvidesCacheableValidationRules::class, MyCustomRulesProvider::class); ``` From c9a0453385263f7b374e2344ce4f0c610576e254 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 22 Aug 2024 17:42:49 +0200 Subject: [PATCH 11/23] fix tests for enabled validation cache --- tests/Integration/QueryCachingTest.php | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/Integration/QueryCachingTest.php b/tests/Integration/QueryCachingTest.php index 72e692284..0d497fa6a 100644 --- a/tests/Integration/QueryCachingTest.php +++ b/tests/Integration/QueryCachingTest.php @@ -14,6 +14,7 @@ public function testEnabled(): void { $config = $this->app->make(ConfigRepository::class); $config->set('lighthouse.query_cache.enable', true); + $config->set('lighthouse.validation_cache.enable', false); Event::fake(); @@ -51,6 +52,7 @@ public function testDifferentQueriesHasDifferentKeys(): void { $config = $this->app->make(ConfigRepository::class); $config->set('lighthouse.query_cache.enable', true); + $config->set('lighthouse.validation_cache.enable', false); Event::fake(); @@ -84,6 +86,7 @@ public function testDisabled(): void { $config = $this->app->make(ConfigRepository::class); $config->set('lighthouse.query_cache.enable', false); + $config->set('lighthouse.validation_cache.enable', false); Event::fake(); From ee4f3e2ea89cb3456a6903c89cddd8498d278150 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Mon, 26 Aug 2024 20:20:22 +0200 Subject: [PATCH 12/23] Update CHANGELOG.md --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a0472f184..843ac8052 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,7 +12,7 @@ You can find and compare releases at the [GitHub release page](https://github.co ### Added - Cache query validation results https://github.com/nuwave/lighthouse/pull/2603 -- + ## v6.43.1 ### Changed From 6f69bde71eddb3d9b77d2a3c880691f83184f31c Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 19 Sep 2024 19:33:44 +0200 Subject: [PATCH 13/23] Apply suggestions from code review Co-authored-by: Benedikt Franke --- docs/master/performance/query-caching.md | 7 ++++--- src/LighthouseServiceProvider.php | 1 - src/Support/Contracts/ProvidesCacheableValidationRules.php | 4 ++-- tests/Integration/ValidationCachingTest.php | 2 +- tests/Unit/Schema/AST/DocumentASTTest.php | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/docs/master/performance/query-caching.md b/docs/master/performance/query-caching.md index 313822b3f..08bd9f400 100644 --- a/docs/master/performance/query-caching.md +++ b/docs/master/performance/query-caching.md @@ -17,11 +17,12 @@ APQ is enabled by default, but depends on query caching being enabled. ## Query validation caching -Lighthouse can cache the result of the query validation process as well. It only caches queries without errors. +Lighthouse can cache the result of the query validation process as well. +It only caches queries without errors. `QueryComplexity` validation can not be cached as it is dependent on the query, so it is always executed. -Query validation caching is disabled by default. You can enable it by setting `validation_cache.enable` to `true` in the -configuration in `config/lighthouse.php`. +Query validation caching is disabled by default. +You can enable it by setting `validation_cache.enable` to `true` in `config/lighthouse.php`. ## Testing caveats diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index 28fc6acbe..580cf34ba 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -101,7 +101,6 @@ public function provideSubscriptionResolver(FieldValue $fieldValue): \Closure } }); - // $this->app->bind(ProvidesValidationRules::class, ValidationRulesProvider::class); $this->app->bind(ProvidesValidationRules::class, CacheableValidationRulesProvider::class); $this->commands(self::COMMANDS); diff --git a/src/Support/Contracts/ProvidesCacheableValidationRules.php b/src/Support/Contracts/ProvidesCacheableValidationRules.php index 55fe4b415..f0f333b53 100644 --- a/src/Support/Contracts/ProvidesCacheableValidationRules.php +++ b/src/Support/Contracts/ProvidesCacheableValidationRules.php @@ -7,7 +7,7 @@ interface ProvidesCacheableValidationRules extends ProvidesValidationRules /** * A set of rules for the first query validation step. * - * These rules would be executed first and their result would be cached. + * These rules are executed first and their result is cached. * * @return array */ @@ -16,7 +16,7 @@ public function cacheableValidationRules(): array; /** * A set of rules for the second query validation step. * - * These rules would always be executed and not cached. + * These rules are always be executed and not cached. * * Returning `null` enables all available rules. * Empty array skips query validation entirely. diff --git a/tests/Integration/ValidationCachingTest.php b/tests/Integration/ValidationCachingTest.php index d9cd145d1..e347a784a 100644 --- a/tests/Integration/ValidationCachingTest.php +++ b/tests/Integration/ValidationCachingTest.php @@ -9,7 +9,7 @@ use Tests\TestCase; use Tests\Utils\Queries\Foo; -class ValidationCachingTest extends TestCase +final class ValidationCachingTest extends TestCase { public function testEnabled(): void { diff --git a/tests/Unit/Schema/AST/DocumentASTTest.php b/tests/Unit/Schema/AST/DocumentASTTest.php index de7477dc9..c965847a5 100644 --- a/tests/Unit/Schema/AST/DocumentASTTest.php +++ b/tests/Unit/Schema/AST/DocumentASTTest.php @@ -114,6 +114,6 @@ public function testBeSerialized(): void $this->assertInstanceOf(SchemaExtensionNode::class, $schemaExtension); $this->assertInstanceOf(DirectiveNode::class, $schemaExtension->directives[0]); - $this->assertEquals($documentAST->hash, $reserialized->hash); + $this->assertSame($documentAST->hash, $reserialized->hash); } } From 2f5ddcdc46824d483e6965bd4de5399de81631c2 Mon Sep 17 00:00:00 2001 From: k0ka Date: Thu, 19 Sep 2024 17:34:34 +0000 Subject: [PATCH 14/23] Apply php-cs-fixer changes --- src/LighthouseServiceProvider.php | 1 - 1 file changed, 1 deletion(-) diff --git a/src/LighthouseServiceProvider.php b/src/LighthouseServiceProvider.php index 580cf34ba..e4b2da68b 100644 --- a/src/LighthouseServiceProvider.php +++ b/src/LighthouseServiceProvider.php @@ -32,7 +32,6 @@ use Nuwave\Lighthouse\Execution\ContextSerializer; use Nuwave\Lighthouse\Execution\ErrorPool; use Nuwave\Lighthouse\Execution\SingleResponse; -use Nuwave\Lighthouse\Execution\ValidationRulesProvider; use Nuwave\Lighthouse\Http\Responses\ResponseStream; use Nuwave\Lighthouse\Schema\AST\ASTBuilder; use Nuwave\Lighthouse\Schema\DirectiveLocator; From 9ba4441fa041fffe93d5a86233dc8615d060f486 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 19 Sep 2024 20:05:32 +0200 Subject: [PATCH 15/23] use Event::fake object assertions instead of static functions --- tests/Integration/QueryCachingTest.php | 30 ++++---- tests/Integration/ValidationCachingTest.php | 81 +++++++++++---------- 2 files changed, 57 insertions(+), 54 deletions(-) diff --git a/tests/Integration/QueryCachingTest.php b/tests/Integration/QueryCachingTest.php index 0d497fa6a..dec95eaef 100644 --- a/tests/Integration/QueryCachingTest.php +++ b/tests/Integration/QueryCachingTest.php @@ -16,7 +16,7 @@ public function testEnabled(): void $config->set('lighthouse.query_cache.enable', true); $config->set('lighthouse.validation_cache.enable', false); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -28,9 +28,9 @@ public function testEnabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 1); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 1); // second request should be hit $this->graphQL(/** @lang GraphQL */ ' @@ -43,9 +43,9 @@ public function testEnabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 1); - Event::assertDispatchedTimes(KeyWritten::class, 1); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 1); + $event->assertDispatchedTimes(KeyWritten::class, 1); } public function testDifferentQueriesHasDifferentKeys(): void @@ -54,7 +54,7 @@ public function testDifferentQueriesHasDifferentKeys(): void $config->set('lighthouse.query_cache.enable', true); $config->set('lighthouse.validation_cache.enable', false); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -77,9 +77,9 @@ public function testDifferentQueriesHasDifferentKeys(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 2); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 2); + $event->assertDispatchedTimes(CacheMissed::class, 2); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 2); } public function testDisabled(): void @@ -88,7 +88,7 @@ public function testDisabled(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', false); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -100,8 +100,8 @@ public function testDisabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 0); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 0); + $event->assertDispatchedTimes(CacheMissed::class, 0); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 0); } } diff --git a/tests/Integration/ValidationCachingTest.php b/tests/Integration/ValidationCachingTest.php index e347a784a..f5dadd104 100644 --- a/tests/Integration/ValidationCachingTest.php +++ b/tests/Integration/ValidationCachingTest.php @@ -5,6 +5,8 @@ use Illuminate\Cache\Events\KeyWritten; use Illuminate\Contracts\Cache\Factory as CacheFactory; use Illuminate\Contracts\Config\Repository as ConfigRepository; +use Illuminate\Contracts\Events\Dispatcher as EventsDispatcher; +use Illuminate\Support\Facades\Cache; use Illuminate\Support\Facades\Event; use Tests\TestCase; use Tests\Utils\Queries\Foo; @@ -17,7 +19,7 @@ public function testEnabled(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -29,9 +31,9 @@ public function testEnabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 1); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 1); // second request should be hit $this->graphQL(/** @lang GraphQL */ ' @@ -44,9 +46,9 @@ public function testEnabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 1); - Event::assertDispatchedTimes(KeyWritten::class, 1); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 1); + $event->assertDispatchedTimes(KeyWritten::class, 1); } public function testDisabled(): void @@ -55,7 +57,7 @@ public function testDisabled(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', false); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -67,9 +69,9 @@ public function testDisabled(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 0); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 0); + $event->assertDispatchedTimes(CacheMissed::class, 0); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 0); } public function testConfigMissing(): void @@ -78,7 +80,7 @@ public function testConfigMissing(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache', null); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -90,9 +92,9 @@ public function testConfigMissing(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 0); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 0); + $event->assertDispatchedTimes(CacheMissed::class, 0); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 0); } public function testErrorsAreNotCached(): void @@ -101,7 +103,7 @@ public function testErrorsAreNotCached(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -109,9 +111,9 @@ public function testErrorsAreNotCached(): void } ')->assertGraphQLErrorMessage('Cannot query field "bar" on type "Query".'); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 0); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 0); } public function testDifferentQueriesHasDifferentKeys(): void @@ -120,7 +122,7 @@ public function testDifferentQueriesHasDifferentKeys(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -143,9 +145,9 @@ public function testDifferentQueriesHasDifferentKeys(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 2); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 2); + $event->assertDispatchedTimes(CacheMissed::class, 2); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 2); } public function testSameSchemaAndSameQueryHaveSameKeys(): void @@ -173,16 +175,15 @@ public function testSameSchemaAndSameQueryHaveSameKeys(): void // refresh container, but keep the same cache $cacheFactory = $this->app->make(CacheFactory::class); $this->refreshApplication(); - $this->app->instance(CacheFactory::class, $cacheFactory); + $this->setUp(); - $this->setUpTestSchema(); + $this->app->instance(EventsDispatcher::class, $event); + $this->app->instance(CacheFactory::class, $cacheFactory); $config = $this->app->make(ConfigRepository::class); $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); - $this->graphQL(/** @lang GraphQL */ ' { foo @@ -204,7 +205,7 @@ public function testDifferentSchemasHasDifferentKeys(): void $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); + $event = Event::fake(); $this->graphQL(/** @lang GraphQL */ ' { @@ -216,12 +217,9 @@ public function testDifferentSchemasHasDifferentKeys(): void ], ]); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 1); - - // refresh container - $this->refreshApplication(); + $event->assertDispatchedTimes(CacheMissed::class, 1); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 1); $this->schema = /** @lang GraphQL */ <<<'GRAPHQL' type Query { @@ -229,22 +227,27 @@ public function testDifferentSchemasHasDifferentKeys(): void } GRAPHQL; + // refresh container, but keep the same cache + $cacheFactory = $this->app->make(CacheFactory::class); + + $this->refreshApplication(); $this->setUp(); + $this->app->instance(EventsDispatcher::class, $event); + $this->app->instance(CacheFactory::class, $cacheFactory); + $config = $this->app->make(ConfigRepository::class); $config->set('lighthouse.query_cache.enable', false); $config->set('lighthouse.validation_cache.enable', true); - Event::fake(); - $this->graphQL(/** @lang GraphQL */ ' { foo } ')->assertGraphQLErrorMessage('Cannot query field "foo" on type "Query".'); - Event::assertDispatchedTimes(CacheMissed::class, 1); - Event::assertDispatchedTimes(CacheHit::class, 0); - Event::assertDispatchedTimes(KeyWritten::class, 0); + $event->assertDispatchedTimes(CacheMissed::class, 2); + $event->assertDispatchedTimes(CacheHit::class, 0); + $event->assertDispatchedTimes(KeyWritten::class, 1); } } From b7ee225d35f32ec866654f939457ed68158c8812 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Thu, 19 Sep 2024 20:08:50 +0200 Subject: [PATCH 16/23] rename `executeAndCacheValidationRules` to `validateCacheableRules` --- src/GraphQL.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index 66548653a..aa61f6695 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -134,7 +134,7 @@ public function executeParsedQueryRaw( new StartExecution($schema, $query, $variables, $operationName, $context), ); - $errors = $this->executeAndCacheValidationRules($schema, $this->schemaBuilder->schemaHash(), $query, $queryHash); + $errors = $this->validateCacheableRules($schema, $this->schemaBuilder->schemaHash(), $query, $queryHash); if ($errors !== []) { return new ExecutionResult(null, $errors); } @@ -386,11 +386,11 @@ protected function parseQuery(string $query): DocumentNode } /** - * Execute the validation rules that are cacheable. + * Validate rules that are cacheable. Either by using the cache or by running them. * * @return array */ - protected function executeAndCacheValidationRules( + protected function validateCacheableRules( SchemaType $schema, ?string $schemaHash, DocumentNode $query, From 289f80ad3d788210e57734246d69d0ecb89a99c0 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Fri, 20 Sep 2024 08:14:24 +0200 Subject: [PATCH 17/23] make `DocumentAST::$hash` not nullable --- src/GraphQL.php | 4 ++-- src/Schema/AST/DocumentAST.php | 2 +- src/Schema/SchemaBuilder.php | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index aa61f6695..0949ccdbf 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -392,7 +392,7 @@ protected function parseQuery(string $query): DocumentNode */ protected function validateCacheableRules( SchemaType $schema, - ?string $schemaHash, + string $schemaHash, DocumentNode $query, ?string $queryHash, ): array { @@ -409,7 +409,7 @@ protected function validateCacheableRules( $cacheConfig = $this->configRepository->get('lighthouse.validation_cache'); - if ($schemaHash === null || $queryHash === null || ! ($cacheConfig['enable'] ?? false)) { + if ($queryHash === null || ! ($cacheConfig['enable'] ?? false)) { return DocumentValidator::validate($schema, $query, $validationRules); } diff --git a/src/Schema/AST/DocumentAST.php b/src/Schema/AST/DocumentAST.php index 8f10a029f..a840a15fe 100644 --- a/src/Schema/AST/DocumentAST.php +++ b/src/Schema/AST/DocumentAST.php @@ -92,7 +92,7 @@ class DocumentAST implements Arrayable public array $schemaExtensions = []; /** A hash of the schema. */ - public ?string $hash = null; + public string $hash; /** Create a new DocumentAST instance from a schema. */ public static function fromSource(string $schema): self diff --git a/src/Schema/SchemaBuilder.php b/src/Schema/SchemaBuilder.php index 03e81bb93..b6df5884f 100644 --- a/src/Schema/SchemaBuilder.php +++ b/src/Schema/SchemaBuilder.php @@ -28,7 +28,7 @@ public function schema(): Schema ); } - public function schemaHash(): ?string + public function schemaHash(): string { return $this->astBuilder->documentAST()->hash; } From 9b50d119734230751c603abe6f2a723794b7aa01 Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Fri, 20 Sep 2024 08:21:39 +0200 Subject: [PATCH 18/23] assert actual hash value in `DocumentASTTest::testParsesSimpleSchema` --- tests/Unit/Schema/AST/DocumentASTTest.php | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/tests/Unit/Schema/AST/DocumentASTTest.php b/tests/Unit/Schema/AST/DocumentASTTest.php index c965847a5..f2c36174e 100644 --- a/tests/Unit/Schema/AST/DocumentASTTest.php +++ b/tests/Unit/Schema/AST/DocumentASTTest.php @@ -18,18 +18,21 @@ final class DocumentASTTest extends TestCase { public function testParsesSimpleSchema(): void { - $documentAST = DocumentAST::fromSource(/** @lang GraphQL */ ' + $schema = /** @lang GraphQL */ ' type Query { foo: Int } - '); + '; + // calculated as hash('sha256', $schema) + $schemaHash = "99fd7bd3f58a98d8932c1f5d1da718707f6f471e93d96e0bc913436445a947ac"; + $documentAST = DocumentAST::fromSource($schema); $this->assertInstanceOf( ObjectTypeDefinitionNode::class, $documentAST->types[RootType::QUERY], ); - $this->assertNotNull($documentAST->hash); + $this->assertSame($schemaHash, $documentAST->hash); } public function testThrowsOnInvalidSchema(): void From bd8185a3bc9ad7db182626af15082849d3f423fc Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Fri, 20 Sep 2024 08:24:05 +0200 Subject: [PATCH 19/23] Update src/Support/Contracts/ProvidesCacheableValidationRules.php Co-authored-by: Benedikt Franke --- src/Support/Contracts/ProvidesCacheableValidationRules.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Support/Contracts/ProvidesCacheableValidationRules.php b/src/Support/Contracts/ProvidesCacheableValidationRules.php index f0f333b53..5e9337dec 100644 --- a/src/Support/Contracts/ProvidesCacheableValidationRules.php +++ b/src/Support/Contracts/ProvidesCacheableValidationRules.php @@ -16,7 +16,7 @@ public function cacheableValidationRules(): array; /** * A set of rules for the second query validation step. * - * These rules are always be executed and not cached. + * These rules are always executed and not cached. * * Returning `null` enables all available rules. * Empty array skips query validation entirely. From 76475717fa563a75362796f3396350b21b3c4d42 Mon Sep 17 00:00:00 2001 From: k0ka Date: Fri, 20 Sep 2024 06:24:30 +0000 Subject: [PATCH 20/23] Apply php-cs-fixer changes --- tests/Unit/Schema/AST/DocumentASTTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Unit/Schema/AST/DocumentASTTest.php b/tests/Unit/Schema/AST/DocumentASTTest.php index f2c36174e..9bf6788cf 100644 --- a/tests/Unit/Schema/AST/DocumentASTTest.php +++ b/tests/Unit/Schema/AST/DocumentASTTest.php @@ -24,7 +24,7 @@ public function testParsesSimpleSchema(): void } '; // calculated as hash('sha256', $schema) - $schemaHash = "99fd7bd3f58a98d8932c1f5d1da718707f6f471e93d96e0bc913436445a947ac"; + $schemaHash = '99fd7bd3f58a98d8932c1f5d1da718707f6f471e93d96e0bc913436445a947ac'; $documentAST = DocumentAST::fromSource($schema); $this->assertInstanceOf( From c7857a79bd781124d11270828a10372f2b756f9b Mon Sep 17 00:00:00 2001 From: Konstantin Babushkin Date: Fri, 20 Sep 2024 08:28:01 +0200 Subject: [PATCH 21/23] fix phpstan --- src/Schema/AST/DocumentAST.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Schema/AST/DocumentAST.php b/src/Schema/AST/DocumentAST.php index a840a15fe..62653f1fb 100644 --- a/src/Schema/AST/DocumentAST.php +++ b/src/Schema/AST/DocumentAST.php @@ -32,7 +32,7 @@ * directives: array>, * classNameToObjectTypeName: ClassNameToObjectTypeName, * schemaExtensions: array>, - * hash: string|null, + * hash: string, * } * * @implements \Illuminate\Contracts\Support\Arrayable From 7df6e218d6a3ca9b3626b59cf2d4064b52c2ba1c Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 20 Sep 2024 09:55:32 +0200 Subject: [PATCH 22/23] clean up --- benchmarks/HugeRequestBench.php | 9 ++-- benchmarks/QueryBench.php | 18 +++++--- src/GraphQL.php | 43 +++++++++++-------- .../ProvidesCacheableValidationRules.php | 14 +++--- .../Contracts/ProvidesValidationRules.php | 6 +-- 5 files changed, 53 insertions(+), 37 deletions(-) diff --git a/benchmarks/HugeRequestBench.php b/benchmarks/HugeRequestBench.php index a2b1e5e99..dbbb4952d 100644 --- a/benchmarks/HugeRequestBench.php +++ b/benchmarks/HugeRequestBench.php @@ -30,6 +30,7 @@ private function generateQuery(int $count): string for ($i = 0; $i < $count; ++$i) { $query .= '...foo' . $i . PHP_EOL; } + $query .= '}' . PHP_EOL; for ($i = 0; $i < $count; ++$i) { $query .= 'fragment foo' . $i . ' on Query {' . PHP_EOL; @@ -70,9 +71,7 @@ public function benchmark1(): void */ public function benchmark10(): void { - if ($this->query === null) { - $this->query = $this->generateQuery(10); - } + $this->query ??= $this->generateQuery(10); $this->graphQL($this->query); } @@ -89,9 +88,7 @@ public function benchmark10(): void */ public function benchmark100(): void { - if ($this->query === null) { - $this->query = $this->generateQuery(100); - } + $this->query ??= $this->generateQuery(100); $this->graphQL($this->query); } } diff --git a/benchmarks/QueryBench.php b/benchmarks/QueryBench.php index e14798cc3..a66f38a0e 100644 --- a/benchmarks/QueryBench.php +++ b/benchmarks/QueryBench.php @@ -20,7 +20,10 @@ public function setUp(): void { parent::setUp(); - $routeName = config('lighthouse.route.name'); + $configRepository = $this->app->make(ConfigRepository::class); + assert($configRepository instanceof ConfigRepository); + + $routeName = $configRepository->get('lighthouse.route.name'); $this->graphQLEndpoint = route($routeName); } @@ -42,11 +45,16 @@ protected function graphQLEndpointUrl(array $routeParams = []): string public function setPerformanceTuning(array $params): void { $this->setUp(); + + $configRepository = $this->app->make(ConfigRepository::class); + assert($configRepository instanceof ConfigRepository); + if ($params[0]) { - $this->app->make(ConfigRepository::class)->set('lighthouse.field_middleware', []); + $configRepository->set('lighthouse.field_middleware', []); } - $this->app->make(ConfigRepository::class)->set('lighthouse.query_cache.enable', $params[1]); - $this->app->make(ConfigRepository::class)->set('lighthouse.validation_cache.enable', $params[2]); + + $configRepository->set('lighthouse.query_cache.enable', $params[1]); + $configRepository->set('lighthouse.validation_cache.enable', $params[2]); } /** @@ -62,7 +70,7 @@ public function providePerformanceTuning(): array return [ 'nothing' => [false, false, false], 'query cache' => [false, true, false], - 'validation cache' => [false, true, true], + 'query + validation cache' => [false, true, true], 'everything' => [true, true, true], ]; } diff --git a/src/GraphQL.php b/src/GraphQL.php index 0949ccdbf..d5020973f 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -12,7 +12,7 @@ use GraphQL\Server\Helper as GraphQLHelper; use GraphQL\Server\OperationParams; use GraphQL\Server\RequestError; -use GraphQL\Type\Schema as SchemaType; +use GraphQL\Type\Schema; use GraphQL\Validator\DocumentValidator; use GraphQL\Validator\Rules\QueryComplexity; use Illuminate\Container\Container; @@ -28,11 +28,11 @@ use Nuwave\Lighthouse\Events\StartExecution; use Nuwave\Lighthouse\Events\StartOperationOrOperations; use Nuwave\Lighthouse\Execution\BatchLoader\BatchLoaderRegistry; +use Nuwave\Lighthouse\Execution\CacheableValidationRulesProvider; use Nuwave\Lighthouse\Execution\ErrorPool; use Nuwave\Lighthouse\Schema\SchemaBuilder; use Nuwave\Lighthouse\Schema\Values\FieldValue; use Nuwave\Lighthouse\Support\Contracts\GraphQLContext; -use Nuwave\Lighthouse\Support\Contracts\ProvidesCacheableValidationRules; use Nuwave\Lighthouse\Support\Contracts\ProvidesValidationRules; use Nuwave\Lighthouse\Support\Utils as LighthouseUtils; @@ -46,6 +46,8 @@ */ class GraphQL { + protected const CACHEABLE_RULES_ERROR_FREE_RESULT = true; + /** * Lazily initialized. * @@ -134,9 +136,13 @@ public function executeParsedQueryRaw( new StartExecution($schema, $query, $variables, $operationName, $context), ); - $errors = $this->validateCacheableRules($schema, $this->schemaBuilder->schemaHash(), $query, $queryHash); - if ($errors !== []) { - return new ExecutionResult(null, $errors); + if ($this->providesValidationRules instanceof CacheableValidationRulesProvider) { + $validationRules = $this->providesValidationRules->cacheableValidationRules(); + + $errors = $this->validateCacheableRules($validationRules, $schema, $this->schemaBuilder->schemaHash(), $query, $queryHash); + if ($errors !== []) { + return new ExecutionResult(null, $errors); + } } $result = GraphQLBase::executeQuery( @@ -386,39 +392,42 @@ protected function parseQuery(string $query): DocumentNode } /** - * Validate rules that are cacheable. Either by using the cache or by running them. + * Provides a result for cacheable validation rules by running them or retrieving it from the cache. + * + * @param array $validationRules * * @return array */ protected function validateCacheableRules( - SchemaType $schema, + array $validationRules, + Schema $schema, string $schemaHash, DocumentNode $query, ?string $queryHash, ): array { - if (! $this->providesValidationRules instanceof ProvidesCacheableValidationRules) { - return []; - } - - $validationRules = $this->providesValidationRules->cacheableValidationRules(); foreach ($validationRules as $rule) { if ($rule instanceof QueryComplexity) { - throw new \InvalidArgumentException('QueryComplexity rule should not be registered in cacheableValidationRules'); + throw new \InvalidArgumentException('The QueryComplexity rule must not be registered in cacheableValidationRules, as it depends on variables.'); } } + if ($queryHash === null) { + return DocumentValidator::validate($schema, $query, $validationRules); + } + $cacheConfig = $this->configRepository->get('lighthouse.validation_cache'); - if ($queryHash === null || ! ($cacheConfig['enable'] ?? false)) { + if (! isset($cacheConfig['enable']) || ! $cacheConfig['enable']) { return DocumentValidator::validate($schema, $query, $validationRules); } $cacheKey = "lighthouse:validation:{$schemaHash}:{$queryHash}"; - /** @var CacheFactory $cacheFactory */ $cacheFactory = Container::getInstance()->make(CacheFactory::class); + assert($cacheFactory instanceof CacheFactory); + $store = $cacheFactory->store($cacheConfig['store']); - if ($store->get($cacheKey) === true) { + if ($store->get($cacheKey) === self::CACHEABLE_RULES_ERROR_FREE_RESULT) { return []; } @@ -427,7 +436,7 @@ protected function validateCacheableRules( return $result; } - $store->put($cacheKey, true, $cacheConfig['ttl']); + $store->put($cacheKey, self::CACHEABLE_RULES_ERROR_FREE_RESULT, $cacheConfig['ttl']); return []; } diff --git a/src/Support/Contracts/ProvidesCacheableValidationRules.php b/src/Support/Contracts/ProvidesCacheableValidationRules.php index 5e9337dec..d0224e631 100644 --- a/src/Support/Contracts/ProvidesCacheableValidationRules.php +++ b/src/Support/Contracts/ProvidesCacheableValidationRules.php @@ -2,24 +2,26 @@ namespace Nuwave\Lighthouse\Support\Contracts; +/** Allows splitting validation into a cacheable first step and a non-cacheable second step. */ interface ProvidesCacheableValidationRules extends ProvidesValidationRules { /** - * A set of rules for the first query validation step. + * Rules where the result depends only on the schema and the query string. * - * These rules are executed first and their result is cached. + * These rules are executed before non-cacheable rules and may not run + * at all when their result is already cached. * * @return array */ public function cacheableValidationRules(): array; /** - * A set of rules for the second query validation step. + * Rules where the result also depends on variables or other data. * - * These rules are always executed and not cached. + * These rules are always executed and their result is never cached. * - * Returning `null` enables all available rules. - * Empty array skips query validation entirely. + * Returning `null` enables all available rules, + * returning `[]` skips query validation entirely. * * @return array|null */ diff --git a/src/Support/Contracts/ProvidesValidationRules.php b/src/Support/Contracts/ProvidesValidationRules.php index 0905bc7a0..100fe13f5 100644 --- a/src/Support/Contracts/ProvidesValidationRules.php +++ b/src/Support/Contracts/ProvidesValidationRules.php @@ -5,10 +5,10 @@ interface ProvidesValidationRules { /** - * A set of rules for query validation step. + * Rules to use for query validation. * - * Returning `null` enables all available rules. - * Empty array skips query validation entirely. + * Returning `null` enables all available rules, + * returning `[]` skips query validation entirely. * * @return array|null */ From baaa1a2afee415fb977a928ed1faa34ff7ecb33d Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Fri, 20 Sep 2024 11:51:45 +0200 Subject: [PATCH 23/23] explain why errors are not cached --- src/GraphQL.php | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/GraphQL.php b/src/GraphQL.php index d5020973f..36e71f3e0 100644 --- a/src/GraphQL.php +++ b/src/GraphQL.php @@ -46,8 +46,6 @@ */ class GraphQL { - protected const CACHEABLE_RULES_ERROR_FREE_RESULT = true; - /** * Lazily initialized. * @@ -396,7 +394,7 @@ protected function parseQuery(string $query): DocumentNode * * @param array $validationRules * - * @return array + * @return array<\GraphQL\Error\Error> */ protected function validateCacheableRules( array $validationRules, @@ -427,17 +425,22 @@ protected function validateCacheableRules( assert($cacheFactory instanceof CacheFactory); $store = $cacheFactory->store($cacheConfig['store']); - if ($store->get($cacheKey) === self::CACHEABLE_RULES_ERROR_FREE_RESULT) { - return []; + $cachedResult = $store->get($cacheKey); + if ($cachedResult !== null) { + return $cachedResult; } $result = DocumentValidator::validate($schema, $query, $validationRules); + + // If there are any errors, we return them without caching them. + // As of webonyx/graphql-php 15.14.0, GraphQL\Error\Error is not serializable. + // We would have to figure out how to serialize them properly to cache them. if ($result !== []) { return $result; } - $store->put($cacheKey, self::CACHEABLE_RULES_ERROR_FREE_RESULT, $cacheConfig['ttl']); + $store->put($cacheKey, $result, $cacheConfig['ttl']); - return []; + return $result; } }