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.