Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Update query variable coercion to meet the rules outlined in the specification. #171

Merged
merged 1 commit into from
Sep 20, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/Type/Definition/BooleanType.php
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function serialize($value)
*/
public function parseValue($value)
{
return !!$value;
return is_bool($value) ? $value : null;
}

/**
Expand Down
35 changes: 11 additions & 24 deletions src/Type/Definition/FloatType.php
Original file line number Diff line number Diff line change
Expand Up @@ -31,25 +31,6 @@ class FloatType extends ScalarType
* @return float|null
*/
public function serialize($value)
{
return $this->coerceFloat($value, false);
}

/**
* @param mixed $value
* @return float|null
*/
public function parseValue($value)
{
return $this->coerceFloat($value, true);
}

/**
* @param mixed $value
* @param bool $isInput
* @return float|null
*/
private function coerceFloat($value, $isInput)
{
if (is_numeric($value) || $value === true || $value === false) {
return (float) $value;
Expand All @@ -58,12 +39,18 @@ private function coerceFloat($value, $isInput)
if ($value === '') {
$err = 'Float cannot represent non numeric value: (empty string)';
} else {
$err = sprintf(
'Float cannot represent non numeric value: %s',
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
);
$err = sprintf('Float cannot represent non numeric value: %s', Utils::printSafe($value));
}
throw ($isInput ? new Error($err) : new InvariantViolation($err));
throw new InvariantViolation($err);
}

/**
* @param mixed $value
* @return float|null
*/
public function parseValue($value)
{
return (is_numeric($value) && !is_string($value)) ? (float) $value : null;
}

/**
Expand Down
11 changes: 1 addition & 10 deletions src/Type/Definition/IDType.php
Original file line number Diff line number Diff line change
Expand Up @@ -55,19 +55,10 @@ public function serialize($value)
*/
public function parseValue($value)
{
if ($value === true) {
return 'true';
}
if ($value === false) {
return 'false';
}
if ($value === null) {
return 'null';
}
if (!is_scalar($value)) {
throw new Error("ID type cannot represent non scalar value: " . Utils::printSafeJson($value));
}
return (string) $value;
return (is_string($value) || is_int($value)) ? (string) $value : null;
}

/**
Expand Down
42 changes: 14 additions & 28 deletions src/Type/Definition/IntType.php
Original file line number Diff line number Diff line change
Expand Up @@ -38,39 +38,16 @@ class IntType extends ScalarType
*/
public function serialize($value)
{
return $this->coerceInt($value, false);
}

/**
* @param mixed $value
* @return int|null
*/
public function parseValue($value)
{
return $this->coerceInt($value, true);
}

/**
* @param $value
* @param bool $isInput
* @return int|null
*/
private function coerceInt($value, $isInput)
{
$errClass = $isInput ? Error::class : InvariantViolation::class;

if ($value === '') {
throw new $errClass(
'Int cannot represent non 32-bit signed integer value: (empty string)'
);
throw new InvariantViolation('Int cannot represent non 32-bit signed integer value: (empty string)');
}
if (false === $value || true === $value) {
return (int) $value;
}
if (!is_numeric($value) || $value > self::MAX_INT || $value < self::MIN_INT) {
throw new $errClass(sprintf(
throw new InvariantViolation(sprintf(
'Int cannot represent non 32-bit signed integer value: %s',
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
Utils::printSafe($value)
));
}
$num = (float) $value;
Expand All @@ -82,15 +59,24 @@ private function coerceInt($value, $isInput)
// 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 $errClass(sprintf(
throw new InvariantViolation(sprintf(
'Int cannot represent non-integer value: %s',
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
Utils::printSafe($value)
));
}
}
return (int) $value;
}

/**
* @param mixed $value
* @return int|null
*/
public function parseValue($value)
{
return (is_int($value) && $value <= self::MAX_INT && $value >= self::MIN_INT) ? $value : null;
}

/**
* @param $ast
* @return int|null
Expand Down
11 changes: 1 addition & 10 deletions src/Type/Definition/StringType.php
Original file line number Diff line number Diff line change
Expand Up @@ -52,19 +52,10 @@ public function serialize($value)
*/
public function parseValue($value)
{
if ($value === true) {
return 'true';
}
if ($value === false) {
return 'false';
}
if ($value === null) {
return 'null';
}
if (!is_scalar($value)) {
throw new Error("String cannot represent non scalar value: " . Utils::printSafe($value));
}
return (string) $value;
return is_string($value) ? $value : null;
}

/**
Expand Down
2 changes: 1 addition & 1 deletion tests/Executor/ExecutorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ public function testProvidesInfoAboutCurrentExecutionState()

$rootValue = [ 'root' => 'val' ];

Executor::execute($schema, $ast, $rootValue, null, [ 'var' => 123 ]);
Executor::execute($schema, $ast, $rootValue, null, [ 'var' => '123' ]);

$this->assertEquals([
'fieldName',
Expand Down
208 changes: 208 additions & 0 deletions tests/Executor/ValuesTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,208 @@
<?php
namespace GraphQL\Tests\Executor;

use GraphQL\Executor\Values;
use GraphQL\Language\AST\NamedTypeNode;
use GraphQL\Language\AST\NameNode;
use GraphQL\Language\AST\VariableDefinitionNode;
use GraphQL\Language\AST\VariableNode;
use GraphQL\Type\Definition\ObjectType;
use GraphQL\Type\Definition\Type;
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 testGetBooleanVariableValues()
{
$this->expectInputVariablesMatchOutputVariables(['boolInput' => true]);
$this->expectInputVariablesMatchOutputVariables(['boolInput' => false]);
}

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]);
}

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 testBooleanForIDVariableThrowsError()
{
$this->expectGraphQLError(['idInput' => true]);
}

public function testFloatForIDVariableThrowsError()
{
$this->expectGraphQLError(['idInput' => 1.0]);
}

public function testStringForBooleanVariableThrowsError()
{
$this->expectGraphQLError(['boolInput' => 'true']);
}

public function testIntForBooleanVariableThrowsError()
{
$this->expectGraphQLError(['boolInput' => 1]);
}

public function testFloatForBooleanVariableThrowsError()
{
$this->expectGraphQLError(['boolInput' => 1.0]);
}

public function testBooleanForIntVariableThrowsError()
{
$this->expectGraphQLError(['intInput' => true]);
}

public function testStringForIntVariableThrowsError()
{
$this->expectGraphQLError(['intInput' => 'true']);
}

public function testFloatForIntVariableThrowsError()
{
$this->expectGraphQLError(['intInput' => 1.0]);
}

public function testPositiveBigIntForIntVariableThrowsError()
{
$this->expectGraphQLError(['intInput' => 2147483648]);
}

public function testNegativeBigIntForIntVariableThrowsError()
{
$this->expectGraphQLError(['intInput' => -2147483649]);
}

public function testBooleanForStringVariableThrowsError()
{
$this->expectGraphQLError(['stringInput' => true]);
}

public function testIntForStringVariableThrowsError()
{
$this->expectGraphQLError(['stringInput' => 1]);
}

public function testFloatForStringVariableThrowsError()
{
$this->expectGraphQLError(['stringInput' => 1.0]);
}

public function testBooleanForFloatVariableThrowsError()
{
$this->expectGraphQLError(['floatInput' => true]);
}

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);
}
}
Loading