From 731f97b2c9d7ba03ff7ffabeeca40b148ef2aeba Mon Sep 17 00:00:00 2001 From: minhaj-shakeel Date: Tue, 16 Mar 2021 17:36:49 +0530 Subject: [PATCH] fix(GraphQL): Fix mutation with Int Xid variables. (#7565) (#7588) (cherry picked from commit eb6e27bc02f66d904da9b45c090037065c6da726) --- graphql/e2e/common/mutation.go | 35 +++++++-- graphql/resolve/mutation_rewriter.go | 102 ++++++++++++++++++--------- 2 files changed, 95 insertions(+), 42 deletions(-) diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go index c20e4ad8073..ac1ea477f5d 100644 --- a/graphql/e2e/common/mutation.go +++ b/graphql/e2e/common/mutation.go @@ -4954,24 +4954,38 @@ func filterInUpdateMutationsWithFilterAndOr(t *testing.T) { func idDirectiveWithInt64Mutation(t *testing.T) { query := &GraphQLParams{ - Query: `mutation { + Query: `mutation addBook($bookId2: Int64!, $bookId3: Int64!){ addBook(input:[ { bookId: 1234567890123 name: "Graphql" desc: "Graphql is the next big thing" - } + }, + { + bookId: $bookId2 + name: "Dgraph" + desc: "A GraphQL database" + }, + { + bookId: $bookId3 + name: "DQL" + desc: "Query Language for Dgraph" + } ]) { numUids } }`, + Variables: map[string]interface{}{ + "bookId2": "1234512345", + "bookId3": 5432154321, + }, } response := query.ExecuteAsPost(t, GraphqlURL) RequireNoGQLErrors(t, response) expected := `{ "addBook": { - "numUids": 1 + "numUids": 3 } }` require.JSONEq(t, expected, string(response.Data)) @@ -4980,26 +4994,33 @@ func idDirectiveWithInt64Mutation(t *testing.T) { response = query.ExecuteAsPost(t, GraphqlURL) require.Contains(t, response.Errors.Error(), "already exists") - DeleteGqlType(t, "Book", map[string]interface{}{}, 2, nil) + DeleteGqlType(t, "Book", map[string]interface{}{}, 4, nil) } func idDirectiveWithIntMutation(t *testing.T) { query := &GraphQLParams{ - Query: `mutation { + Query: `mutation addChapter($chId: Int!){ addChapter(input:[{ chapterId: 2 name: "Graphql and more" + }, + { + chapterId: $chId + name: "Authorization" }]) { numUids } }`, + Variables: map[string]interface{}{ + "chId": 10, + }, } response := query.ExecuteAsPost(t, GraphqlURL) RequireNoGQLErrors(t, response) var expected = `{ "addChapter": { - "numUids": 1 + "numUids": 2 } }` require.JSONEq(t, expected, string(response.Data)) @@ -5008,7 +5029,7 @@ func idDirectiveWithIntMutation(t *testing.T) { response = query.ExecuteAsPost(t, GraphqlURL) require.Contains(t, response.Errors.Error(), "already exists") - DeleteGqlType(t, "Chapter", map[string]interface{}{}, 2, nil) + DeleteGqlType(t, "Chapter", map[string]interface{}{}, 3, nil) } func idDirectiveWithFloatMutation(t *testing.T) { diff --git a/graphql/resolve/mutation_rewriter.go b/graphql/resolve/mutation_rewriter.go index 432c32df7ac..7ee299d94a8 100644 --- a/graphql/resolve/mutation_rewriter.go +++ b/graphql/resolve/mutation_rewriter.go @@ -1406,17 +1406,7 @@ func rewriteObject( for _, xid := range xids { var xidString string if xidVal, ok := obj[xid.Name()]; ok && xidVal != nil { - // TODO: Add a function for parsing idVal. This is repeatitive - switch xid.Type().Name() { - case "Int": - val, _ := xidVal.(int64) - xidString = strconv.FormatInt(val, 10) - case "Float": - val, _ := xidVal.(float64) - xidString = strconv.FormatFloat(val, 'f', -1, 64) - default: - xidString, _ = xidVal.(string) - } + xidString, _ = extractVal(xidVal, xid.Name(), xid.Type().Name()) variable = varGen.Next(typ, xid.Name(), xidString, false) // Three cases: @@ -1750,33 +1740,13 @@ func existenceQueries( xids := typ.XIDFields() var xidString string + var err error if len(xids) != 0 { for _, xid := range xids { if xidVal, ok := obj[xid.Name()]; ok && xidVal != nil { - switch xid.Type().Name() { - case "Int": - val, ok := xidVal.(int64) - if !ok { - retErrors = append(retErrors, errors.New(fmt.Sprintf("encountered an XID %s with %s that isn't "+ - "a Int but data type in schema is Int", xid.Name(), xid.Type().Name()))) - return nil, retErrors - } - xidString = strconv.FormatInt(val, 10) - case "Float": - val, ok := xidVal.(float64) - if !ok { - retErrors = append(retErrors, errors.New(fmt.Sprintf("encountered an XID %s with %s that isn't "+ - "a Float but data type in schema is Float", xid.Name(), xid.Type().Name()))) - return nil, retErrors - } - xidString = strconv.FormatFloat(val, 'f', -1, 64) - default: - xidString, ok = xidVal.(string) - if !ok { - retErrors = append(retErrors, errors.New(fmt.Sprintf("encountered an XID %s with %s that isn't "+ - "a String or Int64", xid.Name(), xid.Type().Name()))) - return nil, retErrors - } + xidString, err = extractVal(xidVal, xid.Name(), xid.Type().Name()) + if err != nil { + return nil, append(retErrors, err) } variable := varGen.Next(typ, xid.Name(), xidString, false) // There are two cases: @@ -2313,3 +2283,65 @@ func copyTypeMap(from, to map[string]schema.Type) { to[name] = typ } } + +func extractVal(xidVal interface{}, xidName, typeName string) (string, error) { + fmt.Println(typeName) + switch typeName { + case "Int": + switch xVal := xidVal.(type) { + case json.Number: + val, err := xVal.Int64() + if err != nil { + return "", err + } + return strconv.FormatInt(val, 10), nil + case int64: + return strconv.FormatInt(xVal, 10), nil + default: + return "", fmt.Errorf("encountered an XID %s with %s that isn't "+ + "a Int but data type in schema is Int", xidName, typeName) + } + case "Int64": + switch xVal := xidVal.(type) { + case json.Number: + val, err := xVal.Int64() + if err != nil { + return "", err + } + return strconv.FormatInt(val, 10), nil + case int64: + return strconv.FormatInt(xVal, 10), nil + // If the xid field is of type Int64, both String and Int forms are allowed. + case string: + return xVal, nil + default: + 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) + if !ok { + return "", fmt.Errorf("encountered an XID %s with %s that isn't "+ + "a String", xidName, typeName) + } + return xidString, nil + default: + return "", fmt.Errorf("encountered an XID %s with %s that isn't"+ + "allowed as Xid", xidName, typeName) + } +}