Skip to content

Parse OperationParams into strongly typed data structures #1019

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
spawnia wants to merge 1 commit into
base: master
Choose a base branch
from
+160 −128
Draft

Parse OperationParams into strongly typed data structures #1019

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
239 changes: 115 additions & 124 deletions src/Server/Helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
use GraphQL\Error\Error;
use GraphQL\Error\FormattedError;
use GraphQL\Error\InvariantViolation;
use GraphQL\Error\SyntaxError;
use GraphQL\Executor\ExecutionResult;
use GraphQL\Executor\Executor;
use GraphQL\Executor\Promise\Adapter\SyncPromiseAdapter;
Expand Down Expand Up @@ -141,54 +142,6 @@ public function parseRequestParams(string $method, array $bodyParams, array $que
throw new RequestError('HTTP Method "' . $method . '" is not supported');
}

/**
* Checks validity of OperationParams extracted from HTTP request and returns an array of errors
* if params are invalid (or empty array when params are valid)
*
* @return array<int, RequestError>
*
* @api
*/
public function validateOperationParams(OperationParams $params): array
{
$errors = [];
$query = $params->query ?? '';
$queryId = $params->queryId ?? '';
if ($query === '' && $queryId === '') {
$errors[] = new RequestError('GraphQL Request must include at least one of those two parameters: "query" or "queryId"');
}

if (! is_string($query)) {
$errors[] = new RequestError(
'GraphQL Request parameter "query" must be string, but got ' .
Utils::printSafeJson($params->query)
);
}

if (! is_string($queryId)) {
$errors[] = new RequestError(
'GraphQL Request parameter "queryId" must be string, but got ' .
Utils::printSafeJson($params->queryId)
);
}

if ($params->operation !== null && ! is_string($params->operation)) {
$errors[] = new RequestError(
'GraphQL Request parameter "operation" must be string, but got ' .
Utils::printSafeJson($params->operation)
);
}

if ($params->variables !== null && (! is_array($params->variables) || isset($params->variables[0]))) {
$errors[] = new RequestError(
'GraphQL Request parameter "variables" must be object or JSON string parsed to object, but got ' .
Utils::printSafeJson($params->originalInput['variables'])
);
}

return $errors;
}

/**
* Executes GraphQL operation with given server configuration and returns execution result
* (or promise when promise adapter is different from SyncPromiseAdapter)
Expand Down Expand Up @@ -241,72 +194,138 @@ public function executeBatch(ServerConfig $config, array $operations)
private function promiseToExecuteOperation(
PromiseAdapter $promiseAdapter,
ServerConfig $config,
OperationParams $op,
OperationParams $params,
bool $isBatch = false
): Promise {
try {
if ($config->getSchema() === null) {
throw new InvariantViolation('Schema is required for the server');
}
$schema = $config->getSchema();
if ($schema === null) {
throw new InvariantViolation('Schema is required for the server');
}

if ($isBatch && ! $config->getQueryBatching()) {
throw new RequestError('Batched queries are not supported by this server');
}
if ($isBatch && ! $config->getQueryBatching()) {
$batchedQueriesAreNotSupported = new RequestError('Batched queries are not supported by this server');

$errors = $this->validateOperationParams($op);
return $promiseAdapter->createFulfilled(
new ExecutionResult(null, [Error::createLocatedError($batchedQueriesAreNotSupported)])
);
Comment on lines +208 to +210
}

if (count($errors) > 0) {
$locatedErrors = array_map(
[Error::class, 'createLocatedError'],
$errors
);
/** @var array<int, RequestError|SyntaxError> $errors */
$errors = [];

$query = $params->query;
$queryId = $params->queryId;
$operation = $params->operation;
$variables = $params->variables;
$extensions = $params->extensions;

if ($operation !== null && ! is_string($operation)) {
$errors[] = new RequestError(
'GraphQL Request parameter "operation" must be string, but got ' .
Utils::printSafeJson($operation)
);
}

if ($variables !== null && (! is_array($variables) || isset($variables[0]))) {
$errors[] = new RequestError(
'GraphQL Request parameter "variables" must be object or JSON string parsed to object, but got ' .
Utils::printSafeJson($params->getOriginalInput('variables'))
);
}

return $promiseAdapter->createFulfilled(
new ExecutionResult(null, $locatedErrors)
if ($extensions !== null && (! is_array($extensions) || isset($variables[0]))) {
$errors[] = new RequestError(
'GraphQL Request parameter "extensions" must be object or JSON string parsed to object, but got ' .
Utils::printSafeJson($params->getOriginalInput('extensions'))
);
}

if ($queryId !== null) {
if (! is_string($queryId)) {
$errors[] = new RequestError(
'GraphQL Request parameter "queryId" must be string, but got ' .
Utils::printSafeJson($params->queryId)
Comment on lines +243 to +247
);
}

$doc = $op->queryId !== null && $op->query === null
? $this->loadPersistedQuery($config, $op)
: $op->query;
$loader = $config->getPersistedQueryLoader();

if ($loader === null) {
$errors[] = new RequestError('Persisted queries are not supported by this server');
$source = null;
} else {
$source = $loader($queryId, $params);

if (! $doc instanceof DocumentNode) {
$doc = Parser::parse($doc);
// @phpstan-ignore-next-line unless PHP gains function types, we have to check this at runtime
if (! is_string($source) && ! $source instanceof DocumentNode) {
throw new InvariantViolation(
'Persistent query loader must return query string or instance of ' . DocumentNode::class
. ' but got: ' . Utils::printSafe($source)
);
}
}
} elseif ($query !== null) {
if (! is_string($query)) {
$errors[] = new RequestError(
'GraphQL Request parameter "query" must be string, but got ' .
Utils::printSafeJson($params->query)
);
}

$operationAST = AST::getOperationAST($doc, $op->operation);
$source = $query;
} else {
$errors[] = new RequestError(
'GraphQL Request must include at least one of those two parameters: "query" or "queryId"'
);
$source = null;
}

if ($operationAST === null) {
throw new RequestError('Failed to determine operation type');
}
if (is_string($source)) {
try {
$source = Parser::parse($source);

$operationAST = AST::getOperationAST($source, $operation);

$operationType = $operationAST->operation;
if ($operationType !== 'query' && $op->readOnly) {
throw new RequestError('GET supports only query operation');
if ($operationAST === null) {
$errors[] = new RequestError('Failed to determine operation type');
}

$operationType = $operationAST->operation;
Comment on lines +289 to +293
if ($operationType !== 'query' && $params->readOnly) {
$errors[] = new RequestError('GET supports only query operation');
}
} catch (SyntaxError $error) {
$errors[] = $error;
}
}

$result = GraphQL::promiseToExecute(
$promiseAdapter,
$config->getSchema(),
$doc,
$this->resolveRootValue($config, $op, $doc, $operationType),
$this->resolveContextValue($config, $op, $doc, $operationType),
$op->variables,
$op->operation,
$config->getFieldResolver(),
$this->resolveValidationRules($config, $op, $doc, $operationType)
);
} catch (RequestError $e) {
$result = $promiseAdapter->createFulfilled(
new ExecutionResult(null, [Error::createLocatedError($e)])
);
} catch (Error $e) {
$result = $promiseAdapter->createFulfilled(
new ExecutionResult(null, [$e])
if (count($errors) > 0) {
return $promiseAdapter->createFulfilled(
new ExecutionResult(
null,
array_map([Error::class, 'createLocatedError'], $errors)
)
);
}
Comment on lines +302 to 309

$applyErrorHandling = static function (ExecutionResult $result) use ($config): ExecutionResult {
$result = GraphQL::promiseToExecute(
$promiseAdapter,
$schema,
$source,
$this->resolveRootValue($config, $params, $source, $operationType),
$this->resolveContextValue($config, $params, $source, $operationType),
$variables,
$operation,
$config->getFieldResolver(),
$this->resolveValidationRules($config, $params, $source, $operationType)
);

return $this->applyErrorHandling($result, $config);
}

private function applyErrorHandling(Promise $result, ServerConfig $config): Promise
{
return $result->then(static function (ExecutionResult $result) use ($config): ExecutionResult {
if ($config->getErrorsHandler() !== null) {
$result->setErrorsHandler($config->getErrorsHandler());
}
Expand All @@ -321,35 +340,7 @@ private function promiseToExecuteOperation(
}

return $result;
};

return $result->then($applyErrorHandling);
}

/**
* @return mixed
*
* @throws RequestError
*/
private function loadPersistedQuery(ServerConfig $config, OperationParams $operationParams)
{
$loader = $config->getPersistedQueryLoader();

if ($loader === null) {
throw new RequestError('Persisted queries are not supported by this server');
}

$source = $loader($operationParams->queryId, $operationParams);

// @phpstan-ignore-next-line Necessary until PHP gains function types
if (! is_string($source) && ! $source instanceof DocumentNode) {
throw new InvariantViolation(
'Persisted query loader must return query string or instance of ' . DocumentNode::class
. ' but got: ' . Utils::printSafe($source)
);
}

return $source;
});
}

/**
Expand Down
20 changes: 20 additions & 0 deletions src/Server/PersistedOperation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace GraphQL\Server;

class PersistedOperation
{
public string $queryId;

public ?string $operation;

/** @var array<string, mixed>|null */
public ?array $variables;

/** @var array<string, mixed>|null */
public ?array $extensions;

public bool $readOnly;
}
20 changes: 20 additions & 0 deletions src/Server/ValidOperation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<?php

declare(strict_types=1);

namespace GraphQL\Server;

class ValidOperation
{
public string $query;

public ?string $operation;

/** @var array<string, mixed>|null */
public ?array $variables;

/** @var array<string, mixed>|null */
public ?array $extensions;

public bool $readOnly;
}
5 changes: 3 additions & 2 deletions tests/Server/QueryExecutionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use GraphQL\Language\Parser;
use GraphQL\Server\Helper;
use GraphQL\Server\OperationParams;
use GraphQL\Server\PersistedOperation;
use GraphQL\Server\RequestError;
use GraphQL\Server\ServerConfig;
use GraphQL\Validator\DocumentValidator;
Expand Down Expand Up @@ -403,8 +404,8 @@ public function testPersistedQueriesAreStillValidatedByDefault(): void
public function testAllowSkippingValidationForPersistedQueries(): void
{
$this->config
->setPersistedQueryLoader(static function ($queryId) {
if ($queryId === 'some-id') {
->setPersistedQueryLoader(static function (PersistedOperation $persistedOperation): string {
if ($persistedOperation->queryId === 'some-id') {
Comment on lines +407 to +408
return '{invalid}';
}

Expand Down
4 changes: 2 additions & 2 deletions tests/Server/RequestValidationTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public function testSimpleRequestShouldValidate(): void
private static function assertValid($parsedRequest): void
{
$helper = new Helper();
$errors = $helper->validateOperationParams($parsedRequest);
$errors = $helper->parseOperationParams($parsedRequest);
self::assertEmpty($errors, isset($errors[0]) ? $errors[0]->getMessage() : '');
Comment on lines 28 to 32
}

Expand Down Expand Up @@ -63,7 +63,7 @@ public function testRequiresQueryOrQueryId(): void
private function assertInputError($parsedRequest, $expectedMessage): void
{
$helper = new Helper();
$errors = $helper->validateOperationParams($parsedRequest);
$errors = $helper->parseOperationParams($parsedRequest);
if (isset($errors[0])) {
self::assertEquals($expectedMessage, $errors[0]->getMessage());
} else {
Expand Down