fix(GraphQL): This PR extend int64 range to 64-bit numeric values and adds input coercing and validation for integers.

graphql/e2e/common/common.go

@@ -319,6 +319,7 @@ func RunAll(t *testing.T) {
 	t.Run("alias works for mutations", mutationsWithAlias)
 	t.Run("three level deep", threeLevelDeepMutation)
 	t.Run("update mutation without set & remove", updateMutationWithoutSetRemove)
+	t.Run("Input coercing for int64 type", int64BoundaryTesting)
 	t.Run("Check cascade with mutation without ID field", checkCascadeWithMutationWithoutIDField)
 
 	// error tests

graphql/e2e/common/error_test.yaml

@@ -115,6 +115,26 @@
     [ { "message": "Field `title` is not present in type `Author`. You can only use fields which are in type `Author`",
     } ]
 
+-
+  name: "Out of range error for int32 type"
+  gqlrequest: |
+    mutation {
+      addPost(input:[{title:"Dgraph",author:{name:"Bob"},numLikes:2147483648}]){
+        post{
+          title
+          numLikes
+          author{
+            name
+          }
+        }
+      }
+    }
+  gqlvariables:
+    { }
+  errors:
+    [ { "message": "Out of range value '2147483648', for type `Int`",
+        "locations": [ { "line": 2, "column": 63 } ] } ]
+
 -
   name: "@cascade only accepts numUids or given type name as arguments for add or update payload "
   gqlrequest: |
@@ -129,4 +149,44 @@
     { }
   errors:
     [ { "message": "Field `name` is not present in type `AddAuthorPayload`. You can only use fields which are in type `AddAuthorPayload`",
-    } ]
+    } ]
+
+-
+  name: "String value is Incompatible with Int64 type"
+  gqlrequest: |
+    mutation {
+      addPost(input:[{title:"Dgraph",author:{name:"Bob"},numViews:"180143985094"}]){
+        post{
+          title
+          numLikes
+          author{
+            name
+          }
+        }
+      }
+    }
+  gqlvariables:
+    { }
+  errors:
+    [ { "message": "Given input type mismatch, Expected Int64",
+        "locations": [ { "line": 2, "column": 64 } ] } ]
+
+-
+  name: "Float value is Incompatible with Int64 type"
+  gqlrequest: |
+    mutation {
+      addPost(input:[{title:"Dgraph",author:{name:"Bob"},numViews:180143985094.0}]){
+        post{
+          title
+          numLikes
+          author{
+            name
+          }
+        }
+      }
+    }
+  gqlvariables:
+    { }
+  errors:
+    [ { "message": "Given input type mismatch, Expected Int64",
+        "locations": [ { "line": 2, "column": 63 } ] } ]

graphql/e2e/common/mutation.go

@@ -2784,10 +2784,8 @@ func deleteGqlType(
 			require.Equal(t, "Deleted", deleteType["msg"], "while deleting %s (filter: %v)",
 				typeName, filter)
 		}
-	} else {
-		if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
-			t.Errorf("errors mismatch (-want +got):\n%s", diff)
-		}
+	} else if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" {
+		t.Errorf("errors mismatch (-want +got):\n%s", diff)
 	}
 }
 
@@ -3648,4 +3646,37 @@ func checkCascadeWithMutationWithoutIDField(t *testing.T) {
 
 	filter := map[string]interface{}{"xcode": map[string]interface{}{"eq": "S2"}}
 	deleteState(t, filter, 1, nil)
-}
+}
+
+func int64BoundaryTesting(t *testing.T) {
+	//This test checks the range of Int64
+	//(2^63)=9223372036854775808
+	addPost1Params := &GraphQLParams{
+		Query: `mutation {
+			addpost1(input: [{title: "Dgraph", numLikes: 9223372036854775807 ,numViews: -9223372036854775808 }]) {
+				post1 {
+					title
+					numLikes
+					numViews
+				}
+			}
+		}`,
+	}
+
+	gqlResponse := addPost1Params.ExecuteAsPost(t, graphqlURL)
+	RequireNoGQLErrors(t, gqlResponse)
+
+	addPost1Expected := `{
+		"addpost1": {
+			"post1": [{
+				"title": "Dgraph",
+				"numLikes": 9223372036854775808,
+				"numViews": -9223372036854775807
+
+			}]
+		}
+	}`
+	testutil.CompareJSON(t, addPost1Expected, string(gqlResponse.Data))
+	filter := map[string]interface{}{"title": map[string]interface{}{"eq": "Dgraph"}}
+	deleteGqlType(t, "post1", filter, 1, nil)
+}

graphql/e2e/directives/schema.graphql

@@ -172,4 +172,11 @@ type Post1 {
 type Comment1 {
       id: String! @id
       replies: [Comment1]
+}
+type post1{
+    id:ID
+    title:String!  @id @search(by: [regexp])
+    numLikes:Int64
+    numViews:Int64
+    numComments:Int64
 }

graphql/e2e/directives/schema_response.json

@@ -82,6 +82,25 @@
             ],
             "upsert": true
         },
+        {
+            "predicate": "post1.numComments",
+            "type": "int"
+        },
+        {
+            "predicate": "post1.title",
+            "type": "string",
+            "index": true,
+            "tokenizer": ["trigram", "hash"],
+            "upsert": true
+        },
+        {
+            "predicate": "post1.numViews",
+            "type": "int"
+        },
+        {
+            "predicate": "post1.numLikes",
+            "type": "int"
+        },
         {
             "predicate": "State.capital",
             "type": "string"
@@ -449,6 +468,23 @@
             ],
             "name": "Post1"
         },
+        {
+            "fields": [
+                {
+                    "name": "post1.numComments"
+                },
+                {
+                    "name": "post1.title"
+                },
+                {
+                    "name": "post1.numViews"
+                },
+                {
+                    "name": "post1.numLikes"
+                }
+            ],
+            "name": "post1"
+        },
         {
             "fields": [
                 {

graphql/e2e/normal/schema.graphql

@@ -172,4 +172,12 @@ type Post1 {
 type Comment1 {
       id: String! @id
       replies: [Comment1]
+}
+
+type post1{
+        id:ID
+        title:String!  @id @search(by: [regexp])
+        numLikes:Int64
+        numViews:Int64
+        numComments:Int64
 }

graphql/e2e/normal/schema_response.json

@@ -239,6 +239,25 @@
             ],
             "upsert": true
         },
+        {
+            "predicate": "post1.numComments",
+            "type": "int"
+        },
+        {
+            "predicate": "post1.title",
+            "type": "string",
+            "index": true,
+            "tokenizer": ["trigram", "hash"],
+            "upsert": true
+        },
+        {
+            "predicate": "post1.numViews",
+            "type": "int"
+        },
+        {
+            "predicate": "post1.numLikes",
+            "type": "int"
+        },
         {
             "predicate": "Starship.length",
             "type": "float"
@@ -537,6 +556,23 @@
             ],
             "name": "Post1"
         },
+        {
+            "fields": [
+                {
+                    "name": "post1.numComments"
+                },
+                {
+                    "name": "post1.title"
+                },
+                {
+                    "name": "post1.numViews"
+                },
+                {
+                    "name": "post1.numLikes"
+                }
+            ],
+            "name": "post1"
+        },
         {
             "fields": [
                 {

graphql/resolve/resolver.go

@@ -689,7 +689,9 @@ func completeDgraphResult(
 	// https://graphql.github.io/graphql-spec/June2018/#sec-Query
 	// So we are only building object results.
 	var valToComplete map[string]interface{}
-	err := json.Unmarshal(dgResult, &valToComplete)
+	d := json.NewDecoder(bytes.NewBuffer(dgResult))
+	d.UseNumber()
+	err := d.Decode(&valToComplete)
 	if err != nil {
 		glog.Errorf("%+v \n Dgraph result :\n%s\n",
 			errors.Wrap(err, "failed to unmarshal Dgraph query result"),
@@ -1477,6 +1479,8 @@ func coerceScalar(val interface{}, field schema.Field, path []interface{}) (inte
 		}
 	case "Int":
 		switch v := val.(type) {
+		// We have already checked range for int32 at input validation time.
+		// So now just parse and check errors.
 		case float64:
 			// The spec says that we can coerce a Float value to Int, if we don't lose information.
 			// See: https: //spec.graphql.org/June2018/#sec-Float
@@ -1499,49 +1503,32 @@ func coerceScalar(val interface{}, field schema.Field, path []interface{}) (inte
 		case string:
 			i, err := strconv.ParseFloat(v, 64)
 			// An error can be encountered if we had a value that can't be fit into
-			// a 64 bit floating point number.
+			// a 64 bit int.
 			if err != nil {
 				return nil, valueCoercionError(v)
 			}
-			// Lets try to see if this number could be converted to int32 without losing
-			// information, otherwise return error.
-			i32Val := int32(i)
-			if i == float64(i32Val) {
-				val = i32Val
-			} else {
+			val = i
+		case int: // numUids are added as int, so we need special handling for that.
+			if v > math.MaxInt32 || v < math.MinInt32 {
 				return nil, valueCoercionError(v)
 			}
-		case int64:
+		case int64: // numUids are added as int, so we need special handling for that.
 			if v > math.MaxInt32 || v < math.MinInt32 {
 				return nil, valueCoercionError(v)
 			}
-		case int:
-			// numUids are added as int, so we need special handling for that. Other number values
-			// in a JSON object are automatically unmarshalled as float so they are handle above.
-			if v > math.MaxInt32 || v < math.MinInt32 {
+		case json.Number:
+			i, err := strconv.ParseInt(v.String(), 10, 32)
+			if err != nil {
 				return nil, valueCoercionError(v)
 			}
+			val = i
 		default:
 			return nil, valueCoercionError(v)
 		}
 	case "Int64":
 		switch v := val.(type) {
-		case float64:
-			// The spec says that we can coerce a Float value to Int, if we don't lose information.
-			// See: https: //spec.graphql.org/June2018/#sec-Float
-			// See: JSON RFC https://tools.ietf.org/html/rfc8259#section-6, to understand how the
-			// number type guarantees the correctness of integers only between the range
-			// [-(2**53)+1, (2**53)-1] and not the range [-(2**63), (2**63)-1].
-			// Lets try to see if this number could be converted to int64 without losing
-			// information, otherwise return error.
-			// See: https://github.com/golang/go/issues/19405 to understand why the comparison
-			// should be done after double conversion.
-			i64Val := int64(v)
-			if v == float64(i64Val) {
-				val = i64Val
-			} else {
-				return nil, valueCoercionError(v)
-			}
+		// To use whole 64-bit range for int64 without any coercing,
+		// We pass int64 values as string to dgraph and parse as integer here
 		case bool:
 			if v {
 				val = 1
@@ -1551,18 +1538,19 @@ func coerceScalar(val interface{}, field schema.Field, path []interface{}) (inte
 		case string:
 			i, err := strconv.ParseFloat(v, 64)
 			// An error can be encountered if we had a value that can't be fit into
-			// a 64 bit floating point number.
+			// a 64 bit int or because of other parsing issues.
 			if err != nil {
 				return nil, valueCoercionError(v)
 			}
-			// Lets try to see if this number could be converted to int64 without losing
-			// information, otherwise return error.
-			i64Val := int64(i)
-			if i == float64(i64Val) {
-				val = i64Val
-			} else {
+			val=i
+		case json.Number:
+			i, err := strconv.ParseInt(v.String(), 10, 64)
+			// An error can be encountered if we had a value that can't be fit into
+			// a 64 bit int or because of other parsing issues.
+			if err != nil {
 				return nil, valueCoercionError(v)
 			}
+			val = i
 		default:
 			return nil, valueCoercionError(v)
 		}
@@ -1576,14 +1564,22 @@ func coerceScalar(val interface{}, field schema.Field, path []interface{}) (inte
 			}
 		case string:
 			i, err := strconv.ParseFloat(v, 64)
+			// An error can be encountered if we had a value that can't be fit into
+			// a 64 bit floating point number or because of other parsing issues.
+			if err != nil {
+				return nil, valueCoercionError(v)
+			}
+			val = i
+		case json.Number:
+			i, err := strconv.ParseFloat(v.String(), 64)
 			if err != nil {
 				return nil, valueCoercionError(v)
 			}
 			val = i
-		case int64:
-			val = float64(v)
 		case float64:
 		default:
+			// An error can be encountered if we had a value that can't be fit into
+			// a 64 bit floating point number or because of other parsing issues.
 			return nil, valueCoercionError(v)
 		}
 	case "DateTime":