Fix parsing of default values in build-schema

* Generalizes building a value from an AST, since "scalar" could be misleading, and supporting variable values within custom scalar literals can be valuable.
* Replaces isNullish with isInvalid since `null` is a meaningful value as a result of literal parsing.
* Provide reasonable default version of 'parseLiteral'

ref: 714ee980aa
ref: https://github.com/graphql/graphql-js/pull/903

# Conflicts:
#	src/Utils/BuildSchema.php
#	tests/Utils/BuildSchemaTest.php
This commit is contained in:
Daniel Tschinder 2018-02-09 11:26:22 +01:00
parent 2123946dbd
commit 27ce24b5fe
20 changed files with 333 additions and 130 deletions

View File

@ -42,20 +42,21 @@ class UrlType extends ScalarType
/**
* Parses an externally provided literal value to use as an input (e.g. in Query AST)
*
* @param $ast Node
* @param Node $valueNode
* @param array|null $variables
* @return null|string
* @throws Error
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
// Note: throwing GraphQL\Error\Error vs \UnexpectedValueException to benefit from GraphQL
// error location in query:
if (!($ast instanceof StringValueNode)) {
throw new Error('Query error: Can only parse strings got: ' . $ast->kind, [$ast]);
if (!($valueNode instanceof StringValueNode)) {
throw new Error('Query error: Can only parse strings got: ' . $valueNode->kind, [$valueNode]);
}
if (!is_string($ast->value) || !filter_var($ast->value, FILTER_VALIDATE_URL)) {
throw new Error('Query error: Not a valid URL', [$ast]);
if (!is_string($valueNode->value) || !filter_var($valueNode->value, FILTER_VALIDATE_URL)) {
throw new Error('Query error: Not a valid URL', [$valueNode]);
}
return $ast->value;
return $valueNode->value;
}
}

View File

@ -1,9 +1,7 @@
<?php
namespace GraphQL\Executor;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\ArgumentNode;
use GraphQL\Language\AST\DirectiveNode;
use GraphQL\Language\AST\EnumValueDefinitionNode;
@ -15,12 +13,13 @@ use GraphQL\Language\AST\NodeList;
use GraphQL\Language\AST\VariableNode;
use GraphQL\Language\AST\VariableDefinitionNode;
use GraphQL\Language\Printer;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\ScalarType;
use GraphQL\Type\Schema;
use GraphQL\Type\Definition\Directive;
use GraphQL\Type\Definition\FieldDefinition;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\InputType;
use GraphQL\Type\Definition\LeafType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull;
use GraphQL\Type\Definition\Type;
@ -274,18 +273,19 @@ class Values
return $errors;
}
if ($type instanceof LeafType) {
Utils::invariant($type instanceof EnumType || $type instanceof ScalarType, 'Must be input type');
try {
// Scalar/Enum input checks to ensure the type can parse the value to
// a non-null value.
$parseResult = $type->parseValue($value);
if (null === $parseResult && !$type->isValidValue($value)) {
if (!$type->isValidValue($value)) {
$v = Utils::printSafeJson($value);
return [
"Expected type \"{$type->name}\", found $v."
];
}
return [];
} catch (\Exception $e) {
return [
"Expected type \"{$type->name}\", found " . Utils::printSafeJson($value) . ': ' .
@ -297,9 +297,9 @@ class Values
$e->getMessage()
];
}
}
throw new InvariantViolation('Must be input type');
return [];
}
/**
@ -370,16 +370,12 @@ class Values
return $coercedObj;
}
if ($type instanceof LeafType) {
$parsed = $type->parseValue($value);
if (null === $parsed) {
// null or invalid values represent a failure to parse correctly,
// in which case no value is returned.
return $undefined;
}
return $parsed;
Utils::invariant($type instanceof EnumType || $type instanceof ScalarType, 'Must be input type');
if ($type->isValidValue($value)) {
return $type->parseValue($value);
}
throw new InvariantViolation('Must be input type');
return $undefined;
}
}

View File

@ -7,7 +7,7 @@ class ListValueNode extends Node implements ValueNode
public $kind = NodeKind::LST;
/**
* @var ValueNode[]
* @var ValueNode[]|NodeList
*/
public $values;
}

View File

@ -6,7 +6,7 @@ class ObjectValueNode extends Node implements ValueNode
public $kind = NodeKind::OBJECT;
/**
* @var ObjectFieldNode[]
* @var ObjectFieldNode[]|NodeList
*/
public $fields;
}

View File

@ -2,6 +2,7 @@
namespace GraphQL\Type\Definition;
use GraphQL\Language\AST\BooleanValueNode;
use GraphQL\Utils\Utils;
/**
* Class BooleanType
@ -34,18 +35,19 @@ class BooleanType extends ScalarType
*/
public function parseValue($value)
{
return is_bool($value) ? $value : null;
return is_bool($value) ? $value : Utils::undefined();
}
/**
* @param $ast
* @param $valueNode
* @param array|null $variables
* @return bool|null
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
if ($ast instanceof BooleanValueNode) {
return (bool) $ast->value;
if ($valueNode instanceof BooleanValueNode) {
return (bool) $valueNode->value;
}
return null;
return Utils::undefined();
}
}

View File

@ -1,6 +1,7 @@
<?php
namespace GraphQL\Type\Definition;
use GraphQL\Utils\AST;
use GraphQL\Utils\Utils;
/**
@ -24,23 +25,28 @@ class CustomScalarType extends ScalarType
*/
public function parseValue($value)
{
if (Utils::isInvalid($value)) {
return Utils::undefined();
}
if (isset($this->config['parseValue'])) {
return call_user_func($this->config['parseValue'], $value);
} else {
return null;
return $value;
}
}
/**
* @param $valueNode
* @param array|null $variables
* @return mixed
*/
public function parseLiteral(/* GraphQL\Language\AST\ValueNode */ $valueNode)
public function parseLiteral(/* GraphQL\Language\AST\ValueNode */ $valueNode, array $variables = null)
{
if (isset($this->config['parseLiteral'])) {
return call_user_func($this->config['parseLiteral'], $valueNode);
return call_user_func($this->config['parseLiteral'], $valueNode, $variables);
} else {
return null;
return AST::valueFromASTUntyped($valueNode, $variables);
}
}

View File

@ -122,9 +122,10 @@ class EnumType extends Type implements InputType, OutputType, LeafType
/**
* @param $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode)
public function isValidLiteral($valueNode, array $variables = null)
{
return $valueNode instanceof EnumValueNode && $this->getNameLookup()->offsetExists($valueNode->value);
}
@ -136,14 +137,15 @@ class EnumType extends Type implements InputType, OutputType, LeafType
public function parseValue($value)
{
$lookup = $this->getNameLookup();
return isset($lookup[$value]) ? $lookup[$value]->value : null;
return isset($lookup[$value]) ? $lookup[$value]->value : Utils::undefined();
}
/**
* @param $value
* @param array|null $variables
* @return null
*/
public function parseLiteral($value)
public function parseLiteral($value, array $variables = null)
{
if ($value instanceof EnumValueNode) {
$lookup = $this->getNameLookup();

View File

@ -1,7 +1,6 @@
<?php
namespace GraphQL\Type\Definition;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\FloatValueNode;
use GraphQL\Language\AST\IntValueNode;
@ -50,18 +49,19 @@ values as specified by
*/
public function parseValue($value)
{
return (is_numeric($value) && !is_string($value)) ? (float) $value : null;
return (is_numeric($value) && !is_string($value)) ? (float) $value : Utils::undefined();
}
/**
* @param $ast
* @param $valueNode
* @param array|null $variables
* @return float|null
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
if ($ast instanceof FloatValueNode || $ast instanceof IntValueNode) {
return (float) $ast->value;
if ($valueNode instanceof FloatValueNode || $valueNode instanceof IntValueNode) {
return (float) $valueNode->value;
}
return null;
return Utils::undefined();
}
}

View File

@ -1,7 +1,6 @@
<?php
namespace GraphQL\Type\Definition;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\IntValueNode;
use GraphQL\Language\AST\StringValueNode;
@ -55,18 +54,19 @@ When expected as an input type, any string (such as `"4"`) or integer
*/
public function parseValue($value)
{
return (is_string($value) || is_int($value)) ? (string) $value : null;
return (is_string($value) || is_int($value)) ? (string) $value : Utils::undefined();
}
/**
* @param $ast
* @param array|null $variables
* @return null|string
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
if ($ast instanceof StringValueNode || $ast instanceof IntValueNode) {
return $ast->value;
if ($valueNode instanceof StringValueNode || $valueNode instanceof IntValueNode) {
return $valueNode->value;
}
return null;
return Utils::undefined();
}
}

View File

@ -1,7 +1,6 @@
<?php
namespace GraphQL\Type\Definition;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\IntValueNode;
use GraphQL\Utils\Utils;
@ -77,21 +76,22 @@ values. Int can represent values between -(2^31) and 2^31 - 1. ';
// 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 : null;
return $isInt && $value <= self::MAX_INT && $value >= self::MIN_INT ? $value : Utils::undefined();
}
/**
* @param $ast
* @param $valueNode
* @param array|null $variables
* @return int|null
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
if ($ast instanceof IntValueNode) {
$val = (int) $ast->value;
if ($ast->value === (string) $val && self::MIN_INT <= $val && $val <= self::MAX_INT) {
if ($valueNode instanceof IntValueNode) {
$val = (int) $valueNode->value;
if ($valueNode->value === (string) $val && self::MIN_INT <= $val && $val <= self::MAX_INT) {
return $val;
}
}
return null;
return Utils::undefined();
}
}

View File

@ -1,6 +1,8 @@
<?php
namespace GraphQL\Type\Definition;
use \GraphQL\Language\AST\Node;
/*
export type GraphQLLeafType =
GraphQLScalarType |
@ -19,6 +21,8 @@ interface LeafType
/**
* Parses an externally provided value (query variable) to use as an input
*
* In the case of an invalid value this method must return Utils::undefined()
*
* @param mixed $value
* @return mixed
*/
@ -27,10 +31,13 @@ interface LeafType
/**
* Parses an externally provided literal value (hardcoded in GraphQL query) to use as an input
*
* @param \GraphQL\Language\AST\Node $valueNode
* In the case of an invalid value this method must return Utils::undefined()
*
* @param Node $valueNode
* @param array|null $variables
* @return mixed
*/
public function parseLiteral($valueNode);
public function parseLiteral($valueNode, array $variables = null);
/**
* @param string $value
@ -39,8 +46,9 @@ interface LeafType
public function isValidValue($value);
/**
* @param \GraphQL\Language\AST\Node $valueNode
* @return mixed
* @param Node $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode);
public function isValidLiteral($valueNode, array $variables = null);
}

View File

@ -41,14 +41,13 @@ abstract class ScalarType extends Type implements OutputType, InputType, LeafTyp
/**
* Determines if an internal value is valid for this type.
* Equivalent to checking for if the parsedValue is nullish.
*
* @param $value
* @return bool
*/
public function isValidValue($value)
{
return null !== $this->parseValue($value);
return !Utils::isInvalid($this->parseValue($value));
}
/**
@ -56,10 +55,11 @@ abstract class ScalarType extends Type implements OutputType, InputType, LeafTyp
* Equivalent to checking for if the parsedLiteral is nullish.
*
* @param $valueNode
* @param array|null $variables
* @return bool
*/
public function isValidLiteral($valueNode)
public function isValidLiteral($valueNode, array $variables = null)
{
return null !== $this->parseLiteral($valueNode);
return !Utils::isInvalid($this->parseLiteral($valueNode, $variables));
}
}

View File

@ -1,7 +1,6 @@
<?php
namespace GraphQL\Type\Definition;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\StringValueNode;
use GraphQL\Utils\Utils;
@ -52,18 +51,19 @@ represent free-form human-readable text.';
*/
public function parseValue($value)
{
return is_string($value) ? $value : null;
return is_string($value) ? $value : Utils::undefined();
}
/**
* @param $ast
* @param $valueNode
* @param array|null $variables
* @return null|string
*/
public function parseLiteral($ast)
public function parseLiteral($valueNode, array $variables = null)
{
if ($ast instanceof StringValueNode) {
return $ast->value;
if ($valueNode instanceof StringValueNode) {
return $valueNode->value;
}
return null;
return Utils::undefined();
}
}

View File

@ -30,9 +30,9 @@ use GraphQL\Type\Definition\InputType;
use GraphQL\Type\Definition\LeafType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull;
use GraphQL\Type\Definition\ScalarType;
use GraphQL\Type\Definition\Type;
use GraphQL\Type\Schema;
use GraphQL\Utils\Utils;
/**
* Various utilities dealing with AST
@ -383,19 +383,77 @@ class AST
return $coercedObj;
}
if ($type instanceof LeafType) {
$parsed = $type->parseLiteral($valueNode);
if (!$type instanceof ScalarType && !$type instanceof EnumType) {
throw new InvariantViolation('Must be input type');
}
if ($type->isValidLiteral($valueNode, $variables)) {
return $type->parseLiteral($valueNode, $variables);
}
if (null === $parsed && !$type->isValidLiteral($valueNode)) {
// Invalid values represent a failure to parse correctly, in which case
// no value is returned.
return $undefined;
}
return $parsed;
/**
* Produces a PHP value given a GraphQL Value AST.
*
* Unlike `valueFromAST()`, no type is provided. The resulting JavaScript value
* will reflect the provided GraphQL value AST.
*
* | GraphQL Value | PHP Value |
* | -------------------- | ------------- |
* | Input Object | Assoc Array |
* | List | Array |
* | Boolean | Boolean |
* | String | String |
* | Int / Float | Int / Float |
* | Enum | Mixed |
* | Null | null |
*
* @api
* @param Node $valueNode
* @param array|null $variables
* @return mixed
* @throws \Exception
*/
public static function valueFromASTUntyped($valueNode, array $variables = null) {
switch (true) {
case $valueNode instanceof NullValueNode:
return null;
case $valueNode instanceof IntValueNode:
return intval($valueNode->value, 10);
case $valueNode instanceof FloatValueNode:
return floatval($valueNode->value);
case $valueNode instanceof StringValueNode:
case $valueNode instanceof EnumValueNode:
case $valueNode instanceof BooleanValueNode:
return $valueNode->value;
case $valueNode instanceof ListValueNode:
return array_map(
function($node) use ($variables) {
return self::valueFromASTUntyped($node, $variables);
},
iterator_to_array($valueNode->values)
);
case $valueNode instanceof ObjectValueNode:
return array_combine(
array_map(
function($field) { return $field->name->value; },
iterator_to_array($valueNode->fields)
),
array_map(
function($field) use ($variables) { return self::valueFromASTUntyped($field->value, $variables); },
iterator_to_array($valueNode->fields)
)
);
case $valueNode instanceof VariableNode:
$variableName = $valueNode->name->value;
return ($variables && isset($variables[$variableName]) && !Utils::isInvalid($variables[$variableName]))
? $variables[$variableName]
: null;
default:
throw new InvariantViolation('Unexpected value kind: ' . $valueNode->kind);
}
throw new InvariantViolation('Must be input type');
}
/**

View File

@ -539,19 +539,9 @@ class BuildSchema
'name' => $def->name->value,
'description' => $this->getDescription($def),
'astNode' => $def,
'serialize' => function () {
return false;
'serialize' => function($value) {
return $value;
},
// Note: validation calls the parse functions to determine if a
// literal value is correct. Returning null would cause use of custom
// scalars to always fail validation. Returning false causes them to
// always pass validation.
'parseValue' => function () {
return false;
},
'parseLiteral' => function () {
return false;
}
];
}

View File

@ -15,12 +15,23 @@ class Utils
return $undefined ?: $undefined = new \stdClass();
}
/**
* Check if the value is invalid
*
* @param mixed $value
* @return bool
*/
public static function isInvalid($value)
{
return self::undefined() === $value;
}
/**
* @param object $obj
* @param array $vars
* @param array $requiredKeys
*
* @return array
* @return object
*/
public static function assign($obj, array $vars, array $requiredKeys = [])
{

View File

@ -2,7 +2,6 @@
namespace GraphQL\Validator;
use GraphQL\Error\Error;
use GraphQL\Error\InvariantViolation;
use GraphQL\Language\AST\ListValueNode;
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Language\AST\NodeKind;
@ -11,11 +10,12 @@ use GraphQL\Language\AST\VariableNode;
use GraphQL\Language\Printer;
use GraphQL\Language\Visitor;
use GraphQL\Type\Schema;
use GraphQL\Type\Definition\EnumType;
use GraphQL\Type\Definition\InputObjectType;
use GraphQL\Type\Definition\LeafType;
use GraphQL\Type\Definition\ListOfType;
use GraphQL\Type\Definition\NonNull;
use GraphQL\Type\Definition\Type;
use GraphQL\Type\Definition\ScalarType;
use GraphQL\Utils\Utils;
use GraphQL\Utils\TypeInfo;
use GraphQL\Validator\Rules\AbstractValidationRule;
@ -306,8 +306,9 @@ class DocumentValidator
return $errors;
}
if ($type instanceof LeafType) {
// Scalars must parse to a non-null value
Utils::invariant($type instanceof ScalarType || $type instanceof EnumType, 'Must be input type');
// Scalars determine if a literal values is valid.
if (!$type->isValidLiteral($valueNode)) {
$printed = Printer::doPrint($valueNode);
return [ "Expected type \"{$type->name}\", found $printed." ];
@ -316,9 +317,6 @@ class DocumentValidator
return [];
}
throw new InvariantViolation('Must be input type');
}
/**
* This uses a specialized visitor which runs multiple visitors in parallel,
* while maintaining the visitor skip and break API.

View File

@ -2,6 +2,7 @@
namespace GraphQL\Tests\Executor;
use GraphQL\Type\Definition\ScalarType;
use GraphQL\Utils\Utils;
class Dog
{
@ -65,15 +66,15 @@ class ComplexScalar extends ScalarType
if ($value === 'SerializedValue') {
return 'DeserializedValue';
}
return null;
return Utils::undefined();
}
public function parseLiteral($valueNode)
public function parseLiteral($valueNode, array $variables = null)
{
if ($valueNode->value === 'SerializedValue') {
return 'DeserializedValue';
}
return null;
return Utils::undefined();
}
}

View File

@ -0,0 +1,110 @@
<?php
namespace GraphQL\Tests\Utils;
use GraphQL\Language\Parser;
use GraphQL\Utils\AST;
class ASTFromValueUntypedTest extends \PHPUnit_Framework_TestCase
{
// Describe: valueFromASTUntyped
private function assertTestCase($valueText, $expected, array $variables = null) {
$this->assertEquals(
$expected,
AST::valueFromASTUntyped(Parser::parseValue($valueText), $variables)
);
}
/**
* @it parses simple values
*/
public function testParsesSimpleValues()
{
$this->assertTestCase('null', null);
$this->assertTestCase('true', true);
$this->assertTestCase('false', false);
$this->assertTestCase('123', 123);
$this->assertTestCase('123.456', 123.456);
$this->assertTestCase('abc123', 'abc123');
}
/**
* @it parses lists of values
*/
public function testParsesListsOfValues()
{
$this->assertTestCase('[true, false]', [true, false]);
$this->assertTestCase('[true, 123.45]', [true, 123.45]);
$this->assertTestCase('[true, null]', [true, null]);
$this->assertTestCase('[true, ["foo", 1.2]]', [true, ['foo', 1.2]]);
}
/**
* @it parses input objects
*/
public function testParsesInputObjects()
{
$this->assertTestCase(
'{ int: 123, bool: false }',
['int' => 123, 'bool' => false]
);
$this->assertTestCase(
'{ foo: [ { bar: "baz"} ] }',
['foo' => [['bar' => 'baz']]]
);
}
/**
* @it parses enum values as plain strings
*/
public function testParsesEnumValuesAsPlainStrings()
{
$this->assertTestCase(
'TEST_ENUM_VALUE',
'TEST_ENUM_VALUE'
);
$this->assertTestCase(
'[TEST_ENUM_VALUE]',
['TEST_ENUM_VALUE']
);
}
/**
* @it parses enum values as plain strings
*/
public function testParsesVariables()
{
$this->assertTestCase(
'$testVariable',
'foo',
['testVariable' => 'foo']
);
$this->assertTestCase(
'[$testVariable]',
['foo'],
['testVariable' => 'foo']
);
$this->assertTestCase(
'{a:[$testVariable]}',
['a' => ['foo']],
['testVariable' => 'foo']
);
$this->assertTestCase(
'$testVariable',
null,
['testVariable' => null]
);
$this->assertTestCase(
'$testVariable',
null,
[]
);
$this->assertTestCase(
'$testVariable',
null,
null
);
}
}

View File

@ -636,6 +636,26 @@ type Hello {
$this->assertEquals($output, $body);
}
/**
* @it Custom scalar argument field with default
*/
public function testCustomScalarArgumentFieldWithDefault()
{
$body = '
schema {
query: Hello
}
scalar CustomScalar
type Hello {
str(int: CustomScalar = 2): String
}
';
$output = $this->cycleOutput($body);
$this->assertEquals($output, $body);
}
/**
* @it Simple type with mutation
*/