diff --git a/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java b/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java index 266e64c4a5..6ef64c5976 100644 --- a/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlBooleanCoercing.java @@ -68,13 +68,12 @@ private Boolean serializeImpl(@NotNull Object input, @NotNull Locale locale) { @NotNull private Boolean parseValueImpl(@NotNull Object input, @NotNull Locale locale) { - Boolean result = convertImpl(input); - if (result == null) { + if (!(input instanceof Boolean)) { throw new CoercingParseValueException( - i18nMsg(locale, "Boolean.notBoolean", typeName(input)) + i18nMsg(locale, "Boolean.unexpectedRawValueType", typeName(input)) ); } - return result; + return (Boolean) input; } private static boolean parseLiteralImpl(@NotNull Object input, @NotNull Locale locale) { diff --git a/src/main/java/graphql/scalar/GraphqlFloatCoercing.java b/src/main/java/graphql/scalar/GraphqlFloatCoercing.java index cb7d43b43b..683f915634 100644 --- a/src/main/java/graphql/scalar/GraphqlFloatCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlFloatCoercing.java @@ -64,13 +64,20 @@ private Double serialiseImpl(Object input, @NotNull Locale locale) { } @NotNull - private Double parseValueImpl(Object input, @NotNull Locale locale) { + private Double parseValueImpl(@NotNull Object input, @NotNull Locale locale) { + if (!(input instanceof Number)) { + throw new CoercingParseValueException( + i18nMsg(locale, "Float.unexpectedRawValueType", typeName(input)) + ); + } + Double result = convertImpl(input); if (result == null) { throw new CoercingParseValueException( i18nMsg(locale, "Float.notFloat", typeName(input)) ); } + return result; } diff --git a/src/main/java/graphql/scalar/GraphqlIntCoercing.java b/src/main/java/graphql/scalar/GraphqlIntCoercing.java index 1ca0f5b2ee..350c4f4814 100644 --- a/src/main/java/graphql/scalar/GraphqlIntCoercing.java +++ b/src/main/java/graphql/scalar/GraphqlIntCoercing.java @@ -63,14 +63,46 @@ private Integer serialiseImpl(Object input, @NotNull Locale locale) { } @NotNull - private Integer parseValueImpl(Object input, @NotNull Locale locale) { - Integer result = convertImpl(input); + private Integer parseValueImpl(@NotNull Object input, @NotNull Locale locale) { + if (!(input instanceof Number)) { + throw new CoercingParseValueException( + i18nMsg(locale, "Int.notInt", typeName(input)) + ); + } + + if (input instanceof Integer) { + return (Integer) input; + } + + BigInteger result = convertParseValueImpl(input); if (result == null) { throw new CoercingParseValueException( i18nMsg(locale, "Int.notInt", typeName(input)) ); } - return result; + + if (result.compareTo(INT_MIN) < 0 || result.compareTo(INT_MAX) > 0) { + throw new CoercingParseValueException( + i18nMsg(locale, "Int.outsideRange", result.toString()) + ); + } + return result.intValueExact(); + } + + private BigInteger convertParseValueImpl(Object input) { + BigDecimal value; + try { + value = new BigDecimal(input.toString()); + } catch (NumberFormatException e) { + return null; + } + + try { + return value.toBigIntegerExact(); + } catch (ArithmeticException e) { + // Exception if number has non-zero fractional part + return null; + } } private static int parseLiteralImpl(Object input, @NotNull Locale locale) { diff --git a/src/main/resources/i18n/Scalars.properties b/src/main/resources/i18n/Scalars.properties index 4dbc68d683..39fc6b4105 100644 --- a/src/main/resources/i18n/Scalars.properties +++ b/src/main/resources/i18n/Scalars.properties @@ -24,8 +24,10 @@ ID.unexpectedAstType=Expected an AST type of ''IntValue'' or ''StringValue'' but # Float.notFloat=Expected a value that can be converted to type ''Float'' but it was a ''{0}'' Float.unexpectedAstType=Expected an AST type of ''IntValue'' or ''FloatValue'' but it was a ''{0}'' +Float.unexpectedRawValueType=Expected a Number input, but it was a ''{0}'' # Boolean.notBoolean=Expected a value that can be converted to type ''Boolean'' but it was a ''{0}'' Boolean.unexpectedAstType=Expected an AST type of ''BooleanValue'' but it was a ''{0}'' +Boolean.unexpectedRawValueType=Expected a Boolean input, but it was a ''{0}'' # String.unexpectedRawValueType=Expected a String input, but it was a ''{0}'' diff --git a/src/main/resources/i18n/Scalars_de.properties b/src/main/resources/i18n/Scalars_de.properties index ce045d8404..02b1b27a75 100644 --- a/src/main/resources/i18n/Scalars_de.properties +++ b/src/main/resources/i18n/Scalars_de.properties @@ -27,8 +27,10 @@ ID.unexpectedAstType=Erwartet wurde ein AST type von ''IntValue'' oder ''StringV # Float.notFloat=Erwartet wurde ein Wert, der in den Typ ''Float'' konvertiert werden kann, aber es war ein ''{0}'' Float.unexpectedAstType=Erwartet wurde ein AST type von ''IntValue'' oder ''FloatValue'', aber es war ein ''{0}'' +Float.unexpectedRawValueType=Erwartet wurde eine Number-Eingabe, aber es war ein ''{0}'' # Boolean.notBoolean=Erwartet wurde ein Wert, der in den Typ ''Boolean'' konvertiert werden kann, aber es war ein ''{0}'' Boolean.unexpectedAstType=Erwartet wurde ein AST type ''BooleanValue'', aber es war ein ''{0}'' +Boolean.unexpectedRawValueType=Erwartet wurde eine Boolean-Eingabe, aber es war ein ''{0}'' # String.unexpectedRawValueType=Erwartet wurde eine String-Eingabe, aber es war ein ''{0}'' diff --git a/src/test/groovy/graphql/ScalarsBooleanTest.groovy b/src/test/groovy/graphql/ScalarsBooleanTest.groovy index c059d5c53c..5b316765ad 100644 --- a/src/test/groovy/graphql/ScalarsBooleanTest.groovy +++ b/src/test/groovy/graphql/ScalarsBooleanTest.groovy @@ -53,7 +53,6 @@ class ScalarsBooleanTest extends Specification { def "Boolean serialize #value into #result (#result.class)"() { expect: Scalars.GraphQLBoolean.getCoercing().serialize(value, GraphQLContext.default, Locale.default) == result - Scalars.GraphQLBoolean.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result where: value | result @@ -75,7 +74,6 @@ class ScalarsBooleanTest extends Specification { def "Boolean serialize #value into #result (#result.class) with deprecated methods"() { expect: Scalars.GraphQLBoolean.getCoercing().serialize(value) == result // Retain deprecated method for test coverage - Scalars.GraphQLBoolean.getCoercing().parseValue(value) == result // Retain deprecated method for test coverage where: value | result @@ -111,6 +109,28 @@ class ScalarsBooleanTest extends Specification { "f" | _ } + @Unroll + def "Boolean parseValue #value into #result (#result.class)"() { + expect: + Scalars.GraphQLBoolean.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result + + where: + value | result + true | true + false | false + } + + @Unroll + def "Boolean parseValue #value into #result (#result.class) with deprecated methods"() { + expect: + Scalars.GraphQLBoolean.getCoercing().parseValue(value) == result // Retain deprecated method for test coverage + + where: + value | result + true | true + false | false + } + @Unroll def "parseValue throws exception for invalid input #value"() { when: @@ -119,8 +139,19 @@ class ScalarsBooleanTest extends Specification { thrown(CoercingParseValueException) where: - value | _ - new Object() | _ + value | _ + new Object() | _ + "false" | _ + "true" | _ + "True" | _ + 0 | _ + 1 | _ + -1 | _ + new Long(42345784398534785l) | _ + new Double(42.3) | _ + new Float(42.3) | _ + Integer.MAX_VALUE + 1l | _ + Integer.MIN_VALUE - 1l | _ } } diff --git a/src/test/groovy/graphql/ScalarsFloatTest.groovy b/src/test/groovy/graphql/ScalarsFloatTest.groovy index 07fea26fd1..6f6a195d65 100644 --- a/src/test/groovy/graphql/ScalarsFloatTest.groovy +++ b/src/test/groovy/graphql/ScalarsFloatTest.groovy @@ -58,7 +58,6 @@ class ScalarsFloatTest extends Specification { def "Float serialize #value into #result (#result.class)"() { expect: Scalars.GraphQLFloat.getCoercing().serialize(value, GraphQLContext.default, Locale.default) == result - Scalars.GraphQLFloat.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result where: value | result @@ -84,7 +83,6 @@ class ScalarsFloatTest extends Specification { def "Float serialize #value into #result (#result.class) with deprecated methods"() { expect: Scalars.GraphQLFloat.getCoercing().serialize(value) == result // Retain deprecated method for coverage - Scalars.GraphQLFloat.getCoercing().parseValue(value) == result // Retain deprecated method for coverage where: value | result @@ -131,6 +129,51 @@ class ScalarsFloatTest extends Specification { Float.NEGATIVE_INFINITY.toString() | _ } + @Unroll + def "Float parseValue #value into #result (#result.class)"() { + expect: + Scalars.GraphQLFloat.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result + + where: + value | result + 42.0000d | 42 + new Integer(42) | 42 + new BigInteger("42") | 42 + new BigDecimal("42") | 42 + new BigDecimal("4.2") | 4.2d + 42.3f | 42.3d + 42.0d | 42d + new Byte("42") | 42 + new Short("42") | 42 + 1234567l | 1234567d + new AtomicInteger(42) | 42 + Double.MAX_VALUE | Double.MAX_VALUE + Double.MIN_VALUE | Double.MIN_VALUE + } + + @Unroll + def "Float parseValue #value into #result (#result.class) with deprecated methods"() { + expect: + Scalars.GraphQLFloat.getCoercing().parseValue(value) == result // Retain deprecated method for coverage + + where: + value | result + 42.0000d | 42 + new Integer(42) | 42 + new BigInteger("42") | 42 + new BigDecimal("42") | 42 + new BigDecimal("4.2") | 4.2d + 42.3f | 42.3d + 42.0d | 42d + new Byte("42") | 42 + new Short("42") | 42 + 1234567l | 1234567d + new AtomicInteger(42) | 42 + Double.MAX_VALUE | Double.MAX_VALUE + Double.MIN_VALUE | Double.MIN_VALUE + } + + @Unroll def "parseValue throws exception for invalid input #value"() { when: @@ -154,6 +197,9 @@ class ScalarsFloatTest extends Specification { Float.POSITIVE_INFINITY.toString() | _ Float.NEGATIVE_INFINITY | _ Float.NEGATIVE_INFINITY.toString() | _ + "42" | _ + "42.123" | _ + "-1" | _ } } diff --git a/src/test/groovy/graphql/ScalarsIntTest.groovy b/src/test/groovy/graphql/ScalarsIntTest.groovy index 883d9dbf17..7a5b43ed9e 100644 --- a/src/test/groovy/graphql/ScalarsIntTest.groovy +++ b/src/test/groovy/graphql/ScalarsIntTest.groovy @@ -60,7 +60,6 @@ class ScalarsIntTest extends Specification { def "Int serialize #value into #result (#result.class)"() { expect: Scalars.GraphQLInt.getCoercing().serialize(value, GraphQLContext.default, Locale.default) == result - Scalars.GraphQLInt.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result where: value | result @@ -85,7 +84,6 @@ class ScalarsIntTest extends Specification { def "Int serialize #value into #result (#result.class) with deprecated methods"() { expect: Scalars.GraphQLInt.getCoercing().serialize(value) == result // Retain deprecated for test coverage - Scalars.GraphQLInt.getCoercing().parseValue(value) == result // Retain deprecated for test coverage where: value | result @@ -126,6 +124,48 @@ class ScalarsIntTest extends Specification { new Object() | _ } + @Unroll + def "Int parseValue #value into #result (#result.class)"() { + expect: + Scalars.GraphQLInt.getCoercing().parseValue(value, GraphQLContext.default, Locale.default) == result + + where: + value | result + new Integer(42) | 42 + new BigInteger("42") | 42 + new Byte("42") | 42 + new Short("42") | 42 + 1234567l | 1234567 + new AtomicInteger(42) | 42 + Integer.MAX_VALUE | Integer.MAX_VALUE + Integer.MIN_VALUE | Integer.MIN_VALUE + 42.0000d | 42 + new BigDecimal("42") | 42 + 42.0f | 42 + 42.0d | 42 + } + + @Unroll + def "Int parseValue #value into #result (#result.class) with deprecated methods"() { + expect: + Scalars.GraphQLInt.getCoercing().parseValue(value) == result // Retain deprecated for test coverage + + where: + value | result + 42.0000d | 42 + new Integer(42) | 42 + new BigInteger("42") | 42 + new BigDecimal("42") | 42 + 42.0f | 42 + 42.0d | 42 + new Byte("42") | 42 + new Short("42") | 42 + 1234567l | 1234567 + new AtomicInteger(42) | 42 + Integer.MAX_VALUE | Integer.MAX_VALUE + Integer.MIN_VALUE | Integer.MIN_VALUE + } + @Unroll def "parseValue throws exception for invalid input #value"() { when: @@ -144,6 +184,9 @@ class ScalarsIntTest extends Specification { Integer.MAX_VALUE + 1l | _ Integer.MIN_VALUE - 1l | _ new Object() | _ + "42" | _ + "42.0000" | _ + "-1" | _ } } diff --git a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy index 203d12403e..9c2ec5d2d0 100644 --- a/src/test/groovy/graphql/execution/ValuesResolverTest.groovy +++ b/src/test/groovy/graphql/execution/ValuesResolverTest.groovy @@ -56,8 +56,8 @@ class ValuesResolverTest extends Specification { inputType | variableType | inputValue || outputValue GraphQLInt | new TypeName("Int") | 100 || 100 GraphQLString | new TypeName("String") | 'someString' || 'someString' - GraphQLBoolean | new TypeName("Boolean") | 'true' || true - GraphQLFloat | new TypeName("Float") | '42.43' || 42.43d + GraphQLBoolean | new TypeName("Boolean") | true || true + GraphQLFloat | new TypeName("Float") | 42.43d || 42.43d } def "getVariableValues: map object as variable input"() { @@ -641,7 +641,7 @@ class ValuesResolverTest extends Specification { executionResult.data == null executionResult.errors.size() == 1 executionResult.errors[0].errorType == ErrorType.ValidationError - executionResult.errors[0].message == "Variable 'input' has an invalid value: Expected a value that can be converted to type 'Boolean' but it was a 'String'" + executionResult.errors[0].message == "Variable 'input' has an invalid value: Expected a Boolean input, but it was a 'String'" executionResult.errors[0].locations == [new SourceLocation(2, 35)] } @@ -679,7 +679,7 @@ class ValuesResolverTest extends Specification { executionResult.data == null executionResult.errors.size() == 1 executionResult.errors[0].errorType == ErrorType.ValidationError - executionResult.errors[0].message == "Variable 'input' has an invalid value: Expected a value that can be converted to type 'Float' but it was a 'String'" + executionResult.errors[0].message == "Variable 'input' has an invalid value: Expected a Number input, but it was a 'String'" executionResult.errors[0].locations == [new SourceLocation(2, 35)] } } \ No newline at end of file