diff --git a/src/Type/Definition/BooleanType.php b/src/Type/Definition/BooleanType.php index 63df5eeaa..64746a9e1 100644 --- a/src/Type/Definition/BooleanType.php +++ b/src/Type/Definition/BooleanType.php @@ -34,7 +34,7 @@ public function serialize($value) */ public function parseValue($value) { - return !!$value; + return is_bool($value) ? $value : null; } /** diff --git a/src/Type/Definition/FloatType.php b/src/Type/Definition/FloatType.php index 0c8853a99..092f2583f 100644 --- a/src/Type/Definition/FloatType.php +++ b/src/Type/Definition/FloatType.php @@ -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; @@ -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; } /** diff --git a/src/Type/Definition/IDType.php b/src/Type/Definition/IDType.php index 594bea9e5..49cf7c3d0 100644 --- a/src/Type/Definition/IDType.php +++ b/src/Type/Definition/IDType.php @@ -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; } /** diff --git a/src/Type/Definition/IntType.php b/src/Type/Definition/IntType.php index 4e797df74..b73279294 100644 --- a/src/Type/Definition/IntType.php +++ b/src/Type/Definition/IntType.php @@ -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; @@ -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 diff --git a/src/Type/Definition/StringType.php b/src/Type/Definition/StringType.php index 46bc9424f..4cd3815b6 100644 --- a/src/Type/Definition/StringType.php +++ b/src/Type/Definition/StringType.php @@ -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; } /** diff --git a/tests/Executor/ExecutorTest.php b/tests/Executor/ExecutorTest.php index 5e629350a..c38045e43 100644 --- a/tests/Executor/ExecutorTest.php +++ b/tests/Executor/ExecutorTest.php @@ -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', diff --git a/tests/Executor/ValuesTest.php b/tests/Executor/ValuesTest.php new file mode 100644 index 000000000..a6da3fe9e --- /dev/null +++ b/tests/Executor/ValuesTest.php @@ -0,0 +1,208 @@ +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); + } +} \ No newline at end of file diff --git a/tests/Utils/IsValidPHPValueTest.php b/tests/Utils/IsValidPHPValueTest.php index 051f7043e..937fa4f81 100644 --- a/tests/Utils/IsValidPHPValueTest.php +++ b/tests/Utils/IsValidPHPValueTest.php @@ -9,29 +9,43 @@ class IsValidPHPValueTest extends \PHPUnit_Framework_TestCase { public function testValidIntValue() { - // returns no error for int input - $result = Values::isValidPHPValue('1', Type::int()); + // returns no error for positive int value + $result = Values::isValidPHPValue(1, Type::int()); $this->expectNoErrors($result); - // returns no error for negative int input: - $result = Values::isValidPHPValue('-1', Type::int()); + // returns no error for negative int value + $result = Values::isValidPHPValue(-1, Type::int()); $this->expectNoErrors($result); - // returns no error for exponent input: - $result = Values::isValidPHPValue('1e3', Type::int()); - $this->expectNoErrors($result); - $result = Values::isValidPHPValue('0e3', Type::int()); - $this->expectNoErrors($result); - - // returns no error for null value: + // 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 @@ public function testValidIntValue() public function testValidFloatValue() { - // returns no error for int input - $result = Values::isValidPHPValue('1', Type::float()); + // returns no error for positive float value + $result = Values::isValidPHPValue(1.2, Type::float()); $this->expectNoErrors($result); - // returns no error for exponent input - $result = Values::isValidPHPValue('1e3', Type::float()); + // returns no error for exponential float value + $result = Values::isValidPHPValue(1e3, Type::float()); $this->expectNoErrors($result); - $result = Values::isValidPHPValue('0e3', Type::float()); + + // returns no error for negative float value + $result = Values::isValidPHPValue(-1.2, Type::float()); $this->expectNoErrors($result); - // returns no error for float input - $result = Values::isValidPHPValue('1.5', Type::float()); + // 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);