Skip to content

Commit 537dbab

Browse files
authored
Merge pull request #171 from dereklavigne18/query_variable_coercion
Update query variable coercion to meet the rules outlined in the specification.
2 parents 7cc863d + d22385c commit 537dbab

File tree

8 files changed

+298
-93
lines changed

8 files changed

+298
-93
lines changed

src/Type/Definition/BooleanType.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ public function serialize($value)
3434
*/
3535
public function parseValue($value)
3636
{
37-
return !!$value;
37+
return is_bool($value) ? $value : null;
3838
}
3939

4040
/**

src/Type/Definition/FloatType.php

Lines changed: 11 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,6 @@ class FloatType extends ScalarType
3131
* @return float|null
3232
*/
3333
public function serialize($value)
34-
{
35-
return $this->coerceFloat($value, false);
36-
}
37-
38-
/**
39-
* @param mixed $value
40-
* @return float|null
41-
*/
42-
public function parseValue($value)
43-
{
44-
return $this->coerceFloat($value, true);
45-
}
46-
47-
/**
48-
* @param mixed $value
49-
* @param bool $isInput
50-
* @return float|null
51-
*/
52-
private function coerceFloat($value, $isInput)
5334
{
5435
if (is_numeric($value) || $value === true || $value === false) {
5536
return (float) $value;
@@ -58,12 +39,18 @@ private function coerceFloat($value, $isInput)
5839
if ($value === '') {
5940
$err = 'Float cannot represent non numeric value: (empty string)';
6041
} else {
61-
$err = sprintf(
62-
'Float cannot represent non numeric value: %s',
63-
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
64-
);
42+
$err = sprintf('Float cannot represent non numeric value: %s', Utils::printSafe($value));
6543
}
66-
throw ($isInput ? new Error($err) : new InvariantViolation($err));
44+
throw new InvariantViolation($err);
45+
}
46+
47+
/**
48+
* @param mixed $value
49+
* @return float|null
50+
*/
51+
public function parseValue($value)
52+
{
53+
return (is_numeric($value) && !is_string($value)) ? (float) $value : null;
6754
}
6855

6956
/**

src/Type/Definition/IDType.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,19 +55,10 @@ public function serialize($value)
5555
*/
5656
public function parseValue($value)
5757
{
58-
if ($value === true) {
59-
return 'true';
60-
}
61-
if ($value === false) {
62-
return 'false';
63-
}
64-
if ($value === null) {
65-
return 'null';
66-
}
6758
if (!is_scalar($value)) {
6859
throw new Error("ID type cannot represent non scalar value: " . Utils::printSafeJson($value));
6960
}
70-
return (string) $value;
61+
return (is_string($value) || is_int($value)) ? (string) $value : null;
7162
}
7263

7364
/**

src/Type/Definition/IntType.php

Lines changed: 14 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -38,39 +38,16 @@ class IntType extends ScalarType
3838
*/
3939
public function serialize($value)
4040
{
41-
return $this->coerceInt($value, false);
42-
}
43-
44-
/**
45-
* @param mixed $value
46-
* @return int|null
47-
*/
48-
public function parseValue($value)
49-
{
50-
return $this->coerceInt($value, true);
51-
}
52-
53-
/**
54-
* @param $value
55-
* @param bool $isInput
56-
* @return int|null
57-
*/
58-
private function coerceInt($value, $isInput)
59-
{
60-
$errClass = $isInput ? Error::class : InvariantViolation::class;
61-
6241
if ($value === '') {
63-
throw new $errClass(
64-
'Int cannot represent non 32-bit signed integer value: (empty string)'
65-
);
42+
throw new InvariantViolation('Int cannot represent non 32-bit signed integer value: (empty string)');
6643
}
6744
if (false === $value || true === $value) {
6845
return (int) $value;
6946
}
7047
if (!is_numeric($value) || $value > self::MAX_INT || $value < self::MIN_INT) {
71-
throw new $errClass(sprintf(
48+
throw new InvariantViolation(sprintf(
7249
'Int cannot represent non 32-bit signed integer value: %s',
73-
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
50+
Utils::printSafe($value)
7451
));
7552
}
7653
$num = (float) $value;
@@ -82,15 +59,24 @@ private function coerceInt($value, $isInput)
8259
// Additionally account for scientific notation (i.e. 1e3), because (float)'1e3' is 1000, but (int)'1e3' is 1
8360
$trimmed = floor($num);
8461
if ($trimmed !== $num) {
85-
throw new $errClass(sprintf(
62+
throw new InvariantViolation(sprintf(
8663
'Int cannot represent non-integer value: %s',
87-
$isInput ? Utils::printSafeJson($value) : Utils::printSafe($value)
64+
Utils::printSafe($value)
8865
));
8966
}
9067
}
9168
return (int) $value;
9269
}
9370

71+
/**
72+
* @param mixed $value
73+
* @return int|null
74+
*/
75+
public function parseValue($value)
76+
{
77+
return (is_int($value) && $value <= self::MAX_INT && $value >= self::MIN_INT) ? $value : null;
78+
}
79+
9480
/**
9581
* @param $ast
9682
* @return int|null

src/Type/Definition/StringType.php

Lines changed: 1 addition & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,10 @@ public function serialize($value)
5252
*/
5353
public function parseValue($value)
5454
{
55-
if ($value === true) {
56-
return 'true';
57-
}
58-
if ($value === false) {
59-
return 'false';
60-
}
61-
if ($value === null) {
62-
return 'null';
63-
}
6455
if (!is_scalar($value)) {
6556
throw new Error("String cannot represent non scalar value: " . Utils::printSafe($value));
6657
}
67-
return (string) $value;
58+
return is_string($value) ? $value : null;
6859
}
6960

7061
/**

tests/Executor/ExecutorTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -248,7 +248,7 @@ public function testProvidesInfoAboutCurrentExecutionState()
248248

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

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

253253
$this->assertEquals([
254254
'fieldName',

tests/Executor/ValuesTest.php

Lines changed: 208 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,208 @@
1+
<?php
2+
namespace GraphQL\Tests\Executor;
3+
4+
use GraphQL\Executor\Values;
5+
use GraphQL\Language\AST\NamedTypeNode;
6+
use GraphQL\Language\AST\NameNode;
7+
use GraphQL\Language\AST\VariableDefinitionNode;
8+
use GraphQL\Language\AST\VariableNode;
9+
use GraphQL\Type\Definition\ObjectType;
10+
use GraphQL\Type\Definition\Type;
11+
use GraphQL\Type\Schema;
12+
13+
class ValuesTest extends \PHPUnit_Framework_Testcase {
14+
15+
public function testGetIDVariableValues()
16+
{
17+
$this->expectInputVariablesMatchOutputVariables(['idInput' => '123456789']);
18+
$this->assertEquals(
19+
['idInput' => '123456789'],
20+
self::runTestCase(['idInput' => 123456789]),
21+
'Integer ID was not converted to string'
22+
);
23+
}
24+
25+
public function testGetBooleanVariableValues()
26+
{
27+
$this->expectInputVariablesMatchOutputVariables(['boolInput' => true]);
28+
$this->expectInputVariablesMatchOutputVariables(['boolInput' => false]);
29+
}
30+
31+
public function testGetIntVariableValues()
32+
{
33+
$this->expectInputVariablesMatchOutputVariables(['intInput' => -1]);
34+
$this->expectInputVariablesMatchOutputVariables(['intInput' => 0]);
35+
$this->expectInputVariablesMatchOutputVariables(['intInput' => 1]);
36+
37+
// Test the int size limit
38+
$this->expectInputVariablesMatchOutputVariables(['intInput' => 2147483647]);
39+
$this->expectInputVariablesMatchOutputVariables(['intInput' => -2147483648]);
40+
}
41+
42+
public function testGetStringVariableValues()
43+
{
44+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => 'meow']);
45+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => '']);
46+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => '1']);
47+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => '0']);
48+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => 'false']);
49+
$this->expectInputVariablesMatchOutputVariables(['stringInput' => '1.2']);
50+
}
51+
52+
public function testGetFloatVariableValues()
53+
{
54+
$this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.2]);
55+
$this->expectInputVariablesMatchOutputVariables(['floatInput' => 1.0]);
56+
$this->expectInputVariablesMatchOutputVariables(['floatInput' => 1]);
57+
$this->expectInputVariablesMatchOutputVariables(['floatInput' => 0]);
58+
$this->expectInputVariablesMatchOutputVariables(['floatInput' => 1e3]);
59+
}
60+
61+
public function testBooleanForIDVariableThrowsError()
62+
{
63+
$this->expectGraphQLError(['idInput' => true]);
64+
}
65+
66+
public function testFloatForIDVariableThrowsError()
67+
{
68+
$this->expectGraphQLError(['idInput' => 1.0]);
69+
}
70+
71+
public function testStringForBooleanVariableThrowsError()
72+
{
73+
$this->expectGraphQLError(['boolInput' => 'true']);
74+
}
75+
76+
public function testIntForBooleanVariableThrowsError()
77+
{
78+
$this->expectGraphQLError(['boolInput' => 1]);
79+
}
80+
81+
public function testFloatForBooleanVariableThrowsError()
82+
{
83+
$this->expectGraphQLError(['boolInput' => 1.0]);
84+
}
85+
86+
public function testBooleanForIntVariableThrowsError()
87+
{
88+
$this->expectGraphQLError(['intInput' => true]);
89+
}
90+
91+
public function testStringForIntVariableThrowsError()
92+
{
93+
$this->expectGraphQLError(['intInput' => 'true']);
94+
}
95+
96+
public function testFloatForIntVariableThrowsError()
97+
{
98+
$this->expectGraphQLError(['intInput' => 1.0]);
99+
}
100+
101+
public function testPositiveBigIntForIntVariableThrowsError()
102+
{
103+
$this->expectGraphQLError(['intInput' => 2147483648]);
104+
}
105+
106+
public function testNegativeBigIntForIntVariableThrowsError()
107+
{
108+
$this->expectGraphQLError(['intInput' => -2147483649]);
109+
}
110+
111+
public function testBooleanForStringVariableThrowsError()
112+
{
113+
$this->expectGraphQLError(['stringInput' => true]);
114+
}
115+
116+
public function testIntForStringVariableThrowsError()
117+
{
118+
$this->expectGraphQLError(['stringInput' => 1]);
119+
}
120+
121+
public function testFloatForStringVariableThrowsError()
122+
{
123+
$this->expectGraphQLError(['stringInput' => 1.0]);
124+
}
125+
126+
public function testBooleanForFloatVariableThrowsError()
127+
{
128+
$this->expectGraphQLError(['floatInput' => true]);
129+
}
130+
131+
public function testStringForFloatVariableThrowsError()
132+
{
133+
$this->expectGraphQLError(['floatInput' => '1.0']);
134+
}
135+
136+
// Helpers for running test cases and making assertions
137+
138+
private function expectInputVariablesMatchOutputVariables($variables)
139+
{
140+
$this->assertEquals(
141+
$variables,
142+
self::runTestCase($variables),
143+
'Output variables did not match input variables' . PHP_EOL . var_export($variables, true) . PHP_EOL
144+
);
145+
}
146+
147+
private function expectGraphQLError($variables)
148+
{
149+
$this->setExpectedException(\GraphQL\Error\Error::class);
150+
self::runTestCase($variables);
151+
}
152+
153+
private static $schema;
154+
155+
private static function getSchema()
156+
{
157+
if (!self::$schema) {
158+
self::$schema = new Schema([
159+
'query' => new ObjectType([
160+
'name' => 'Query',
161+
'fields' => [
162+
'test' => [
163+
'type' => Type::boolean(),
164+
'args' => [
165+
'idInput' => Type::id(),
166+
'boolInput' => Type::boolean(),
167+
'intInput' => Type::int(),
168+
'stringInput' => Type::string(),
169+
'floatInput' => Type::float()
170+
]
171+
],
172+
]
173+
])
174+
]);
175+
}
176+
return self::$schema;
177+
}
178+
179+
private static function getVariableDefinitionNodes()
180+
{
181+
$idInputDefinition = new VariableDefinitionNode([
182+
'variable' => new VariableNode(['name' => new NameNode(['value' => 'idInput'])]),
183+
'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'ID'])])
184+
]);
185+
$boolInputDefinition = new VariableDefinitionNode([
186+
'variable' => new VariableNode(['name' => new NameNode(['value' => 'boolInput'])]),
187+
'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Boolean'])])
188+
]);
189+
$intInputDefinition = new VariableDefinitionNode([
190+
'variable' => new VariableNode(['name' => new NameNode(['value' => 'intInput'])]),
191+
'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Int'])])
192+
]);
193+
$stringInputDefintion = new VariableDefinitionNode([
194+
'variable' => new VariableNode(['name' => new NameNode(['value' => 'stringInput'])]),
195+
'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'String'])])
196+
]);
197+
$floatInputDefinition = new VariableDefinitionNode([
198+
'variable' => new VariableNode(['name' => new NameNode(['value' => 'floatInput'])]),
199+
'type' => new NamedTypeNode(['name' => new NameNode(['value' => 'Float'])])
200+
]);
201+
return [$idInputDefinition, $boolInputDefinition, $intInputDefinition, $stringInputDefintion, $floatInputDefinition];
202+
}
203+
204+
private function runTestCase($variables)
205+
{
206+
return Values::getVariableValues(self::getSchema(), self::getVariableDefinitionNodes(), $variables);
207+
}
208+
}

0 commit comments

Comments
 (0)