From 733de0c3b564a0b3abab0bf84bc42c8088043a26 Mon Sep 17 00:00:00 2001 From: Benedikt Franke Date: Thu, 25 Nov 2021 11:43:05 +0100 Subject: [PATCH] Parse OperationParams into strongly typed data structures --- src/Server/Helper.php | 239 ++++++++++++------------- src/Server/PersistedOperation.php | 20 +++ src/Server/ValidOperation.php | 20 +++ tests/Server/QueryExecutionTest.php | 5 +- tests/Server/RequestValidationTest.php | 4 +- 5 files changed, 160 insertions(+), 128 deletions(-) create mode 100644 src/Server/PersistedOperation.php create mode 100644 src/Server/ValidOperation.php diff --git a/src/Server/Helper.php b/src/Server/Helper.php index a526174f0..12a52afe2 100644 --- a/src/Server/Helper.php +++ b/src/Server/Helper.php @@ -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; @@ -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 - * - * @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) @@ -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)]) + ); + } - if (count($errors) > 0) { - $locatedErrors = array_map( - [Error::class, 'createLocatedError'], - $errors - ); + /** @var array $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) ); } - $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; + 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) + ) ); } - $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()); } @@ -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; + }); } /** diff --git a/src/Server/PersistedOperation.php b/src/Server/PersistedOperation.php new file mode 100644 index 000000000..f5ec983ea --- /dev/null +++ b/src/Server/PersistedOperation.php @@ -0,0 +1,20 @@ +|null */ + public ?array $variables; + + /** @var array|null */ + public ?array $extensions; + + public bool $readOnly; +} diff --git a/src/Server/ValidOperation.php b/src/Server/ValidOperation.php new file mode 100644 index 000000000..1a83677d1 --- /dev/null +++ b/src/Server/ValidOperation.php @@ -0,0 +1,20 @@ +|null */ + public ?array $variables; + + /** @var array|null */ + public ?array $extensions; + + public bool $readOnly; +} diff --git a/tests/Server/QueryExecutionTest.php b/tests/Server/QueryExecutionTest.php index 9f777a568..4d487fa78 100644 --- a/tests/Server/QueryExecutionTest.php +++ b/tests/Server/QueryExecutionTest.php @@ -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; @@ -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') { return '{invalid}'; } diff --git a/tests/Server/RequestValidationTest.php b/tests/Server/RequestValidationTest.php index 58a581682..7070fb97f 100644 --- a/tests/Server/RequestValidationTest.php +++ b/tests/Server/RequestValidationTest.php @@ -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() : ''); } @@ -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 {