From 60df83f47e5bddc9315e725c0a779f7eeb249c4f Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Tue, 13 Feb 2018 16:51:44 +0100 Subject: [PATCH] Preserve original coercion errors, improve error quality. This is a fairly major refactoring of coerceValue which returns an Either so it can return a complete collection of errors. This allows originalError to be preserved for scalar coercion errors and ensures *all* errors are represented in the response. This had a minor change to the logic in execute / subscribe to allow for buildExecutionContext to abrupt complete with multiple errors. ref: graphql/graphql-js#1133 --- src/Error/Error.php | 8 + src/Executor/Executor.php | 93 ++++--- src/Executor/Values.php | 280 ++++----------------- src/Type/Definition/FloatType.php | 34 ++- src/Type/Definition/IDType.php | 4 +- src/Type/Definition/IntType.php | 62 +++-- src/Type/Definition/StringType.php | 21 +- src/Utils/Utils.php | 38 +-- src/Utils/Value.php | 224 +++++++++++++++++ src/Validator/Rules/QueryComplexity.php | 23 +- tests/Executor/ValuesTest.php | 313 +++++++++++------------- tests/Executor/VariablesTest.php | 72 +++--- tests/Server/QueryExecutionTest.php | 2 +- tests/Server/RequestValidationTest.php | 6 +- tests/Type/EnumTypeTest.php | 6 +- tests/Type/ScalarSerializationTest.php | 33 ++- tests/Type/TypeLoaderTest.php | 2 +- tests/Type/ValidationTest.php | 4 +- tests/Utils/CoerceValueTest.php | 201 +++++++++++++++ tests/Utils/IsValidPHPValueTest.php | 132 ---------- tools/gendocs.php | 4 +- 21 files changed, 844 insertions(+), 718 deletions(-) create mode 100644 src/Utils/Value.php create mode 100644 tests/Utils/CoerceValueTest.php delete mode 100644 tests/Utils/IsValidPHPValueTest.php diff --git a/src/Error/Error.php b/src/Error/Error.php index ac931bf..be2c9b5 100644 --- a/src/Error/Error.php +++ b/src/Error/Error.php @@ -277,6 +277,14 @@ class Error extends \Exception implements \JsonSerializable, ClientAware return $this->locations; } + /** + * @return array|Node[]|null + */ + public function getNodes() + { + return $this->nodes; + } + /** * Returns an array describing the path from the root value to the field which produced this error. * Only included for execution errors. diff --git a/src/Executor/Executor.php b/src/Executor/Executor.php index 8ce9baa..fac9553 100644 --- a/src/Executor/Executor.php +++ b/src/Executor/Executor.php @@ -100,9 +100,16 @@ class Executor { // TODO: deprecate (just always use SyncAdapter here) and have `promiseToExecute()` for other cases $promiseAdapter = self::getPromiseAdapter(); - - $result = self::promiseToExecute($promiseAdapter, $schema, $ast, $rootValue, $contextValue, - $variableValues, $operationName, $fieldResolver); + $result = self::promiseToExecute( + $promiseAdapter, + $schema, + $ast, + $rootValue, + $contextValue, + $variableValues, + $operationName, + $fieldResolver + ); // Wait for promised results when using sync promises if ($promiseAdapter instanceof SyncPromiseAdapter) { @@ -140,11 +147,19 @@ class Executor callable $fieldResolver = null ) { - try { - $exeContext = self::buildExecutionContext($schema, $ast, $rootValue, $contextValue, $variableValues, - $operationName, $fieldResolver, $promiseAdapter); - } catch (Error $e) { - return $promiseAdapter->createFulfilled(new ExecutionResult(null, [$e])); + $exeContext = self::buildExecutionContext( + $schema, + $ast, + $rootValue, + $contextValue, + $variableValues, + $operationName, + $fieldResolver, + $promiseAdapter + ); + + if (is_array($exeContext)) { + return $promiseAdapter->createFulfilled(new ExecutionResult(null, $exeContext)); } $executor = new self($exeContext); @@ -159,13 +174,12 @@ class Executor * @param DocumentNode $documentNode * @param $rootValue * @param $contextValue - * @param $rawVariableValues + * @param array|\Traversable $rawVariableValues * @param string $operationName * @param callable $fieldResolver * @param PromiseAdapter $promiseAdapter * - * @return ExecutionContext - * @throws Error + * @return ExecutionContext|Error[] */ private static function buildExecutionContext( Schema $schema, @@ -178,30 +192,17 @@ class Executor PromiseAdapter $promiseAdapter = null ) { - if (null !== $rawVariableValues) { - Utils::invariant( - is_array($rawVariableValues) || $rawVariableValues instanceof \ArrayAccess, - "Variable values are expected to be array or instance of ArrayAccess, got " . Utils::getVariableType($rawVariableValues) - ); - } - if (null !== $operationName) { - Utils::invariant( - is_string($operationName), - "Operation name is supposed to be string, got " . Utils::getVariableType($operationName) - ); - } - $errors = []; $fragments = []; + /** @var OperationDefinitionNode $operation */ $operation = null; + $hasMultipleAssumedOperations = false; foreach ($documentNode->definitions as $definition) { switch ($definition->kind) { case NodeKind::OPERATION_DEFINITION: if (!$operationName && $operation) { - throw new Error( - 'Must provide operation name if query contains multiple operations.' - ); + $hasMultipleAssumedOperations = true; } if (!$operationName || (isset($definition->name) && $definition->name->value === $operationName)) { @@ -216,19 +217,40 @@ class Executor if (!$operation) { if ($operationName) { - throw new Error("Unknown operation named \"$operationName\"."); + $errors[] = new Error("Unknown operation named \"$operationName\"."); } else { - throw new Error('Must provide an operation.'); + $errors[] = new Error('Must provide an operation.'); + } + } else if ($hasMultipleAssumedOperations) { + $errors[] = new Error( + 'Must provide operation name if query contains multiple operations.' + ); + + } + + $variableValues = null; + if ($operation) { + $coercedVariableValues = Values::getVariableValues( + $schema, + $operation->variableDefinitions ?: [], + $rawVariableValues ?: [] + ); + + if ($coercedVariableValues['errors']) { + $errors = array_merge($errors, $coercedVariableValues['errors']); + } else { + $variableValues = $coercedVariableValues['coerced']; } } - $variableValues = Values::getVariableValues( - $schema, - $operation->variableDefinitions ?: [], - $rawVariableValues ?: [] - ); + if ($errors) { + return $errors; + } - $exeContext = new ExecutionContext( + Utils::invariant($operation, 'Has operation if no errors.'); + Utils::invariant($variableValues !== null, 'Has variables if no errors.'); + + return new ExecutionContext( $schema, $fragments, $rootValue, @@ -239,7 +261,6 @@ class Executor $fieldResolver ?: self::$defaultFieldResolver, $promiseAdapter ?: self::getPromiseAdapter() ); - return $exeContext; } /** diff --git a/src/Executor/Values.php b/src/Executor/Values.php index 0644db0..ef6a8cf 100644 --- a/src/Executor/Values.php +++ b/src/Executor/Values.php @@ -26,6 +26,7 @@ use GraphQL\Type\Definition\Type; use GraphQL\Utils\AST; use GraphQL\Utils\TypeInfo; use GraphQL\Utils\Utils; +use GraphQL\Utils\Value; use GraphQL\Validator\DocumentValidator; class Values @@ -36,56 +37,62 @@ class Values * to match the variable definitions, a Error will be thrown. * * @param Schema $schema - * @param VariableDefinitionNode[] $definitionNodes + * @param VariableDefinitionNode[] $varDefNodes * @param array $inputs * @return array - * @throws Error */ - public static function getVariableValues(Schema $schema, $definitionNodes, array $inputs) + public static function getVariableValues(Schema $schema, $varDefNodes, array $inputs) { + $errors = []; $coercedValues = []; - foreach ($definitionNodes as $definitionNode) { - $varName = $definitionNode->variable->name->value; - $varType = TypeInfo::typeFromAST($schema, $definitionNode->type); + foreach ($varDefNodes as $varDefNode) { + $varName = $varDefNode->variable->name->value; + /** @var InputType|Type $varType */ + $varType = TypeInfo::typeFromAST($schema, $varDefNode->type); if (!Type::isInputType($varType)) { - throw new Error( - 'Variable "$'.$varName.'" expected value of type ' . - '"' . Printer::doPrint($definitionNode->type) . '" which cannot be used as an input type.', - [$definitionNode->type] + $errors[] = new Error( + "Variable \"\$$varName\" expected value of type " . + '"' . Printer::doPrint($varDefNode->type) . '" which cannot be used as an input type.', + [$varDefNode->type] ); - } - - if (!array_key_exists($varName, $inputs)) { - $defaultValue = $definitionNode->defaultValue; - if ($defaultValue) { - $coercedValues[$varName] = AST::valueFromAST($defaultValue, $varType); - } - if ($varType instanceof NonNull) { - throw new Error( - 'Variable "$'.$varName .'" of required type ' . - '"'. Utils::printSafe($varType) . '" was not provided.', - [$definitionNode] - ); - } } else { - $value = $inputs[$varName]; - $errors = self::isValidPHPValue($value, $varType); - if (!empty($errors)) { - $message = "\n" . implode("\n", $errors); - throw new Error( - 'Variable "$' . $varName . '" got invalid value ' . - json_encode($value) . '.' . $message, - [$definitionNode] - ); - } + if (!array_key_exists($varName, $inputs)) { + if ($varType instanceof NonNull) { + $errors[] = new Error( + "Variable \"\$$varName\" of required type " . + "\"{$varType}\" was not provided.", + [$varDefNode] + ); + } else if ($varDefNode->defaultValue) { + $coercedValues[$varName] = AST::valueFromAST($varDefNode->defaultValue, $varType); + } + } else { + $value = $inputs[$varName]; + $coerced = Value::coerceValue($value, $varType, $varDefNode); + /** @var Error[] $coercionErrors */ + $coercionErrors = $coerced['errors']; + if ($coercionErrors) { + $messagePrelude = "Variable \"\$$varName\" got invalid value " . Utils::printSafeJson($value) . '; '; - $coercedValue = self::coerceValue($varType, $value); - Utils::invariant($coercedValue !== Utils::undefined(), 'Should have reported error.'); - $coercedValues[$varName] = $coercedValue; + foreach($coercionErrors as $error) { + $errors[] = new Error( + $messagePrelude . $error->getMessage(), + $error->getNodes(), + $error->getSource(), + $error->getPositions(), + $error->getPath(), + $error, + $error->getExtensions() + ); + } + } else { + $coercedValues[$varName] = $coerced['value']; + } + } } } - return $coercedValues; + return ['errors' => $errors, 'coerced' => $errors ? null : $coercedValues]; } /** @@ -206,203 +213,16 @@ class Values } /** - * Given a PHP value and a GraphQL type, determine if the value will be - * accepted for that type. This is primarily useful for validating the - * runtime values of query variables. - * + * @deprecated as of 0.12 (Use coerceValue() directly for richer information) * @param $value * @param InputType $type * @return array */ public static function isValidPHPValue($value, InputType $type) { - // A value must be provided if the type is non-null. - if ($type instanceof NonNull) { - if (null === $value) { - return ['Expected "' . Utils::printSafe($type) . '", found null.']; - } - return self::isValidPHPValue($value, $type->getWrappedType()); - } - - if (null === $value) { - return []; - } - - // Lists accept a non-list value as a list of one. - if ($type instanceof ListOfType) { - $itemType = $type->getWrappedType(); - if (is_array($value)) { - $tmp = []; - foreach ($value as $index => $item) { - $errors = self::isValidPHPValue($item, $itemType); - $tmp = array_merge($tmp, Utils::map($errors, function ($error) use ($index) { - return "In element #$index: $error"; - })); - } - return $tmp; - } - return self::isValidPHPValue($value, $itemType); - } - - // Input objects check each defined field. - if ($type instanceof InputObjectType) { - if (!is_object($value) && !is_array($value)) { - return ["Expected \"{$type->name}\", found not an object."]; - } - $fields = $type->getFields(); - $errors = []; - - // Ensure every provided field is defined. - $props = is_object($value) ? get_object_vars($value) : $value; - foreach ($props as $providedField => $tmp) { - if (!isset($fields[$providedField])) { - $errors[] = "In field \"{$providedField}\": Unknown field."; - } - } - - // Ensure every defined field is valid. - foreach ($fields as $fieldName => $tmp) { - $newErrors = self::isValidPHPValue(isset($value[$fieldName]) ? $value[$fieldName] : null, $fields[$fieldName]->getType()); - $errors = array_merge( - $errors, - Utils::map($newErrors, function ($error) use ($fieldName) { - return "In field \"{$fieldName}\": {$error}"; - }) - ); - } - return $errors; - } - - if ($type instanceof EnumType) { - if (!is_string($value) || !$type->getValue($value)) { - $printed = Utils::printSafeJson($value); - return ["Expected type \"{$type->name}\", found $printed."]; - } - - return []; - } - - Utils::invariant($type instanceof ScalarType, 'Must be a scalar type'); - /** @var ScalarType $type */ - - // Scalars determine if a value is valid via parseValue(). - try { - $parseResult = $type->parseValue($value); - if (Utils::isInvalid($parseResult)) { - $printed = Utils::printSafeJson($value); - return [ - "Expected type \"{$type->name}\", found $printed." - ]; - } - } catch (\Exception $error) { - $printed = Utils::printSafeJson($value); - $message = $error->getMessage(); - return ["Expected type \"{$type->name}\", found $printed; $message"]; - } catch (\Throwable $error) { - $printed = Utils::printSafeJson($value); - $message = $error->getMessage(); - return ["Expected type \"{$type->name}\", found $printed; $message"]; - } - - return []; - } - - /** - * Given a type and any value, return a runtime value coerced to match the type. - */ - private static function coerceValue(Type $type, $value) - { - $undefined = Utils::undefined(); - if ($value === $undefined) { - return $undefined; - } - - if ($type instanceof NonNull) { - if ($value === null) { - // Intentionally return no value. - return $undefined; - } - return self::coerceValue($type->getWrappedType(), $value); - } - - if (null === $value) { - return null; - } - - if ($type instanceof ListOfType) { - $itemType = $type->getWrappedType(); - if (is_array($value) || $value instanceof \Traversable) { - $coercedValues = []; - foreach ($value as $item) { - $itemValue = self::coerceValue($itemType, $item); - if ($undefined === $itemValue) { - // Intentionally return no value. - return $undefined; - } - $coercedValues[] = $itemValue; - } - return $coercedValues; - } else { - $coercedValue = self::coerceValue($itemType, $value); - if ($coercedValue === $undefined) { - // Intentionally return no value. - return $undefined; - } - return [$coercedValue]; - } - } - - if ($type instanceof InputObjectType) { - $coercedObj = []; - $fields = $type->getFields(); - foreach ($fields as $fieldName => $field) { - if (!array_key_exists($fieldName, $value)) { - if ($field->defaultValueExists()) { - $coercedObj[$fieldName] = $field->defaultValue; - } else if ($field->getType() instanceof NonNull) { - // Intentionally return no value. - return $undefined; - } - continue; - } - $fieldValue = self::coerceValue($field->getType(), $value[$fieldName]); - if ($fieldValue === $undefined) { - // Intentionally return no value. - return $undefined; - } - $coercedObj[$fieldName] = $fieldValue; - } - return $coercedObj; - } - - if ($type instanceof EnumType) { - if (!is_string($value) || !$type->getValue($value)) { - return $undefined; - } - - $enumValue = $type->getValue($value); - if (!$enumValue) { - return $undefined; - } - - return $enumValue->value; - } - - Utils::invariant($type instanceof ScalarType, 'Must be a scalar type'); - /** @var ScalarType $type */ - - // Scalars determine if a value is valid via parseValue(). - try { - $parseResult = $type->parseValue($value); - if (Utils::isInvalid($parseResult)) { - return $undefined; - } - } catch (\Exception $error) { - return $undefined; - } catch (\Throwable $error) { - return $undefined; - } - - return $parseResult; + $errors = Value::coerceValue($value, $type)['errors']; + return $errors + ? array_map(function(/*\Throwable */$error) { return $error->getMessage(); }, $errors) + : []; } } diff --git a/src/Type/Definition/FloatType.php b/src/Type/Definition/FloatType.php index 826b017..e6391de 100644 --- a/src/Type/Definition/FloatType.php +++ b/src/Type/Definition/FloatType.php @@ -1,7 +1,7 @@ coerceFloat($value); } /** * @param mixed $value * @return float|null + * @throws Error */ public function parseValue($value) { - return (is_numeric($value) && !is_string($value)) ? (float) $value : Utils::undefined(); + return $this->coerceFloat($value); } /** @@ -64,4 +57,21 @@ values as specified by } return Utils::undefined(); } + + private function coerceFloat($value) { + if ($value === '') { + throw new Error( + 'Float cannot represent non numeric value: (empty string)' + ); + } + + if (!is_numeric($value) && $value !== true && $value !== false) { + throw new Error( + 'Float cannot represent non numeric value: ' . + Utils::printSafe($value) + ); + } + + return (float) $value; + } } diff --git a/src/Type/Definition/IDType.php b/src/Type/Definition/IDType.php index 47ed897..d60a3a1 100644 --- a/src/Type/Definition/IDType.php +++ b/src/Type/Definition/IDType.php @@ -1,7 +1,7 @@ self::MAX_INT || $value < self::MIN_INT) { - throw new InvariantViolation(sprintf( - 'Int cannot represent non 32-bit signed integer value: %s', - Utils::printSafe($value) - )); - } - $num = (float) $value; - - // The GraphQL specification does not allow serializing non-integer values - // as Int to avoid accidental data loss. - // Examples: 1.0 == 1; 1.1 != 1, etc - if ($num != (int) $value) { - // Additionally account for scientific notation (i.e. 1e3), because (float)'1e3' is 1000, but (int)'1e3' is 1 - $trimmed = floor($num); - if ($trimmed !== $num) { - throw new InvariantViolation(sprintf( - 'Int cannot represent non-integer value: %s', - Utils::printSafe($value) - )); - } - } - return (int) $value; + return $this->coerceInt($value); } /** * @param mixed $value * @return int|null + * @throws Error */ public function parseValue($value) { - // Below is a fix against PHP bug where (in some combinations of OSs and versions) - // boundary values are treated as "double" vs "integer" and failing is_int() check - $isInt = is_int($value) || $value === self::MIN_INT || $value === self::MAX_INT; - return $isInt && $value <= self::MAX_INT && $value >= self::MIN_INT ? $value : Utils::undefined(); + return $this->coerceInt($value); } /** @@ -94,4 +66,28 @@ values. Int can represent values between -(2^31) and 2^31 - 1. '; } return Utils::undefined(); } + + private function coerceInt($value) { + if ($value === '') { + throw new Error( + 'Int cannot represent non 32-bit signed integer value: (empty string)' + ); + } + + $num = floatval($value); + if (!is_numeric($value) && !is_bool($value) || $num > self::MAX_INT || $num < self::MIN_INT) { + throw new Error( + 'Int cannot represent non 32-bit signed integer value: ' . + Utils::printSafe($value) + ); + } + $int = intval($num); + if ($int != $num) { + throw new Error( + 'Int cannot represent non-integer value: ' . + Utils::printSafe($value) + ); + } + return $int; + } } diff --git a/src/Type/Definition/StringType.php b/src/Type/Definition/StringType.php index 98dab82..a17bdcc 100644 --- a/src/Type/Definition/StringType.php +++ b/src/Type/Definition/StringType.php @@ -1,7 +1,7 @@ coerceString($value); } /** * @param mixed $value * @return string + * @throws Error */ public function parseValue($value) { - return is_string($value) ? $value : Utils::undefined(); + return $this->coerceString($value); } /** @@ -66,4 +68,15 @@ represent free-form human-readable text.'; } return Utils::undefined(); } + + private function coerceString($value) { + if (is_array($value)) { + throw new Error( + 'String cannot represent an array value: ' . + Utils::printSafe($value) + ); + } + + return (string) $value; + } } diff --git a/src/Utils/Utils.php b/src/Utils/Utils.php index cb019ef..0b48c41 100644 --- a/src/Utils/Utils.php +++ b/src/Utils/Utils.php @@ -269,22 +269,7 @@ class Utils $var = (array) $var; } if (is_array($var)) { - $count = count($var); - if (!isset($var[0]) && $count > 0) { - $keys = []; - $keyCount = 0; - foreach ($var as $key => $value) { - $keys[] = '"' . $key . '"'; - if ($keyCount++ > 4) { - break; - } - } - $keysLabel = $keyCount === 1 ? 'key' : 'keys'; - $msg = "object with first $keysLabel: " . implode(', ', $keys); - } else { - $msg = "array($count)"; - } - return $msg; + return json_encode($var); } if ('' === $var) { return '(empty string)'; @@ -296,7 +281,7 @@ class Utils return 'false'; } if (true === $var) { - return 'false'; + return 'true'; } if (is_string($var)) { return "\"$var\""; @@ -320,22 +305,7 @@ class Utils return 'instance of ' . get_class($var); } if (is_array($var)) { - $count = count($var); - if (!isset($var[0]) && $count > 0) { - $keys = []; - $keyCount = 0; - foreach ($var as $key => $value) { - $keys[] = '"' . $key . '"'; - if ($keyCount++ > 4) { - break; - } - } - $keysLabel = $keyCount === 1 ? 'key' : 'keys'; - $msg = "associative array($count) with first $keysLabel: " . implode(', ', $keys); - } else { - $msg = "array($count)"; - } - return $msg; + return json_encode($var); } if ('' === $var) { return '(empty string)'; @@ -350,7 +320,7 @@ class Utils return 'true'; } if (is_string($var)) { - return "\"$var\""; + return $var; } if (is_scalar($var)) { return (string) $var; diff --git a/src/Utils/Value.php b/src/Utils/Value.php new file mode 100644 index 0000000..6d45c49 --- /dev/null +++ b/src/Utils/Value.php @@ -0,0 +1,224 @@ +getWrappedType(), $blameNode, $path); + } + + if (null === $value) { + // Explicitly return the value null. + return self::ofValue(null); + } + + if ($type instanceof ScalarType) { + // Scalars determine if a value is valid via parseValue(), which can + // throw to indicate failure. If it throws, maintain a reference to + // the original error. + try { + $parseResult = $type->parseValue($value); + if (Utils::isInvalid($parseResult)) { + return self::ofErrors([ + self::coercionError("Expected type {$type->name}", $blameNode, $path), + ]); + } + + return self::ofValue($parseResult); + } catch (\Exception $error) { + return self::ofErrors([ + self::coercionError("Expected type {$type->name}", $blameNode, $path, $error), + ]); + } catch (\Throwable $error) { + return self::ofErrors([ + self::coercionError("Expected type {$type->name}", $blameNode, $path, $error), + ]); + } + } + + if ($type instanceof EnumType) { + if (is_string($value)) { + $enumValue = $type->getValue($value); + if ($enumValue) { + return self::ofValue($enumValue->value); + } + } + + return self::ofErrors([ + self::coercionError("Expected type {$type->name}", $blameNode, $path), + ]); + } + + if ($type instanceof ListOfType) { + $itemType = $type->getWrappedType(); + if (is_array($value) || $value instanceof \Traversable) { + $errors = []; + $coercedValue = []; + foreach ($value as $index => $itemValue) { + $coercedItem = self::coerceValue( + $itemValue, + $itemType, + $blameNode, + self::atPath($path, $index) + ); + if ($coercedItem['errors']) { + $errors = self::add($errors, $coercedItem['errors']); + } else { + $coercedValue[] = $coercedItem['value']; + } + } + return $errors ? self::ofErrors($errors) : self::ofValue($coercedValue); + } + // Lists accept a non-list value as a list of one. + $coercedItem = self::coerceValue($value, $itemType, $blameNode); + return $coercedItem['errors'] ? $coercedItem : self::ofValue([$coercedItem['value']]); + } + + if ($type instanceof InputObjectType) { + if (!is_object($value) && !is_array($value) && !$value instanceof \Traversable) { + return self::ofErrors([ + self::coercionError("Expected object type {$type->name}", $blameNode, $path), + ]); + } + + $errors = []; + $coercedValue = []; + $fields = $type->getFields(); + foreach ($fields as $fieldName => $field) { + if (!array_key_exists($fieldName, $value)) { + if ($field->defaultValueExists()) { + $coercedValue[$fieldName] = $field->defaultValue; + } else if ($field->getType() instanceof NonNull) { + $fieldPath = self::printPath(self::atPath($path, $fieldName)); + $errors = self::add( + $errors, + self::coercionError( + "Field {$fieldPath} of required " . + "type {$field->type} was not provided", + $blameNode + ) + ); + } + } else { + $fieldValue = $value[$fieldName]; + $coercedField = self::coerceValue( + $fieldValue, + $field->getType(), + $blameNode, + self::atPath($path, $fieldName) + ); + if ($coercedField['errors']) { + $errors = self::add($errors, $coercedField['errors']); + } else { + $coercedValue[$fieldName] = $coercedField['value']; + } + } + } + + // Ensure every provided field is defined. + foreach ($value as $fieldName => $field) { + if (!array_key_exists($fieldName, $fields)) { + $errors = self::add( + $errors, + self::coercionError( + "Field \"{$fieldName}\" is not defined by type {$type->name}", + $blameNode, + $path + ) + ); + } + } + + return $errors ? self::ofErrors($errors) : self::ofValue($coercedValue); + } + + throw new Error("Unexpected type {$type}"); + } + + private static function ofValue($value) { + return ['errors' => null, 'value' => $value]; + } + + private static function ofErrors($errors) { + return ['errors' => $errors, 'value' => Utils::undefined()]; + } + + private static function add($errors, $moreErrors) { + return array_merge($errors, is_array($moreErrors) ? $moreErrors : [$moreErrors]); + } + + private static function atPath($prev, $key) { + return ['prev' => $prev, 'key' => $key]; + } + + /** + * @param string $message + * @param Node $blameNode + * @param array|null $path + * @param \Exception|\Throwable|null $originalError + * @return Error + */ + private static function coercionError($message, $blameNode, array $path = null, $originalError = null) { + $pathStr = self::printPath($path); + // Return a GraphQLError instance + return new Error( + $message . + ($pathStr ? ' at ' . $pathStr : '') . + ($originalError && $originalError->getMessage() + ? '; ' . $originalError->getMessage() + : '.'), + $blameNode, + null, + null, + null, + $originalError + ); + } + + /** + * Build a string describing the path into the value where the error was found + * + * @param $path + * @return string + */ + private static function printPath(array $path = null) { + $pathStr = ''; + $currentPath = $path; + while($currentPath) { + $pathStr = + (is_string($currentPath['key']) + ? '.' . $currentPath['key'] + : '[' . $currentPath['key'] . ']') . $pathStr; + $currentPath = $currentPath['prev']; + } + return $pathStr ? 'value' . $pathStr : ''; + } +} diff --git a/src/Validator/Rules/QueryComplexity.php b/src/Validator/Rules/QueryComplexity.php index f20334e..0b1d611 100644 --- a/src/Validator/Rules/QueryComplexity.php +++ b/src/Validator/Rules/QueryComplexity.php @@ -203,11 +203,21 @@ class QueryComplexity extends AbstractQuerySecurity $args = []; if ($fieldDef instanceof FieldDefinition) { - $variableValues = Values::getVariableValues( + $variableValuesResult = Values::getVariableValues( $this->context->getSchema(), $this->variableDefs, $rawVariableValues ); + + if ($variableValuesResult['errors']) { + throw new Error(implode("\n\n", array_map( + function ($error) { + return $error->getMessage(); + } + , $variableValuesResult['errors']))); + } + $variableValues = $variableValuesResult['coerced']; + $args = Values::getArgumentValues($fieldDef, $node, $variableValues); } @@ -220,12 +230,21 @@ class QueryComplexity extends AbstractQuerySecurity return false; } - $variableValues = Values::getVariableValues( + $variableValuesResult = Values::getVariableValues( $this->context->getSchema(), $this->variableDefs, $this->getRawVariableValues() ); + if ($variableValuesResult['errors']) { + throw new Error(implode("\n\n", array_map( + function ($error) { + return $error->getMessage(); + } + , $variableValuesResult['errors']))); + } + $variableValues = $variableValuesResult['coerced']; + if ($directiveNode->name->value === 'include') { $directive = Directive::includeDirective(); $directiveArgs = Values::getArgumentValues($directive, $directiveNode, $variableValues); diff --git a/tests/Executor/ValuesTest.php b/tests/Executor/ValuesTest.php index d50fb86..1fe81cf 100644 --- a/tests/Executor/ValuesTest.php +++ b/tests/Executor/ValuesTest.php @@ -12,197 +12,162 @@ use GraphQL\Type\Schema; class ValuesTest extends \PHPUnit_Framework_TestCase { - public function testGetIDVariableValues() - { - $this->expectInputVariablesMatchOutputVariables(['idInput' => '123456789']); - $this->assertEquals( - ['idInput' => '123456789'], - self::runTestCase(['idInput' => 123456789]), - 'Integer ID was not converted to string' - ); - } + public function testGetIDVariableValues() + { + $this->expectInputVariablesMatchOutputVariables(['idInput' => '123456789']); + $this->assertEquals( + ['errors'=> [], 'coerced' => ['idInput' => '123456789']], + self::runTestCase(['idInput' => 123456789]), + 'Integer ID was not converted to string' + ); + } - public function testGetBooleanVariableValues() - { - $this->expectInputVariablesMatchOutputVariables(['boolInput' => true]); - $this->expectInputVariablesMatchOutputVariables(['boolInput' => false]); - } + public function testGetBooleanVariableValues() + { + $this->expectInputVariablesMatchOutputVariables(['boolInput' => true]); + $this->expectInputVariablesMatchOutputVariables(['boolInput' => false]); + } - public function testGetIntVariableValues() - { - $this->expectInputVariablesMatchOutputVariables(['intInput' => -1]); - $this->expectInputVariablesMatchOutputVariables(['intInput' => 0]); - $this->expectInputVariablesMatchOutputVariables(['intInput' => 1]); + public function testGetIntVariableValues() + { + $this->expectInputVariablesMatchOutputVariables(['intInput' => -1]); + $this->expectInputVariablesMatchOutputVariables(['intInput' => 0]); + $this->expectInputVariablesMatchOutputVariables(['intInput' => 1]); - // Test the int size limit - $this->expectInputVariablesMatchOutputVariables(['intInput' => 2147483647]); - $this->expectInputVariablesMatchOutputVariables(['intInput' => -2147483648]); - } + // Test the int size limit + $this->expectInputVariablesMatchOutputVariables(['intInput' => 2147483647]); + $this->expectInputVariablesMatchOutputVariables(['intInput' => -2147483648]); + } - public function testGetStringVariableValues() - { - $this->expectInputVariablesMatchOutputVariables(['stringInput' => 'meow']); - $this->expectInputVariablesMatchOutputVariables(['stringInput' => '']); - $this->expectInputVariablesMatchOutputVariables(['stringInput' => '1']); - $this->expectInputVariablesMatchOutputVariables(['stringInput' => '0']); - $this->expectInputVariablesMatchOutputVariables(['stringInput' => 'false']); - $this->expectInputVariablesMatchOutputVariables(['stringInput' => '1.2']); - } + public function testGetStringVariableValues() + { + $this->expectInputVariablesMatchOutputVariables(['stringInput' => 'meow']); + $this->expectInputVariablesMatchOutputVariables(['stringInput' => '']); + $this->expectInputVariablesMatchOutputVariables(['stringInput' => '1']); + $this->expectInputVariablesMatchOutputVariables(['stringInput' => '0']); + $this->expectInputVariablesMatchOutputVariables(['stringInput' => 'false']); + $this->expectInputVariablesMatchOutputVariables(['stringInput' => '1.2']); + } - public function testGetFloatVariableValues() - { - $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.2]); - $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.0]); - $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1]); - $this->expectInputVariablesMatchOutputVariables(['floatInput' => 0]); - $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1e3]); - } + public function testGetFloatVariableValues() + { + $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.2]); + $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.0]); + $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1]); + $this->expectInputVariablesMatchOutputVariables(['floatInput' => 0]); + $this->expectInputVariablesMatchOutputVariables(['floatInput' => 1e3]); + } - public function testBooleanForIDVariableThrowsError() - { - $this->expectGraphQLError(['idInput' => true]); - } + public function testBooleanForIDVariableThrowsError() + { + $this->expectGraphQLError(['idInput' => true]); + } - public function testFloatForIDVariableThrowsError() - { - $this->expectGraphQLError(['idInput' => 1.0]); - } + public function testFloatForIDVariableThrowsError() + { + $this->expectGraphQLError(['idInput' => 1.0]); + } - public function testStringForBooleanVariableThrowsError() - { - $this->expectGraphQLError(['boolInput' => 'true']); - } + public function testStringForBooleanVariableThrowsError() + { + $this->expectGraphQLError(['boolInput' => 'true']); + } - public function testIntForBooleanVariableThrowsError() - { - $this->expectGraphQLError(['boolInput' => 1]); - } + public function testIntForBooleanVariableThrowsError() + { + $this->expectGraphQLError(['boolInput' => 1]); + } - public function testFloatForBooleanVariableThrowsError() - { - $this->expectGraphQLError(['boolInput' => 1.0]); - } + public function testFloatForBooleanVariableThrowsError() + { + $this->expectGraphQLError(['boolInput' => 1.0]); + } - public function testBooleanForIntVariableThrowsError() - { - $this->expectGraphQLError(['intInput' => true]); - } + public function testStringForIntVariableThrowsError() + { + $this->expectGraphQLError(['intInput' => 'true']); + } - public function testStringForIntVariableThrowsError() - { - $this->expectGraphQLError(['intInput' => 'true']); - } + public function testPositiveBigIntForIntVariableThrowsError() + { + $this->expectGraphQLError(['intInput' => 2147483648]); + } - public function testFloatForIntVariableThrowsError() - { - $this->expectGraphQLError(['intInput' => 1.0]); - } + public function testNegativeBigIntForIntVariableThrowsError() + { + $this->expectGraphQLError(['intInput' => -2147483649]); + } - public function testPositiveBigIntForIntVariableThrowsError() - { - $this->expectGraphQLError(['intInput' => 2147483648]); - } + // Helpers for running test cases and making assertions - public function testNegativeBigIntForIntVariableThrowsError() - { - $this->expectGraphQLError(['intInput' => -2147483649]); - } + private function expectInputVariablesMatchOutputVariables($variables) + { + $this->assertEquals( + $variables, + self::runTestCase($variables)['coerced'], + 'Output variables did not match input variables' . PHP_EOL . var_export($variables, true) . PHP_EOL + ); + } - public function testBooleanForStringVariableThrowsError() - { - $this->expectGraphQLError(['stringInput' => true]); - } + private function expectGraphQLError($variables) + { + $result = self::runTestCase($variables); + $this->assertGreaterThan(0, count($result['errors'])); + } - public function testIntForStringVariableThrowsError() - { - $this->expectGraphQLError(['stringInput' => 1]); - } + private static $schema; - public function testFloatForStringVariableThrowsError() - { - $this->expectGraphQLError(['stringInput' => 1.0]); - } + private static function getSchema() + { + if (!self::$schema) { + self::$schema = new Schema([ + 'query' => new ObjectType([ + 'name' => 'Query', + 'fields' => [ + 'test' => [ + 'type' => Type::boolean(), + 'args' => [ + 'idInput' => Type::id(), + 'boolInput' => Type::boolean(), + 'intInput' => Type::int(), + 'stringInput' => Type::string(), + 'floatInput' => Type::float() + ] + ], + ] + ]) + ]); + } + return self::$schema; + } - public function testBooleanForFloatVariableThrowsError() - { - $this->expectGraphQLError(['floatInput' => true]); - } + private static function getVariableDefinitionNodes() + { + $idInputDefinition = new VariableDefinitionNode([ + 'variable' => new VariableNode(['name' => new NameNode(['value' => 'idInput'])]), + 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'ID'])]) + ]); + $boolInputDefinition = new VariableDefinitionNode([ + 'variable' => new VariableNode(['name' => new NameNode(['value' => 'boolInput'])]), + 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Boolean'])]) + ]); + $intInputDefinition = new VariableDefinitionNode([ + 'variable' => new VariableNode(['name' => new NameNode(['value' => 'intInput'])]), + 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Int'])]) + ]); + $stringInputDefintion = new VariableDefinitionNode([ + 'variable' => new VariableNode(['name' => new NameNode(['value' => 'stringInput'])]), + 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'String'])]) + ]); + $floatInputDefinition = new VariableDefinitionNode([ + 'variable' => new VariableNode(['name' => new NameNode(['value' => 'floatInput'])]), + 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Float'])]) + ]); + return [$idInputDefinition, $boolInputDefinition, $intInputDefinition, $stringInputDefintion, $floatInputDefinition]; + } - public function testStringForFloatVariableThrowsError() - { - $this->expectGraphQLError(['floatInput' => '1.0']); - } - - // Helpers for running test cases and making assertions - - private function expectInputVariablesMatchOutputVariables($variables) - { - $this->assertEquals( - $variables, - self::runTestCase($variables), - 'Output variables did not match input variables' . PHP_EOL . var_export($variables, true) . PHP_EOL - ); - } - - private function expectGraphQLError($variables) - { - $this->setExpectedException(\GraphQL\Error\Error::class); - self::runTestCase($variables); - } - - private static $schema; - - private static function getSchema() - { - if (!self::$schema) { - self::$schema = new Schema([ - 'query' => new ObjectType([ - 'name' => 'Query', - 'fields' => [ - 'test' => [ - 'type' => Type::boolean(), - 'args' => [ - 'idInput' => Type::id(), - 'boolInput' => Type::boolean(), - 'intInput' => Type::int(), - 'stringInput' => Type::string(), - 'floatInput' => Type::float() - ] - ], - ] - ]) - ]); - } - return self::$schema; - } - - private static function getVariableDefinitionNodes() - { - $idInputDefinition = new VariableDefinitionNode([ - 'variable' => new VariableNode(['name' => new NameNode(['value' => 'idInput'])]), - 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'ID'])]) - ]); - $boolInputDefinition = new VariableDefinitionNode([ - 'variable' => new VariableNode(['name' => new NameNode(['value' => 'boolInput'])]), - 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Boolean'])]) - ]); - $intInputDefinition = new VariableDefinitionNode([ - 'variable' => new VariableNode(['name' => new NameNode(['value' => 'intInput'])]), - 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Int'])]) - ]); - $stringInputDefintion = new VariableDefinitionNode([ - 'variable' => new VariableNode(['name' => new NameNode(['value' => 'stringInput'])]), - 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'String'])]) - ]); - $floatInputDefinition = new VariableDefinitionNode([ - 'variable' => new VariableNode(['name' => new NameNode(['value' => 'floatInput'])]), - 'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Float'])]) - ]); - return [$idInputDefinition, $boolInputDefinition, $intInputDefinition, $stringInputDefintion, $floatInputDefinition]; - } - - private function runTestCase($variables) - { - return Values::getVariableValues(self::getSchema(), self::getVariableDefinitionNodes(), $variables); - } -} \ No newline at end of file + private function runTestCase($variables) + { + return Values::getVariableValues(self::getSchema(), self::getVariableDefinitionNodes(), $variables); + } +} diff --git a/tests/Executor/VariablesTest.php b/tests/Executor/VariablesTest.php index 418ed12..0180bbd 100644 --- a/tests/Executor/VariablesTest.php +++ b/tests/Executor/VariablesTest.php @@ -3,6 +3,7 @@ namespace GraphQL\Tests\Executor; require_once __DIR__ . '/TestClasses.php'; +use GraphQL\Error\Error; use GraphQL\Error\InvariantViolation; use GraphQL\Executor\Executor; use GraphQL\Language\Parser; @@ -134,7 +135,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase ]; $this->assertEquals($expected, $result); - // properly parses single value to array: + // properly parses single value to list: $params = ['input' => ['a' => 'foo', 'b' => 'bar', 'c' => 'baz']]; $this->assertEquals( ['data' => ['fieldWithObjectInput' => '{"a":"foo","b":["bar"],"c":"baz"}']], @@ -158,13 +159,15 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value {"a":"foo","b":"bar","c":null}.'. "\n". - 'In field "c": Expected "String!", found null.', + 'Variable "$input" got invalid value ' . + '{"a":"foo","b":"bar","c":null}; ' . + 'Expected non-nullable type String! at value.c.', 'locations' => [['line' => 2, 'column' => 17]], 'category' => 'graphql' ] ] ]; + $this->assertEquals($expected, $result->toArray()); // errors on incorrect type: @@ -174,8 +177,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value "foo bar".' . "\n" . - 'Expected "TestInputObject", found not an object.', + 'Variable "$input" got invalid value "foo bar"; ' . + 'Expected object type TestInputObject.', 'locations' => [ [ 'line' => 2, 'column' => 17 ] ], 'category' => 'graphql', ] @@ -191,8 +194,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value {"a":"foo","b":"bar"}.'. "\n". - 'In field "c": Expected "String!", found null.', + 'Variable "$input" got invalid value {"a":"foo","b":"bar"}; '. + 'Field value.c of required type String! was not provided.', 'locations' => [['line' => 2, 'column' => 17]], 'category' => 'graphql', ] @@ -214,9 +217,15 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value {"na":{"a":"foo"}}.' . "\n" . - 'In field "na": In field "c": Expected "String!", found null.' . "\n" . - 'In field "nb": Expected "String!", found null.', + 'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' . + 'Field value.na.c of required type String! was not provided.', + 'locations' => [['line' => 2, 'column' => 19]], + 'category' => 'graphql', + ], + [ + 'message' => + 'Variable "$input" got invalid value {"na":{"a":"foo"}}; ' . + 'Field value.nb of required type String! was not provided.', 'locations' => [['line' => 2, 'column' => 19]], 'category' => 'graphql', ] @@ -226,14 +235,15 @@ class VariablesTest extends \PHPUnit_Framework_TestCase // errors on addition of unknown input field - $params = ['input' => [ 'a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'd' => 'dog' ]]; + $params = ['input' => [ 'a' => 'foo', 'b' => 'bar', 'c' => 'baz', 'extra' => 'dog' ]]; $result = Executor::execute($schema, $ast, null, null, $params); $expected = [ 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value {"a":"foo","b":"bar","c":"baz","d":"dog"}.'."\n". - 'In field "d": Expected type "ComplexScalar", found "dog".', + 'Variable "$input" got invalid value ' . + '{"a":"foo","b":"bar","c":"baz","extra":"dog"}; ' . + 'Field "extra" is not defined by type TestInputObject.', 'locations' => [['line' => 2, 'column' => 17]], 'category' => 'graphql', ] @@ -401,8 +411,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$value" got invalid value null.' . "\n". - 'Expected "String!", found null.', + 'Variable "$value" got invalid value null; ' . + 'Expected non-nullable type String!.', 'locations' => [['line' => 2, 'column' => 31]], 'category' => 'graphql', ] @@ -482,8 +492,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase $expected = [ 'errors' => [[ 'message' => - 'Variable "$value" got invalid value [1,2,3].' . "\n" . - 'Expected type "String", found array(3).', + 'Variable "$value" got invalid value [1,2,3]; Expected type ' . + 'String; String cannot represent an array value: [1,2,3]', 'category' => 'graphql', 'locations' => [ ['line' => 2, 'column' => 31] @@ -491,7 +501,9 @@ class VariablesTest extends \PHPUnit_Framework_TestCase ]] ]; - $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, $variables)->toArray()); + $result = Executor::execute($this->schema(), $ast, null, null, $variables)->toArray(true); + + $this->assertEquals($expected, $result); } /** @@ -500,8 +512,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase public function testSerializingAnArrayViaGraphQLStringThrowsTypeError() { $this->setExpectedException( - InvariantViolation::class, - 'String cannot represent non scalar value: array(3)' + Error::class, + 'String cannot represent non scalar value: [1,2,3]' ); Type::string()->serialize([1, 2, 3]); } @@ -601,8 +613,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value null.' . "\n" . - 'Expected "[String]!", found null.', + 'Variable "$input" got invalid value null; ' . + 'Expected non-nullable type [String]!.', 'locations' => [['line' => 2, 'column' => 17]], 'category' => 'graphql', ] @@ -623,7 +635,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase '; $ast = Parser::parse($doc); $expected = ['data' => ['nnList' => '["A"]']]; - $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, ['input' => 'A'])->toArray()); + $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, ['input' => ['A']])->toArray()); } /** @@ -670,7 +682,7 @@ class VariablesTest extends \PHPUnit_Framework_TestCase $ast = Parser::parse($doc); $expected = ['data' => ['listNN' => '["A"]']]; - $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, ['input' => 'A'])->toArray()); + $this->assertEquals($expected, Executor::execute($this->schema(), $ast, null, null, ['input' => ['A']])->toArray()); } /** @@ -689,8 +701,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value ["A",null,"B"].' . "\n" . - 'In element #1: Expected "String!", found null.', + 'Variable "$input" got invalid value ["A",null,"B"]; ' . + 'Expected non-nullable type String! at value[1].', 'locations' => [ ['line' => 2, 'column' => 17] ], 'category' => 'graphql', ] @@ -715,8 +727,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value null.' . "\n" . - 'Expected "[String!]!", found null.', + 'Variable "$input" got invalid value null; ' . + 'Expected non-nullable type [String!]!.', 'locations' => [ ['line' => 2, 'column' => 17] ], 'category' => 'graphql', ] @@ -756,8 +768,8 @@ class VariablesTest extends \PHPUnit_Framework_TestCase 'errors' => [ [ 'message' => - 'Variable "$input" got invalid value ["A",null,"B"].'."\n". - 'In element #1: Expected "String!", found null.', + 'Variable "$input" got invalid value ["A",null,"B"]; ' . + 'Expected non-nullable type String! at value[1].', 'locations' => [ ['line' => 2, 'column' => 17] ], 'category' => 'graphql', ] diff --git a/tests/Server/QueryExecutionTest.php b/tests/Server/QueryExecutionTest.php index 00b26ea..487d089 100644 --- a/tests/Server/QueryExecutionTest.php +++ b/tests/Server/QueryExecutionTest.php @@ -322,7 +322,7 @@ class QueryExecutionTest extends TestCase $this->setExpectedException( InvariantViolation::class, 'Persistent query loader must return query string or instance of GraphQL\Language\AST\DocumentNode '. - 'but got: associative array(1) with first key: "err"' + 'but got: {"err":"err"}' ); $this->config->setPersistentQueryLoader(function($queryId, OperationParams $params) use (&$called) { return ['err' => 'err']; diff --git a/tests/Server/RequestValidationTest.php b/tests/Server/RequestValidationTest.php index 2a335f1..0d15a77 100644 --- a/tests/Server/RequestValidationTest.php +++ b/tests/Server/RequestValidationTest.php @@ -70,7 +70,7 @@ class RequestValidationTest extends \PHPUnit_Framework_TestCase $this->assertInputError( $parsedBody, - 'GraphQL Request parameter "query" must be string, but got object with first key: "t"' + 'GraphQL Request parameter "query" must be string, but got {"t":"{my query}"}' ); } @@ -82,7 +82,7 @@ class RequestValidationTest extends \PHPUnit_Framework_TestCase $this->assertInputError( $parsedBody, - 'GraphQL Request parameter "queryId" must be string, but got object with first key: "t"' + 'GraphQL Request parameter "queryId" must be string, but got {"t":"{my query}"}' ); } @@ -95,7 +95,7 @@ class RequestValidationTest extends \PHPUnit_Framework_TestCase $this->assertInputError( $parsedBody, - 'GraphQL Request parameter "operation" must be string, but got array(0)' + 'GraphQL Request parameter "operation" must be string, but got []' ); } diff --git a/tests/Type/EnumTypeTest.php b/tests/Type/EnumTypeTest.php index d278d7b..0761cb1 100644 --- a/tests/Type/EnumTypeTest.php +++ b/tests/Type/EnumTypeTest.php @@ -235,7 +235,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase '{ colorEnum(fromString: "GREEN") }', null, [ - 'message' => 'Expected a value of type "Color" but received: "GREEN"', + 'message' => 'Expected a value of type "Color" but received: GREEN', 'locations' => [new SourceLocation(1, 3)] ] ); @@ -325,7 +325,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase $this->expectFailure( 'query test($color: Color!) { colorEnum(fromEnum: $color) }', ['color' => 2], - "Variable \"\$color\" got invalid value 2.\nExpected type \"Color\", found 2." + 'Variable "$color" got invalid value 2; Expected type Color.' ); } @@ -459,7 +459,7 @@ class EnumTypeTest extends \PHPUnit_Framework_TestCase [ 'data' => ['first' => 'ONE', 'second' => 'TWO', 'third' => null], 'errors' => [[ - 'debugMessage' => 'Expected a value of type "SimpleEnum" but received: "WRONG"', + 'debugMessage' => 'Expected a value of type "SimpleEnum" but received: WRONG', 'locations' => [['line' => 4, 'column' => 13]] ]] ], diff --git a/tests/Type/ScalarSerializationTest.php b/tests/Type/ScalarSerializationTest.php index 3ded023..8fdd0d6 100644 --- a/tests/Type/ScalarSerializationTest.php +++ b/tests/Type/ScalarSerializationTest.php @@ -1,8 +1,7 @@ setExpectedException(InvariantViolation::class, 'Int cannot represent non-integer value: 0.1'); + $this->setExpectedException(Error::class, 'Int cannot represent non-integer value: 0.1'); $intType->serialize(0.1); } public function testSerializesOutputIntCannotRepresentFloat2() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non-integer value: 1.1'); + $this->setExpectedException(Error::class, 'Int cannot represent non-integer value: 1.1'); $intType->serialize(1.1); } @@ -46,7 +45,7 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputIntCannotRepresentNegativeFloat() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non-integer value: -1.1'); + $this->setExpectedException(Error::class, 'Int cannot represent non-integer value: -1.1'); $intType->serialize(-1.1); } @@ -54,7 +53,7 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputIntCannotRepresentNumericString() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, ''); + $this->setExpectedException(Error::class, ''); $intType->serialize('Int cannot represent non-integer value: "-1.1"'); } @@ -64,7 +63,7 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase // Maybe a safe PHP int, but bigger than 2^32, so not // representable as a GraphQL Int $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: 9876504321'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: 9876504321'); $intType->serialize(9876504321); } @@ -72,28 +71,28 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputIntCannotRepresentLowerThan32Bits() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: -9876504321'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: -9876504321'); $intType->serialize(-9876504321); } public function testSerializesOutputIntCannotRepresentBiggerThanSigned32Bits() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: 1.0E+100'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: 1.0E+100'); $intType->serialize(1e100); } public function testSerializesOutputIntCannotRepresentLowerThanSigned32Bits() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: -1.0E+100'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: -1.0E+100'); $intType->serialize(-1e100); } public function testSerializesOutputIntCannotRepresentString() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: "one"'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: one'); $intType->serialize('one'); } @@ -101,7 +100,7 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputIntCannotRepresentEmptyString() { $intType = Type::int(); - $this->setExpectedException(InvariantViolation::class, 'Int cannot represent non 32-bit signed integer value: (empty string)'); + $this->setExpectedException(Error::class, 'Int cannot represent non 32-bit signed integer value: (empty string)'); $intType->serialize(''); } @@ -127,14 +126,14 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputFloatCannotRepresentString() { $floatType = Type::float(); - $this->setExpectedException(InvariantViolation::class, 'Float cannot represent non numeric value: "one"'); + $this->setExpectedException(Error::class, 'Float cannot represent non numeric value: one'); $floatType->serialize('one'); } public function testSerializesOutputFloatCannotRepresentEmptyString() { $floatType = Type::float(); - $this->setExpectedException(InvariantViolation::class, 'Float cannot represent non numeric value: (empty string)'); + $this->setExpectedException(Error::class, 'Float cannot represent non numeric value: (empty string)'); $floatType->serialize(''); } @@ -156,14 +155,14 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputStringsCannotRepresentArray() { $stringType = Type::string(); - $this->setExpectedException(InvariantViolation::class, 'String cannot represent non scalar value: array(0)'); + $this->setExpectedException(Error::class, 'String cannot represent non scalar value: []'); $stringType->serialize([]); } public function testSerializesOutputStringsCannotRepresentObject() { $stringType = Type::string(); - $this->setExpectedException(InvariantViolation::class, 'String cannot represent non scalar value: instance of stdClass'); + $this->setExpectedException(Error::class, 'String cannot represent non scalar value: instance of stdClass'); $stringType->serialize(new \stdClass()); } @@ -202,7 +201,7 @@ class ScalarSerializationTest extends \PHPUnit_Framework_TestCase public function testSerializesOutputIDCannotRepresentObject() { $idType = Type::id(); - $this->setExpectedException(InvariantViolation::class, 'ID type cannot represent non scalar value: instance of stdClass'); + $this->setExpectedException(Error::class, 'ID type cannot represent non scalar value: instance of stdClass'); $idType->serialize(new \stdClass()); } } diff --git a/tests/Type/TypeLoaderTest.php b/tests/Type/TypeLoaderTest.php index 1848d7e..db9cb06 100644 --- a/tests/Type/TypeLoaderTest.php +++ b/tests/Type/TypeLoaderTest.php @@ -165,7 +165,7 @@ class TypeLoaderTest extends \PHPUnit_Framework_TestCase { $this->setExpectedException( InvariantViolation::class, - 'Schema type loader must be callable if provided but got: array(0)' + 'Schema type loader must be callable if provided but got: []' ); new Schema([ diff --git a/tests/Type/ValidationTest.php b/tests/Type/ValidationTest.php index 26626ad..373180d 100644 --- a/tests/Type/ValidationTest.php +++ b/tests/Type/ValidationTest.php @@ -1662,7 +1662,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase public function invalidEnumValueName() { return [ - ['#value', 'SomeEnum has value with invalid name: "#value" (Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "#value" does not.)'], + ['#value', 'SomeEnum has value with invalid name: #value (Names must match /^[_a-zA-Z][_a-zA-Z0-9]*$/ but "#value" does not.)'], ['true', 'SomeEnum: "true" can not be used as an Enum value.'], ['false', 'SomeEnum: "false" can not be used as an Enum value.'], ['null', 'SomeEnum: "null" can not be used as an Enum value.'], @@ -1776,7 +1776,7 @@ class ValidationTest extends \PHPUnit_Framework_TestCase $this->setExpectedException( InvariantViolation::class, - 'BadResolver.badField field resolver must be a function if provided, but got: array(0)' + 'BadResolver.badField field resolver must be a function if provided, but got: []' ); $schema->assertValid(); diff --git a/tests/Utils/CoerceValueTest.php b/tests/Utils/CoerceValueTest.php new file mode 100644 index 0000000..999ca89 --- /dev/null +++ b/tests/Utils/CoerceValueTest.php @@ -0,0 +1,201 @@ +expectError( + $result, + 'Expected type String; String cannot represent an array value: [1,2,3]' + ); + + $this->assertEquals( + 'String cannot represent an array value: [1,2,3]', + $result['errors'][0]->getPrevious()->getMessage() + ); + } + + // Describe: for GraphQLInt + + /** + * @it returns no error for int input + */ + public function testIntReturnsNoErrorForIntInput() + { + $result = Value::coerceValue('1', Type::int()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for negative int input + */ + public function testIntReturnsNoErrorForNegativeIntInput() + { + $result = Value::coerceValue('-1', Type::int()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for exponent input + */ + public function testIntReturnsNoErrorForExponentInput() + { + $result = Value::coerceValue('1e3', Type::int()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for null + */ + public function testIntReturnsASingleErrorNull() + { + $result = Value::coerceValue(null, Type::int()); + $this->expectNoErrors($result); + } + + /** + * @it returns a single error for empty value + */ + public function testIntReturnsASingleErrorForEmptyValue() + { + $result = Value::coerceValue('', Type::int()); + $this->expectError( + $result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: (empty string)' + ); + } + + /** + * @it returns error for float input as int + */ + public function testIntReturnsErrorForFloatInputAsInt() + { + $result = Value::coerceValue('1.5', Type::int()); + $this->expectError( + $result, + 'Expected type Int; Int cannot represent non-integer value: 1.5' + ); + } + + /** + * @it returns a single error for char input + */ + public function testIntReturnsASingleErrorForCharInput() + { + $result = Value::coerceValue('a', Type::int()); + $this->expectError( + $result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: a' + ); + } + + /** + * @it returns a single error for multi char input + */ + public function testIntReturnsASingleErrorForMultiCharInput() + { + $result = Value::coerceValue('meow', Type::int()); + $this->expectError( + $result, + 'Expected type Int; Int cannot represent non 32-bit signed integer value: meow' + ); + } + + // Describe: for GraphQLFloat + + /** + * @it returns no error for int input + */ + public function testFloatReturnsNoErrorForIntInput() + { + $result = Value::coerceValue('1', Type::float()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for exponent input + */ + public function testFloatReturnsNoErrorForExponentInput() + { + $result = Value::coerceValue('1e3', Type::float()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for float input + */ + public function testFloatReturnsNoErrorForFloatInput() + { + $result = Value::coerceValue('1.5', Type::float()); + $this->expectNoErrors($result); + } + + /** + * @it returns no error for null + */ + public function testFloatReturnsASingleErrorNull() + { + $result = Value::coerceValue(null, Type::float()); + $this->expectNoErrors($result); + } + + /** + * @it returns a single error for empty value + */ + public function testFloatReturnsASingleErrorForEmptyValue() + { + $result = Value::coerceValue('', Type::float()); + $this->expectError( + $result, + 'Expected type Float; Float cannot represent non numeric value: (empty string)' + ); + } + + /** + * @it returns a single error for char input + */ + public function testFloatReturnsASingleErrorForCharInput() + { + $result = Value::coerceValue('a', Type::float()); + $this->expectError( + $result, + 'Expected type Float; Float cannot represent non numeric value: a' + ); + } + + /** + * @it returns a single error for multi char input + */ + public function testFloatReturnsASingleErrorForMultiCharInput() + { + $result = Value::coerceValue('meow', Type::float()); + $this->expectError( + $result, + 'Expected type Float; Float cannot represent non numeric value: meow' + ); + } + + private function expectNoErrors($result) + { + $this->assertInternalType('array', $result); + $this->assertNull($result['errors']); + } + + private function expectError($result, $expected) { + $this->assertInternalType('array', $result); + $this->assertInternalType('array', $result['errors']); + $this->assertCount(1, $result['errors']); + $this->assertEquals($expected, $result['errors'][0]->getMessage()); + } +} diff --git a/tests/Utils/IsValidPHPValueTest.php b/tests/Utils/IsValidPHPValueTest.php deleted file mode 100644 index 937fa4f..0000000 --- a/tests/Utils/IsValidPHPValueTest.php +++ /dev/null @@ -1,132 +0,0 @@ -expectNoErrors($result); - - // returns no error for negative int value - $result = Values::isValidPHPValue(-1, Type::int()); - $this->expectNoErrors($result); - - // returns no error for null value - $result = Values::isValidPHPValue(null, Type::int()); - $this->expectNoErrors($result); - - // returns a single error for positive int string value - $result = Values::isValidPHPValue('1', Type::int()); - $this->expectErrorResult($result, 1); - - // returns a single error for negative int string value - $result = Values::isValidPHPValue('-1', Type::int()); - $this->expectErrorResult($result, 1); - - // returns errors for exponential int string value - $result = Values::isValidPHPValue('1e3', Type::int()); - $this->expectErrorResult($result, 1); - $result = Values::isValidPHPValue('0e3', Type::int()); - $this->expectErrorResult($result, 1); - - // returns a single error for empty value - $result = Values::isValidPHPValue('', Type::int()); - $this->expectErrorResult($result, 1); - - // returns error for float value - $result = Values::isValidPHPValue(1.5, Type::int()); - $this->expectErrorResult($result, 1); - $result = Values::isValidPHPValue(1e3, Type::int()); - $this->expectErrorResult($result, 1); - - // returns error for float string value - $result = Values::isValidPHPValue('1.5', Type::int()); - $this->expectErrorResult($result, 1); - - // returns a single error for char input - $result = Values::isValidPHPValue('a', Type::int()); - $this->expectErrorResult($result, 1); - - // returns a single error for char input - $result = Values::isValidPHPValue('meow', Type::int()); - $this->expectErrorResult($result, 1); - } - - public function testValidFloatValue() - { - // returns no error for positive float value - $result = Values::isValidPHPValue(1.2, Type::float()); - $this->expectNoErrors($result); - - // returns no error for exponential float value - $result = Values::isValidPHPValue(1e3, Type::float()); - $this->expectNoErrors($result); - - // returns no error for negative float value - $result = Values::isValidPHPValue(-1.2, Type::float()); - $this->expectNoErrors($result); - - // returns no error for a positive int value - $result = Values::isValidPHPValue(1, Type::float()); - $this->expectNoErrors($result); - - // returns no errors for a negative int value - $result = Values::isValidPHPValue(-1, Type::float()); - $this->expectNoErrors($result); - - // returns no error for null value: - $result = Values::isValidPHPValue(null, Type::float()); - $this->expectNoErrors($result); - - // returns error for positive float string value - $result = Values::isValidPHPValue('1.2', Type::float()); - $this->expectErrorResult($result, 1); - - // returns error for negative float string value - $result = Values::isValidPHPValue('-1.2', Type::float()); - $this->expectErrorResult($result, 1); - - // returns error for a positive int string value - $result = Values::isValidPHPValue('1', Type::float()); - $this->expectErrorResult($result, 1); - - // returns errors for a negative int string value - $result = Values::isValidPHPValue('-1', Type::float()); - $this->expectErrorResult($result, 1); - - // returns error for exponent input - $result = Values::isValidPHPValue('1e3', Type::float()); - $this->expectErrorResult($result, 1); - $result = Values::isValidPHPValue('0e3', Type::float()); - $this->expectErrorResult($result, 1); - - // returns a single error for empty value - $result = Values::isValidPHPValue('', Type::float()); - $this->expectErrorResult($result, 1); - - // returns a single error for char input - $result = Values::isValidPHPValue('a', Type::float()); - $this->expectErrorResult($result, 1); - - // returns a single error for char input - $result = Values::isValidPHPValue('meow', Type::float()); - $this->expectErrorResult($result, 1); - } - - private function expectNoErrors($result) - { - $this->assertInternalType('array', $result); - $this->assertEquals([], $result); - } - - private function expectErrorResult($result, $size) { - $this->assertInternalType('array', $result); - $this->assertEquals($size, count($result)); - } -} diff --git a/tools/gendocs.php b/tools/gendocs.php index 95b5b96..d28b0c9 100644 --- a/tools/gendocs.php +++ b/tools/gendocs.php @@ -41,7 +41,7 @@ function renderClassMethod(ReflectionMethod $method) { if ($p->isDefaultValueAvailable()) { $val = $p->isDefaultValueConstant() ? $p->getDefaultValueConstantName() : $p->getDefaultValue(); - $def .= " = " . (is_array($val) ? '[]' : Utils::printSafe($val)); + $def .= " = " . Utils::printSafeJson($val); } return $def; @@ -63,7 +63,7 @@ TEMPLATE; } function renderConstant($value, $name) { - return "const $name = " . Utils::printSafe($value) . ";"; + return "const $name = " . Utils::printSafeJson($value) . ";"; } function renderProp(ReflectionProperty $prop) {