From 8cb446d3a329c94ec87331049b13a17614375c9f Mon Sep 17 00:00:00 2001 From: Jacob Thomason Date: Tue, 3 Nov 2026 13:28:14 -0500 Subject: [PATCH] chore: require webonyx/graphql-php 15.38.3 and drop the QueryComplexity override webonyx 15.38.3 (webonyx/graphql-php#1969) reports variable coercion errors from its QueryComplexity rule through the validation context, one located error per variable, instead of throwing a single joined error. That is what #839's subclass did, so on 15.38.3 its catch never runs, and its test asserting no locations fails on current webonyx. limitQueryComplexity() registers webonyx's rule again. The unreleased subclass and its unit test are removed; the integration test keeps asserting a located error per invalid variable. --- composer.json | 2 +- src/Http/Psr15GraphQLMiddlewareBuilder.php | 2 +- src/Validator/Rules/QueryComplexity.php | 81 --------- tests/Integration/QueryComplexityTest.php | 5 +- tests/Validator/Rules/QueryComplexityTest.php | 162 ------------------ website/docs/CHANGELOG.md | 4 +- website/docs/operation-complexity.md | 6 - 7 files changed, 6 insertions(+), 256 deletions(-) delete mode 100644 src/Validator/Rules/QueryComplexity.php delete mode 100644 tests/Validator/Rules/QueryComplexityTest.php diff --git a/composer.json b/composer.json index b543066f51..a2c7677464 100644 --- a/composer.json +++ b/composer.json @@ -23,7 +23,7 @@ "psr/simple-cache": "^1.0.1 || ^2 || ^3", "symfony/cache": "^4.3 || ^5 || ^6 || ^7 || ^8", "symfony/expression-language": "^4 || ^5 || ^6 || ^7 || ^8", - "webonyx/graphql-php": "^v15.0", + "webonyx/graphql-php": "^v15.38.3", "kcs/class-finder": "^0.6.0" }, "require-dev": { diff --git a/src/Http/Psr15GraphQLMiddlewareBuilder.php b/src/Http/Psr15GraphQLMiddlewareBuilder.php index cc63729e0e..90ace911ad 100644 --- a/src/Http/Psr15GraphQLMiddlewareBuilder.php +++ b/src/Http/Psr15GraphQLMiddlewareBuilder.php @@ -11,6 +11,7 @@ use GraphQL\Server\ServerConfig; use GraphQL\Type\Schema; use GraphQL\Validator\DocumentValidator; +use GraphQL\Validator\Rules\QueryComplexity; use GraphQL\Validator\Rules\ValidationRule; use Laminas\Diactoros\ResponseFactory; use Laminas\Diactoros\StreamFactory; @@ -23,7 +24,6 @@ use TheCodingMachine\GraphQLite\GraphQLRuntimeException; use TheCodingMachine\GraphQLite\Server\PersistedQuery\CachePersistedQueryLoader; use TheCodingMachine\GraphQLite\Server\PersistedQuery\NotSupportedPersistedQueryLoader; -use TheCodingMachine\GraphQLite\Validator\Rules\QueryComplexity; use function class_exists; use function is_callable; diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php deleted file mode 100644 index 2228d066d1..0000000000 --- a/src/Validator/Rules/QueryComplexity.php +++ /dev/null @@ -1,81 +0,0 @@ -variablesFailCoercion($context->getSchema(), $node)) { - throw $error; - } - } - }; - $visitor[NodeKind::DOCUMENT] = $documentVisitor; - - return $visitor; - } - - /** - * Whether the executor will refuse the variables, whichever operation of the document it runs - * - * The operation name isn't known during validation, so every operation's own variable - * definitions must fail coercion. - */ - private function variablesFailCoercion(Schema $schema, DocumentNode $document): bool - { - $failing = false; - foreach ($document->definitions as $definition) { - if (! $definition instanceof OperationDefinitionNode) { - continue; - } - - [$errors] = Values::getVariableValues($schema, $definition->variableDefinitions, $this->getRawVariableValues()); - if ($errors === null) { - return false; - } - - $failing = true; - } - - return $failing; - } -} diff --git a/tests/Integration/QueryComplexityTest.php b/tests/Integration/QueryComplexityTest.php index b12d6bd892..57b2bce26b 100644 --- a/tests/Integration/QueryComplexityTest.php +++ b/tests/Integration/QueryComplexityTest.php @@ -8,7 +8,6 @@ use GraphQL\Validator\Rules\QueryComplexity; use PHPUnit\Framework\Attributes\DataProvider; use TheCodingMachine\GraphQLite\Schema; -use TheCodingMachine\GraphQLite\Validator\Rules\QueryComplexity as GraphQLiteQueryComplexity; class QueryComplexityTest extends IntegrationTestCase { @@ -60,7 +59,7 @@ public function testExceedsAllowedQueryComplexity(): void $this->assertSame('Max query complexity should be 5 but got 60.', $result->toArray(DebugFlag::RETHROW_UNSAFE_EXCEPTIONS)['errors'][0]['message']); } - public function testInvalidVariableIsRefusedByTheExecutor(): void + public function testInvalidVariableIsRefusedWithItsLocation(): void { $schema = $this->mainContainer->get(Schema::class); assert($schema instanceof Schema); @@ -77,7 +76,7 @@ public function testInvalidVariableIsRefusedByTheExecutor(): void variableValues: ['take' => 'many'], validationRules: [ ...DocumentValidator::allRules(), - new GraphQLiteQueryComplexity(60), + new QueryComplexity(60), ] ); diff --git a/tests/Validator/Rules/QueryComplexityTest.php b/tests/Validator/Rules/QueryComplexityTest.php deleted file mode 100644 index 124bc4b763..0000000000 --- a/tests/Validator/Rules/QueryComplexityTest.php +++ /dev/null @@ -1,162 +0,0 @@ -execute( - 'query ($isbn: Isbn) { books(isbn: $isbn) { title } }', - ['isbn' => 'abc'], - )->toArray(); - - $this->assertNull($result['data'] ?? null); - $this->assertCount(1, $result['errors']); - $this->assertSame( - 'Variable "$isbn" got invalid value "abc"; An ISBN has 13 digits.', - $result['errors'][0]['message'], - ); - $this->assertSame([['line' => 1, 'column' => 8]], $result['errors'][0]['locations']); - } - - public function testEachInvalidVariableIsRefusedSeparately(): void - { - $result = $this->execute( - 'query ($first: Isbn, $second: Isbn) { a: books(isbn: $first) { title } b: books(isbn: $second) { title } }', - ['first' => 'abc', 'second' => 'def'], - )->toArray(); - - $this->assertNull($result['data'] ?? null); - $this->assertCount(2, $result['errors']); - $this->assertStringStartsWith('Variable "$first" got invalid value "abc"', $result['errors'][0]['message']); - $this->assertSame([['line' => 1, 'column' => 8]], $result['errors'][0]['locations']); - $this->assertStringStartsWith('Variable "$second" got invalid value "def"', $result['errors'][1]['message']); - $this->assertSame([['line' => 1, 'column' => 22]], $result['errors'][1]['locations']); - } - - public function testValidOperationOverTheLimitIsRefused(): void - { - $result = $this->execute( - 'query ($isbn: Isbn, $take: Int) { books(isbn: $isbn, take: $take) { title } }', - ['isbn' => '9780000000001', 'take' => 50], - )->toArray(); - - $this->assertNull($result['data'] ?? null); - $this->assertSame('Max query complexity should be 20 but got 100.', $result['errors'][0]['message']); - } - - public function testValidOperationUnderTheLimitIsExecuted(): void - { - $result = $this->execute( - 'query ($isbn: Isbn, $take: Int) { books(isbn: $isbn, take: $take) { title } }', - ['isbn' => '9780000000001', 'take' => 5], - )->toArray(); - - $this->assertArrayNotHasKey('errors', $result); - $this->assertSame(['books' => [['title' => 'Title']]], $result['data']); - } - - public function testUnrelatedPricingErrorIsNotSwallowed(): void - { - $result = $this->execute('{ unpriceable }')->toArray(); - - $this->assertNull($result['data'] ?? null); - $this->assertSame('Pricing failed.', $result['errors'][0]['message']); - } - - public function testInvalidVariableOfAnotherOperationKeepsTheLimit(): void - { - $result = $this->execute( - 'query byIsbn($isbn: Isbn) { books(isbn: $isbn) { title } } query all { books { title } }', - ['isbn' => 'abc'], - 'all', - )->toArray(); - - $this->assertNull($result['data'] ?? null); - $this->assertStringStartsWith('Variable "$isbn" got invalid value "abc"', $result['errors'][0]['message']); - $this->assertArrayNotHasKey('locations', $result['errors'][0]); - } - - /** @param array $variables */ - private function execute(string $query, array $variables = [], string|null $operationName = null): \GraphQL\Executor\ExecutionResult - { - return GraphQL::executeQuery( - $this->createSchema(), - $query, - variableValues: $variables, - operationName: $operationName, - validationRules: [ - ...DocumentValidator::allRules(), - new QueryComplexity(20), - ], - ); - } - - private function createSchema(): Schema - { - $isbn = new CustomScalarType([ - 'name' => 'Isbn', - 'serialize' => static fn (mixed $value): mixed => $value, - 'parseValue' => static function (mixed $value): string { - if (! is_string($value) || ! preg_match('/^\d{13}$/', $value)) { - throw new Error('An ISBN has 13 digits.'); - } - - return $value; - }, - 'parseLiteral' => static function (Node $node): string { - if (! $node instanceof StringValueNode || ! preg_match('/^\d{13}$/', $node->value)) { - throw new Error('An ISBN has 13 digits.'); - } - - return $node->value; - }, - ]); - - $book = new ObjectType([ - 'name' => 'Book', - 'fields' => [ - 'title' => ['type' => Type::string(), 'resolve' => static fn (): string => 'Title'], - ], - ]); - - return new Schema([ - 'query' => new ObjectType([ - 'name' => 'Query', - 'fields' => [ - 'books' => [ - 'type' => Type::listOf($book), - 'args' => [ - 'isbn' => ['type' => $isbn], - 'take' => ['type' => Type::int()], - ], - 'complexity' => static fn (int $childrenComplexity, array $args): int => ($args['take'] ?? 1) * ($childrenComplexity + 1), - 'resolve' => static fn (): array => [[]], - ], - 'unpriceable' => [ - 'type' => Type::string(), - 'complexity' => static fn (): int => throw new Error('Pricing failed.'), - 'resolve' => static fn (): string => 'Resolved', - ], - ], - ]), - ]); - } -} diff --git a/website/docs/CHANGELOG.md b/website/docs/CHANGELOG.md index 70011c5fd5..4a656a19ad 100644 --- a/website/docs/CHANGELOG.md +++ b/website/docs/CHANGELOG.md @@ -69,8 +69,8 @@ sidebar_label: Changelog reach the schema when `SchemaFactory::setDocblockDescriptionsEnabled(false)` is set. ([#838](https://github.com/thecodingmachine/graphqlite/pull/838)) @oojacoboo - `Psr15GraphQLMiddlewareBuilder::limitQueryComplexity()` no longer turns invalid variables into a single joined - error without locations. Its rule, `TheCodingMachine\GraphQLite\Validator\Rules\QueryComplexity`, leaves them - to the executor, which refuses each variable with its own located error. + error without locations. GraphQLite now requires `webonyx/graphql-php` 15.38.3, whose `QueryComplexity` rule + reports each invalid variable with its own located error. ([#839](https://github.com/thecodingmachine/graphqlite/pull/839)) @oojacoboo ## >8.0.0 diff --git a/website/docs/operation-complexity.md b/website/docs/operation-complexity.md index 3de3fd6592..88902c88d9 100644 --- a/website/docs/operation-complexity.md +++ b/website/docs/operation-complexity.md @@ -232,11 +232,5 @@ $builder = new Psr15GraphQLMiddlewareBuilder($schema); $builder->limitQueryComplexity(1000); ``` -`limitQueryComplexity()` registers GraphQLite's `TheCodingMachine\GraphQLite\Validator\Rules\QueryComplexity`, -a subclass of webonyx's `QueryComplexity` rule. Pricing fields by their arguments coerces the operation's variables -during validation; when a variable is invalid, the operation is refused by the executor with one error per variable, -including its location, rather than a single joined validation error. If you register the rule yourself with -`addValidationRule()`, prefer this class over webonyx's. - Beware that introspection queries would also be limited in complexity. A full introspection query sits at around `107` points, so we recommend a minimum of `150` for query complexity limit.