Skip to content

Commit 299be4f

Browse files
cwdtcwdt
authored
Allow passing both query and queryId (fixes #789) (#790)
1 parent 61f4b09 commit 299be4f

File tree

5 files changed

+25
-26
lines changed

5 files changed

+25
-26
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ You can find and compare releases at the [GitHub release page](https://github.co
3434
- Clarify error when attempting to coerce anything but `array` or `stdClass` to an input object
3535
- Allow directives on variable definitions
3636
- Handle `null` parent of list in `ValuesOfCorrectType::getVisitor`
37+
- Allow sending both `query` and `queryId`, ignore `queryId` in that case
3738

3839
### Removed
3940

‎docs/executing-queries.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,7 +105,7 @@ PSR-7 is useful when you want to integrate the server into existing framework:
105105
| validationRules | `array` or `callable` | A set of rules for query validation step. The default value is all available rules. The empty array would allow skipping query validation (may be convenient for persisted queries which are validated before persisting and assumed valid during execution).<br><br>Pass `callable` to return different validation rules for different queries (e.g. empty array for persisted query and a full list of rules for regular queries). When passed, it is expected to have the following signature: <br><br> **function ([OperationParams](class-reference.md#graphqlserveroperationparams) $params, DocumentNode $node, $operationType): array** |
106106
| queryBatching | `bool` | Flag indicating whether this server supports query batching ([apollo-style](https://dev-blog.apollodata.com/query-batching-in-apollo-63acfd859862)).<br><br> Defaults to **false** |
107107
| debugFlag | `int` | Debug flags. See [docs on error debugging](error-handling.md#debugging-tools) (flag values are the same). |
108-
| persistentQueryLoader | `callable` | A function which is called to fetch actual query when server encounters **queryId** in request vs **query**.<br><br> The server does not implement persistence part (which you will have to build on your own), but it allows you to execute queries which were persisted previously.<br><br> Expected function signature:<br> **function ($queryId, [OperationParams](class-reference.md#graphqlserveroperationparams) $params)** <br><br>Function is expected to return query **string** or parsed **DocumentNode** <br><br> [Read more about persisted queries](https://dev-blog.apollodata.com/persisted-graphql-queries-with-apollo-client-119fd7e6bba5). |
108+
| persistentQueryLoader | `callable` | A function which is called to fetch actual query when server encounters a **queryId** without a **query**.<br><br> The server does not implement persistence part (which you will have to build on your own), but it allows you to execute queries which were persisted previously.<br><br> Expected function signature:<br> **function ($queryId, [OperationParams](class-reference.md#graphqlserveroperationparams) $params)** <br><br>Function is expected to return query **string** or parsed **DocumentNode** <br><br> [Read more about persisted queries](https://dev-blog.apollodata.com/persisted-graphql-queries-with-apollo-client-119fd7e6bba5). |
109109
| errorFormatter | `callable` | Custom error formatter. See [error handling docs](error-handling.md#custom-error-handling-and-formatting). |
110110
| errorsHandler | `callable` | Custom errors handler. See [error handling docs](error-handling.md#custom-error-handling-and-formatting). |
111111
| promiseAdapter | [`PromiseAdapter`](class-reference.md#graphqlexecutorpromisepromiseadapter) | Required for [Async PHP](data-fetching.md#async-php) only. |

‎src/Server/Helper.php

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -167,10 +167,6 @@ public function validateOperationParams(OperationParams $params): array
167167
$errors[] = new RequestError('GraphQL Request must include at least one of those two parameters: "query" or "queryId"');
168168
}
169169

170-
if ($query !== '' && $queryId !== '') {
171-
$errors[] = new RequestError('GraphQL Request parameters "query" and "queryId" are mutually exclusive');
172-
}
173-
174170
if (! is_string($query)) {
175171
$errors[] = new RequestError(
176172
'GraphQL Request parameter "query" must be string, but got ' .
@@ -279,9 +275,9 @@ private function promiseToExecuteOperation(
279275
);
280276
}
281277

282-
$doc = ($op->queryId ?? '') === ''
283-
? $op->query
284-
: $this->loadPersistedQuery($config, $op);
278+
$doc = $op->queryId !== null && $op->query === null
279+
? $this->loadPersistedQuery($config, $op)
280+
: $op->query;
285281

286282
if (! $doc instanceof DocumentNode) {
287283
$doc = Parser::parse($doc);

‎tests/Server/QueryExecutionTest.php

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
namespace GraphQL\Tests\Server;
66

77
use DMS\PHPUnitExtensions\ArraySubset\ArraySubsetAsserts;
8+
use Exception;
89
use GraphQL\Error\DebugFlag;
910
use GraphQL\Error\Error;
1011
use GraphQL\Error\InvariantViolation;
@@ -39,7 +40,7 @@ public function setUp(): void
3940

4041
public function testSimpleQueryExecution(): void
4142
{
42-
$query = '{f1}';
43+
$query = /** @lang GraphQL */ '{ f1 }';
4344

4445
$expected = [
4546
'data' => ['f1' => 'f1'],
@@ -48,17 +49,17 @@ public function testSimpleQueryExecution(): void
4849
$this->assertQueryResultEquals($expected, $query);
4950
}
5051

51-
private function assertQueryResultEquals($expected, $query, $variables = null)
52+
private function assertQueryResultEquals($expected, $query, $variables = null, $queryId = null)
5253
{
53-
$result = $this->executeQuery($query, $variables);
54+
$result = $this->executeQuery($query, $variables, false, $queryId);
5455
self::assertArraySubset($expected, $result->toArray(DebugFlag::INCLUDE_DEBUG_MESSAGE));
5556

5657
return $result;
5758
}
5859

59-
private function executeQuery($query, $variables = null, $readonly = false)
60+
private function executeQuery($query, $variables = null, $readonly = false, $queryId = null)
6061
{
61-
$op = OperationParams::create(['query' => $query, 'variables' => $variables], $readonly);
62+
$op = OperationParams::create(['query' => $query, 'variables' => $variables, 'queryId' => $queryId], $readonly);
6263
$helper = new Helper();
6364
$result = $helper->executeOperation($this->config, $op);
6465
self::assertInstanceOf(ExecutionResult::class, $result);
@@ -443,6 +444,20 @@ public function testAllowSkippingValidationForPersistedQueries(): void
443444
self::assertEquals($expected, $result->toArray());
444445
}
445446

447+
public function testExecutesQueryWhenQueryAndQueryIdArePassed(): void
448+
{
449+
$query = /** @lang GraphQL */ '{ f1 }';
450+
451+
$expected = [
452+
'data' => ['f1' => 'f1'],
453+
];
454+
$this->config->setPersistentQueryLoader(static function (): array {
455+
throw new Exception('Should not be called since a query is also passed');
456+
});
457+
458+
$this->assertQueryResultEquals($expected, $query, [], 'some-id');
459+
}
460+
446461
public function testProhibitsUnexpectedValidationRules(): void
447462
{
448463
$this->expectException(InvariantViolation::class);

‎tests/Server/RequestValidationTest.php

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -71,19 +71,6 @@ private function assertInputError($parsedRequest, $expectedMessage)
7171
}
7272
}
7373

74-
public function testFailsWhenBothQueryAndQueryIdArePresent(): void
75-
{
76-
$parsedBody = OperationParams::create([
77-
'query' => '{my query}',
78-
'queryId' => 'my-query-id',
79-
]);
80-
81-
$this->assertInputError(
82-
$parsedBody,
83-
'GraphQL Request parameters "query" and "queryId" are mutually exclusive'
84-
);
85-
}
86-
8774
public function testFailsWhenQueryParameterIsNotString(): void
8875
{
8976
$parsedBody = OperationParams::create([

0 commit comments

Comments
 (0)