Merge pull request #171 from dereklavigne18/query_variable_coercion

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

src/Type/Definition/BooleanType.php

@@ -34,7 +34,7 @@ class BooleanType extends ScalarType
      */
     public function parseValue($value)
     {
-        return !!$value;
+        return is_bool($value) ? $value : null;
     }
 
     /**

src/Type/Definition/FloatType.php

@@ -32,7 +32,16 @@ values as specified by
      */
     public function serialize($value)
     {
-        return $this->coerceFloat($value, false);
+        if (is_numeric($value) || $value === true || $value === false) {
+            return (float) $value;
+        }
+
+        if ($value === '') {
+            $err = 'Float cannot represent non numeric value: (empty string)';
+        } else {
+            $err = sprintf('Float cannot represent non numeric value: %s', Utils::printSafe($value));
+        }
+        throw new InvariantViolation($err);
     }
 
     /**
@@ -41,29 +50,7 @@ values as specified by
      */
     public function parseValue($value)
     {
-        return $this->coerceFloat($value, true);
+        return (is_numeric($value) && !is_string($value)) ? (float) $value : null;
-    }
-
-    /**
-     * @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;
-        }
-
-        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)
-            );
-        }
-        throw ($isInput ? new Error($err) : new InvariantViolation($err));
     }
 
     /**

src/Type/Definition/IDType.php

@@ -55,19 +55,10 @@ When expected as an input type, any string (such as `"4"`) or integer
      */
     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;
     }
 
     /**

src/Type/Definition/IntType.php

@@ -38,39 +38,16 @@ values. Int can represent values between -(2^31) and 2^31 - 1. ';
      */
     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(
+            throw new InvariantViolation('Int cannot represent non 32-bit signed integer value: (empty string)');
-                '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;
@@ -82,15 +59,24 @@ values. Int can represent values between -(2^31) and 2^31 - 1. ';
             // 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

src/Type/Definition/StringType.php

@@ -52,19 +52,10 @@ represent free-form human-readable text.';
      */
     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;
     }
 
     /**

tests/Executor/ExecutorTest.php

@@ -248,7 +248,7 @@ class ExecutorTest extends \PHPUnit_Framework_TestCase
 
         $rootValue = [ 'root' => 'val' ];
 
-        Executor::execute($schema, $ast, $rootValue, null, [ 'var' => 123 ]);
+        Executor::execute($schema, $ast, $rootValue, null, [ 'var' => '123' ]);
 
         $this->assertEquals([
             'fieldName',

tests/Executor/ValuesTest.php

@@ -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);
+  }
+}

tests/Utils/IsValidPHPValueTest.php

@@ -9,29 +9,43 @@ class IsValidPHPValueTest extends \PHPUnit_Framework_TestCase
 {
     public function testValidIntValue()
     {
-        // returns no error for int input
+        // returns no error for positive int value
-        $result = Values::isValidPHPValue('1', Type::int());
+        $result = Values::isValidPHPValue(1, Type::int());
         $this->expectNoErrors($result);
 
-        // returns no error for negative int input:
+        // returns no error for negative int value
-        $result = Values::isValidPHPValue('-1', Type::int());
+        $result = Values::isValidPHPValue(-1, Type::int());
         $this->expectNoErrors($result);
 
-        // returns no error for exponent input:
+        // returns no error for null value
-        $result = Values::isValidPHPValue('1e3', Type::int());
-        $this->expectNoErrors($result);
-        $result = Values::isValidPHPValue('0e3', 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 input as int
+        // 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);
 
@@ -46,24 +60,52 @@ class IsValidPHPValueTest extends \PHPUnit_Framework_TestCase
 
     public function testValidFloatValue()
     {
-        // returns no error for int input
+        // returns no error for positive float value
-        $result = Values::isValidPHPValue('1', Type::float());
+        $result = Values::isValidPHPValue(1.2, Type::float());
         $this->expectNoErrors($result);
 
-        // returns no error for exponent input
+        // returns no error for exponential float value
-        $result = Values::isValidPHPValue('1e3', Type::float());
+        $result = Values::isValidPHPValue(1e3, Type::float());
-        $this->expectNoErrors($result);
-        $result = Values::isValidPHPValue('0e3', Type::float());
         $this->expectNoErrors($result);
 
-        // returns no error for float input
+        // returns no error for negative float value
-        $result = Values::isValidPHPValue('1.5', Type::float());
+        $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);