When resolving a Double value, checking for NaN and returning null

danielkwinsor · danielkwinsor · commit 0964fe1fde09 · 2016-05-12T12:53:47.000-07:00
* When resolving a Double value, checking for NaN and returning null

* When resolving a Double value, checking for NaN and returning null, also for BatchedStrategy
diff --git a/src/main/java/graphql/execution/ExecutionStrategy.java b/src/main/java/graphql/execution/ExecutionStrategy.java
@@ -118,7 +118,12 @@ protected ExecutionResult completeValueForEnum(GraphQLEnumType enumType, Object
     }
 
     protected ExecutionResult completeValueForScalar(GraphQLScalarType scalarType, Object result) {
-        return new ExecutionResultImpl(scalarType.getCoercing().serialize(result), null);
+        Object serialized = scalarType.getCoercing().serialize(result);
+        //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries
+        if (serialized instanceof Double && ((Double) serialized).isNaN()) {
+            serialized = null;
+        }
+        return new ExecutionResultImpl(serialized, null);
     }
 
     protected ExecutionResult completeValueForList(ExecutionContext executionContext, GraphQLList fieldType, List<Field> fields, List<Object> result) {
diff --git a/src/main/java/graphql/execution/ValuesResolver.java b/src/main/java/graphql/execution/ValuesResolver.java
@@ -122,10 +122,7 @@ private Object coerceValueAst(GraphQLType type, Value inputValue, Map<String, Ob
             return variables.get(((VariableReference) inputValue).getName());
         }
         if (type instanceof GraphQLScalarType) {
-            //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries
-            Object value = ((GraphQLScalarType) type).getCoercing().parseLiteral(inputValue);
-            if (value instanceof Double && ((Double) value).isNaN()) return null;
-            return value;
+            return ((GraphQLScalarType) type).getCoercing().parseLiteral(inputValue);
         }
         if (type instanceof GraphQLNonNull) {
             return coerceValueAst(((GraphQLNonNull) type).getWrappedType(), inputValue, variables);
diff --git a/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java b/src/main/java/graphql/execution/batched/BatchedExecutionStrategy.java
@@ -210,6 +210,10 @@ private void handlePrimitives(List<GraphQLExecutionNodeValue> values, String fie
                                   GraphQLType type) {
         for (GraphQLExecutionNodeValue value : values) {
             Object coercedValue = coerce(type, value.getValue());
+            //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries
+            if (coercedValue instanceof Double && ((Double) coercedValue).isNaN()) {
+                coercedValue = null;
+            }
             value.getResultContainer().putResult(fieldName, coercedValue);
         }
     }
diff --git a/src/test/groovy/graphql/ScalarsQuerySchema.java b/src/test/groovy/graphql/ScalarsQuerySchema.java
@@ -24,6 +24,25 @@ public class ScalarsQuerySchema {
                     .type(Scalars.GraphQLBigDecimal)
                     .staticValue(BigDecimal.valueOf(1234.0))
                     .build())
+            .field(newFieldDefinition()
+                    .name("doubleNaN")
+                    .type(Scalars.GraphQLFloat)
+                    .staticValue(Double.NaN)
+                    .build())
+            .field(newFieldDefinition()
+                    .name("doubleNaNInput")
+                    .type(Scalars.GraphQLFloat)
+                    .argument(newArgument()
+                            .name("input")
+                            .type(new GraphQLNonNull(Scalars.GraphQLFloat))
+                            .build())
+                    .dataFetcher(new DataFetcher() {
+                        @Override
+                        public Object get(DataFetchingEnvironment environment) {
+                            return environment.getArgument("input");
+                        }
+                    })
+                    .build())
             .field(newFieldDefinition()
                     .name("bigIntegerInput")
                     .type(Scalars.GraphQLBigInteger)
diff --git a/src/test/groovy/graphql/ScalarsQueryTest.groovy b/src/test/groovy/graphql/ScalarsQueryTest.groovy
@@ -1,5 +1,6 @@
 package graphql
 
+import graphql.execution.batched.BatchedExecutionStrategy
 import spock.lang.Specification
 
 class ScalarsQueryTest extends Specification {
@@ -47,4 +48,24 @@ class ScalarsQueryTest extends Specification {
         then:
         result == expected
     }
+
+    def 'Double NaN Not a Number '() {
+        given:
+        def query = """
+        query DoubleNaN {
+          doubleNaN
+        }
+        """
+        def expected = [
+                doubleNaN: null
+        ]
+
+        when:
+        def result = new GraphQL(ScalarsQuerySchema.scalarsQuerySchema).execute(query).data
+        def resultBatched = new GraphQL(ScalarsQuerySchema.scalarsQuerySchema, new BatchedExecutionStrategy()).execute(query).data
+
+        then:
+        result == expected
+        resultBatched == expected
+    }
 }

Original file line number	Diff line number	Diff line change
`@@ -118,7 +118,12 @@ protected ExecutionResult completeValueForEnum(GraphQLEnumType enumType, Object`
`118`	`118`	`}`
`119`	`119`
`120`	`120`	`protected ExecutionResult completeValueForScalar(GraphQLScalarType scalarType, Object result) {`
`121`		`- return new ExecutionResultImpl(scalarType.getCoercing().serialize(result), null);`
	`121`	`+ Object serialized = scalarType.getCoercing().serialize(result);`
	`122`	`+ //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries`
	`123`	`+ if (serialized instanceof Double && ((Double) serialized).isNaN()) {`
	`124`	`+ serialized = null;`
	`125`	`+ }`
	`126`	`+ return new ExecutionResultImpl(serialized, null);`
`122`	`127`	`}`
`123`	`128`
`124`	`129`	`protected ExecutionResult completeValueForList(ExecutionContext executionContext, GraphQLList fieldType, List<Field> fields, List<Object> result) {`
Original file line number	Diff line number	Diff line change
`@@ -122,10 +122,7 @@ private Object coerceValueAst(GraphQLType type, Value inputValue, Map<String, Ob`
`122`	`122`	`return variables.get(((VariableReference) inputValue).getName());`
`123`	`123`	`}`
`124`	`124`	`if (type instanceof GraphQLScalarType) {`
`125`		`- //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries`
`126`		`- Object value = ((GraphQLScalarType) type).getCoercing().parseLiteral(inputValue);`
`127`		`- if (value instanceof Double && ((Double) value).isNaN()) return null;`
`128`		`- return value;`
	`125`	`+ return ((GraphQLScalarType) type).getCoercing().parseLiteral(inputValue);`
`129`	`126`	`}`
`130`	`127`	`if (type instanceof GraphQLNonNull) {`
`131`	`128`	`return coerceValueAst(((GraphQLNonNull) type).getWrappedType(), inputValue, variables);`
Original file line number	Diff line number	Diff line change
`@@ -210,6 +210,10 @@ private void handlePrimitives(List<GraphQLExecutionNodeValue> values, String fie`
`210`	`210`	`GraphQLType type) {`
`211`	`211`	`for (GraphQLExecutionNodeValue value : values) {`
`212`	`212`	`Object coercedValue = coerce(type, value.getValue());`
	`213`	`+ //6.6.1 http://facebook.github.io/graphql/#sec-Field-entries`
	`214`	`+ if (coercedValue instanceof Double && ((Double) coercedValue).isNaN()) {`
	`215`	`+ coercedValue = null;`
	`216`	`+ }`
`213`	`217`	`value.getResultContainer().putResult(fieldName, coercedValue);`
`214`	`218`	`}`
`215`	`219`	`}`