fix(GraphQL): remove support of @id directive on Float (#7583)

Fixes GRAPHQL-1075.
dgraph-io · Mar 25, 2021 · 882f411 · 882f411
1 parent a22ba1b
commit 882f411
Show file tree

Hide file tree

Showing 15 changed files with 16 additions and 207 deletions.
diff --git a/graphql/e2e/common/common.go b/graphql/e2e/common/common.go
@@ -835,7 +835,6 @@ func RunAll(t *testing.T) {
 	t.Run("checkUserPassword query", passwordTest)
 	t.Run("query id directive with int", idDirectiveWithInt)
 	t.Run("query id directive with int64", idDirectiveWithInt64)
-	t.Run("query id directive with float", idDirectiveWithFloat)
 	t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)
 
 	// mutation tests
@@ -886,7 +885,6 @@ func RunAll(t *testing.T) {
 	t.Run("filter in update mutations with array for AND/OR", filterInUpdateMutationsWithFilterAndOr)
 	t.Run("mutation id directive with int", idDirectiveWithIntMutation)
 	t.Run("mutation id directive with int64", idDirectiveWithInt64Mutation)
-	t.Run("mutation id directive with float", idDirectiveWithFloatMutation)
 	t.Run("add mutation on extended type with field of ID type as key field", addMutationOnExtendedTypeWithIDasKeyField)
 	t.Run("add mutation with deep extended type objects", addMutationWithDeepExtendedTypeObjects)
 	t.Run("three level double XID mutation", threeLevelDoubleXID)

diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go
@@ -5032,40 +5032,6 @@ func idDirectiveWithIntMutation(t *testing.T) {
 	DeleteGqlType(t, "Chapter", map[string]interface{}{}, 3, nil)
 }
 
-func idDirectiveWithFloatMutation(t *testing.T) {
-	query := &GraphQLParams{
-		Query: `mutation {
-          addSection(input:[{
-            chapterId: 2
-            name: "Graphql: Introduction"
-            sectionId: 2.1
-          },
-          {
-            chapterId: 2
-            name: "Graphql Available Data Types"
-            sectionId: 2.2
-          }]) {
-            numUids
-          }
-        }`,
-	}
-
-	response := query.ExecuteAsPost(t, GraphqlURL)
-	RequireNoGQLErrors(t, response)
-	var expected = `{
-            "addSection": {
-              "numUids": 2
-            }
-        }`
-	require.JSONEq(t, expected, string(response.Data))
-
-	// adding same mutation again should result in error because of duplicate id
-	response = query.ExecuteAsPost(t, GraphqlURL)
-	require.Contains(t, response.Errors.Error(), "already exists")
-
-	DeleteGqlType(t, "Section", map[string]interface{}{}, 4, nil)
-}
-
 func addMutationWithDeepExtendedTypeObjects(t *testing.T) {
 	varMap1 := map[string]interface{}{
 		"missionId":   "Mission1",

diff --git a/graphql/e2e/common/query.go b/graphql/e2e/common/query.go
@@ -3913,26 +3913,3 @@ func idDirectiveWithInt(t *testing.T) {
 	}`
 	require.JSONEq(t, expected, string(response.Data))
 }
-
-func idDirectiveWithFloat(t *testing.T) {
-	query := &GraphQLParams{
-		Query: `query {
-		  getSection(sectionId: 1.1) {
-			chapterId
-			sectionId
-			name
-		  }
-		}`,
-	}
-
-	response := query.ExecuteAsPost(t, GraphqlURL)
-	RequireNoGQLErrors(t, response)
-	var expected = `{
-	 	"getSection": {
-			"chapterId": 1,
-			"sectionId": 1.1,
-			"name": "How to define dgraph schema"
-		  }
-	}`
-	require.JSONEq(t, expected, string(response.Data))
-}
diff --git a/graphql/e2e/directives/schema.graphql b/graphql/e2e/directives/schema.graphql
@@ -301,12 +301,6 @@ type Chapter {
 	book: Book
 }
 
-type Section {
-    sectionId: Float! @id
-    name: String!
-    chapterId: Int! @search
-}
-
 type Mission @key(fields: "id") {
     id: String! @id
     crew: [Astronaut] @provides(fields: "name") @hasInverse(field: missions)

diff --git a/graphql/e2e/directives/schema_response.json b/graphql/e2e/directives/schema_response.json
@@ -401,27 +401,6 @@
       "predicate": "Region.name",
       "type": "string"
     },
-    {
-      "predicate": "Section.chapterId",
-      "type": "int",
-      "index": true,
-      "tokenizer": [
-        "int"
-      ]
-    },
-    {
-      "predicate": "Section.name",
-      "type": "string"
-    },
-    {
-      "predicate": "Section.sectionId",
-      "type": "float",
-      "index": true,
-      "tokenizer": [
-        "float"
-      ],
-      "upsert": true
-    },
     {
       "predicate": "SpaceShip.id",
       "type": "string",
@@ -1136,20 +1115,6 @@
       ],
       "name": "Region"
     },
-    {
-      "fields": [
-        {
-          "name": "Section.sectionId"
-        },
-        {
-          "name": "Section.name"
-        },
-        {
-          "name": "Section.chapterId"
-        }
-      ],
-      "name": "Section"
-    },
     {
       "fields": [
         {

diff --git a/graphql/e2e/directives/test_data.json b/graphql/e2e/directives/test_data.json
@@ -143,19 +143,5 @@
         "dgraph.type": "Chapter",
         "Chapter.chapterId": 1,
         "Chapter.name": "How Dgraph Works"
-    },
-    {
-        "uid": "_:section1",
-        "dgraph.type": "Section",
-        "Section.chapterId": 1,
-        "Section.sectionId": 1.1,
-        "Section.name": "How to define dgraph schema"
-    },
-    {
-        "uid": "_:section2",
-        "dgraph.type": "Section",
-        "Section.chapterId": 1,
-        "Section.sectionId": 1.2,
-        "Section.name": "Dgraph Data Types"
     }
 ]
diff --git a/graphql/e2e/normal/schema.graphql b/graphql/e2e/normal/schema.graphql
@@ -307,12 +307,6 @@ type Chapter {
         book: Book
 }
 
-type Section {
-        sectionId: Float! @id
-        name: String!
-        chapterId: Int! @search
-}
-
 # multiple fields with @id directive
 type Worker {
         name: String!

diff --git a/graphql/e2e/normal/schema_response.json b/graphql/e2e/normal/schema_response.json
@@ -562,27 +562,6 @@
       "predicate": "Region.name",
       "type": "string"
     },
-    {
-      "predicate": "Section.chapterId",
-      "type": "int",
-      "index": true,
-      "tokenizer": [
-        "int"
-      ]
-    },
-    {
-      "predicate": "Section.name",
-      "type": "string"
-    },
-    {
-      "predicate": "Section.sectionId",
-      "type": "float",
-      "index": true,
-      "tokenizer": [
-        "float"
-      ],
-      "upsert": true
-    },
     {
       "predicate": "SpaceShip.id",
       "type": "string",
@@ -1227,20 +1206,6 @@
       ],
       "name": "Region"
     },
-    {
-      "fields": [
-        {
-          "name": "Section.sectionId"
-        },
-        {
-          "name": "Section.name"
-        },
-        {
-          "name": "Section.chapterId"
-        }
-      ],
-      "name": "Section"
-    },
     {
       "fields": [
         {

diff --git a/graphql/e2e/normal/test_data.json b/graphql/e2e/normal/test_data.json
@@ -143,19 +143,5 @@
         "dgraph.type": "Chapter",
         "Chapter.chapterId": 1,
         "Chapter.name": "How Dgraph Works"
-    },
-    {
-        "uid": "_:section1",
-        "dgraph.type": "Section",
-        "Section.chapterId": 1,
-        "Section.sectionId": 1.1,
-        "Section.name": "How to define dgraph schema"
-    },
-    {
-        "uid": "_:section2",
-        "dgraph.type": "Section",
-        "Section.chapterId": 1,
-        "Section.sectionId": 1.2,
-        "Section.name": "Dgraph Data Types"
     }
 ]
diff --git a/graphql/resolve/mutation_rewriter.go b/graphql/resolve/mutation_rewriter.go
@@ -2329,20 +2329,6 @@ func extractVal(xidVal interface{}, xidName, typeName string) (string, error) {
 			return "", fmt.Errorf("encountered an XID %s with %s that isn't "+
 				"a Int64 but data type in schema is Int64", xidName, typeName)
 		}
-	case "Float":
-		switch xVal := xidVal.(type) {
-		case json.Number:
-			val, err := xVal.Float64()
-			if err != nil {
-				return "", err
-			}
-			return strconv.FormatFloat(val, 'f', -1, 64), nil
-		case float64:
-			return strconv.FormatFloat(xVal, 'f', -1, 64), nil
-		default:
-			return "", fmt.Errorf("encountered an XID %s with %s that isn't "+
-				"a Float but data type in schema is Float", xidName, typeName)
-		}
 		// "ID" is given as input for the @extended type mutation.
 	case "String", "ID":
 		xidString, ok := xidVal.(string)

diff --git a/graphql/schema/dgraph_schemagen_test.yml b/graphql/schema/dgraph_schemagen_test.yml
@@ -760,20 +760,6 @@ schemas:
       T.id: int @index(int) @upsert .
       T.value: string .
 
-  - name: "Float field with @id Directive"
-    input: |
-      type T {
-        id : Float! @id
-        value: String
-      }
-    output: |
-      type T {
-        T.id
-        T.value
-      }
-      T.id: float @index(float) @upsert .
-      T.value: string .
-
   - name: "type extension having @external field of ID type which is @key"
     input: |
       extend type Product @key(fields: "id") {

diff --git a/graphql/schema/gqlschema_test.yml b/graphql/schema/gqlschema_test.yml
@@ -239,7 +239,7 @@ invalid_schemas:
     errlist: [
       { "message": "Type Z; Field f: Field f is of type U, but @hasInverse directive only applies to fields with object types.", "locations": [{"line":9, "column":3}]},
       { "message": "Type Z; Field f: has the @search directive but fields of type U can't have the @search directive.", "locations": [{"line":9, "column":34}]},
-      { "message": "Type Z; Field f: with @id directive must be of type String!, Int!, Int64! or Float!, not U", "locations": [{"line":9, "column":42}]}
+      { "message": "Type Z; Field f: with @id directive must be of type String!, Int! or Int64!, not U", "locations": [{"line":9, "column":42}]}
     ]
 
   -
@@ -622,18 +622,29 @@ invalid_schemas:
         f1: [String] @id
       }
     errlist: [
-      {"message": "Type X; Field f1: with @id directive must be of type String!, Int!, Int64! or Float!, not [String]",
+      {"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not [String]",
       "locations":[{"line":2, "column":17}]}
       ]
 
+  -
+    name: "@id directive can't be applied on field with Float type"
+    input: |
+      type X {
+        f1: Float! @id
+      }
+    errlist: [
+      {"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not Float!",
+      "locations":[{"line":2, "column":15}]}
+      ]
+
   -
     name: "Field with @id directive should be mandatory"
     input: |
       type X {
         f1: String @id
       }
     errlist: [
-      {"message": "Type X; Field f1: with @id directive must be of type String!, Int!, Int64! or Float!, not String",
+      {"message": "Type X; Field f1: with @id directive must be of type String!, Int! or Int64!, not String",
       "locations":[{"line":2, "column":15}]}
       ]
 

diff --git a/graphql/schema/rules.go b/graphql/schema/rules.go
@@ -2048,13 +2048,12 @@ func idValidation(sch *ast.Schema,
 	secrets map[string]x.SensitiveByteSlice) gqlerror.List {
 	if field.Type.String() == "String!" ||
 		field.Type.String() == "Int!" ||
-		field.Type.String() == "Int64!" ||
-		field.Type.String() == "Float!" {
+		field.Type.String() == "Int64!" {
 		return nil
 	}
 	return []*gqlerror.Error{gqlerror.ErrorPosf(
 		dir.Position,
-		"Type %s; Field %s: with @id directive must be of type String!, Int!, Int64! or Float!, not %s",
+		"Type %s; Field %s: with @id directive must be of type String!, Int! or Int64!, not %s",
 		typ.Name, field.Name, field.Type.String())}
 }
 

diff --git a/graphql/schema/schemagen.go b/graphql/schema/schemagen.go
@@ -655,8 +655,6 @@ func genDgSchema(gqlSch *ast.Schema, definitions []string,
 						switch f.Type.Name() {
 						case "Int", "Int64":
 							indexes = append(indexes, "int")
-						case "Float":
-							indexes = append(indexes, "float")
 						case "String", "ID":
 							if !x.HasString(indexes, "exact") {
 								indexes = append(indexes, "hash")

diff --git a/graphql/schema/wrappers.go b/graphql/schema/wrappers.go
@@ -1396,8 +1396,6 @@ func (f *field) IDArgValue() (xids map[string]string, uid uint64, err error) {
 			switch v := f.ArgValue(xidArgName).(type) {
 			case int64:
 				xidArgVal = strconv.FormatInt(v, 10)
-			case float64:
-				xidArgVal = strconv.FormatFloat(v, 'f', -1, 64)
 			case string:
 				xidArgVal = v
 			default: