From 8adf4d39fc722d6c5e996e963b43872c59c3f8b9 Mon Sep 17 00:00:00 2001 From: Aman Mangal Date: Fri, 22 Nov 2019 13:09:27 +0530 Subject: [PATCH] Fix JSON API for multiple mutations --- dgraph/cmd/alpha/http.go | 58 +++++-- dgraph/cmd/alpha/upsert_test.go | 261 +++++++++++++++++++++++++++++++- 2 files changed, 305 insertions(+), 14 deletions(-) diff --git a/dgraph/cmd/alpha/http.go b/dgraph/cmd/alpha/http.go index 40c39fe2bb1..f4aa91313c3 100644 --- a/dgraph/cmd/alpha/http.go +++ b/dgraph/cmd/alpha/http.go @@ -318,14 +318,7 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) { return } - mu := &api.Mutation{} - req = &api.Request{Mutations: []*api.Mutation{mu}} - if setJSON, ok := ms["set"]; ok && setJSON != nil { - mu.SetJson = setJSON.bs - } - if delJSON, ok := ms["delete"]; ok && delJSON != nil { - mu.DeleteJson = delJSON.bs - } + req = &api.Request{} if queryText, ok := ms["query"]; ok && queryText != nil { req.Query, err = strconv.Unquote(string(queryText.bs)) if err != nil { @@ -333,12 +326,53 @@ func mutationHandler(w http.ResponseWriter, r *http.Request) { return } } - if condText, ok := ms["cond"]; ok && condText != nil { - mu.Cond, err = strconv.Unquote(string(condText.bs)) - if err != nil { - x.SetStatus(w, x.ErrorInvalidRequest, err.Error()) + + extractMutation := func(jsMap map[string]*skipJSONUnmarshal) (*api.Mutation, error) { + mu := &api.Mutation{} + empty := true + if setJSON, ok := jsMap["set"]; ok && setJSON != nil { + empty = false + mu.SetJson = setJSON.bs + } + if delJSON, ok := jsMap["delete"]; ok && delJSON != nil { + empty = false + mu.DeleteJson = delJSON.bs + } + if condText, ok := jsMap["cond"]; ok && condText != nil { + mu.Cond, err = strconv.Unquote(string(condText.bs)) + if err != nil { + return nil, err + } + } + + if empty { + return nil, nil + } + + return mu, nil + } + if mu, err := extractMutation(ms); err != nil { + x.SetStatus(w, x.ErrorInvalidRequest, err.Error()) + return + } else if mu != nil { + req.Mutations = append(req.Mutations, mu) + } + if mus, ok := ms["mutations"]; ok && mus != nil { + var mm []map[string]*skipJSONUnmarshal + if err := json.Unmarshal(mus.bs, &mm); err != nil { + jsonErr := convertJSONError(string(mus.bs), err) + x.SetStatus(w, x.ErrorInvalidRequest, jsonErr.Error()) return } + + for _, m := range mm { + if mu, err := extractMutation(m); err != nil { + x.SetStatus(w, x.ErrorInvalidRequest, err.Error()) + return + } else if mu != nil { + req.Mutations = append(req.Mutations, mu) + } + } } case "application/rdf": diff --git a/dgraph/cmd/alpha/upsert_test.go b/dgraph/cmd/alpha/upsert_test.go index 07b7bb383e3..59208030d44 100644 --- a/dgraph/cmd/alpha/upsert_test.go +++ b/dgraph/cmd/alpha/upsert_test.go @@ -2426,8 +2426,34 @@ upsert { require.NoError(t, err) testutil.CompareJSON(t, res, expectedRes) - // second time - mr, err = mutationWithTs(m1, "application/rdf", false, true, 0) + // second time, using Json mutation + m1Json := ` +{ + "query": "{q(func: eq(email, \"email@company.io\")) {v as uid\n c as count\n nc as math(c+1)}}", + "mutations": [ + { + "set": [ + { + "uid": "uid(v)", + "name": "name", + "email": "email@company.io", + "count": "1" + } + ], + "cond": "@if(eq(len(v), 0))" + }, + { + "set": [ + { + "uid": "uid(v)", + "count": "val(nc)" + } + ], + "cond": "@if(not(eq(len(v), 0)))" + } + ] +}` + mr, err = mutationWithTs(m1Json, "application/json", false, true, 0) require.NoError(t, err) require.True(t, len(mr.keys) == 0) require.Equal(t, []string{"1-count"}, mr.preds) @@ -2555,3 +2581,234 @@ upsert { require.True(t, len(mr.keys) == 0) require.Equal(t, 0, len(mr.preds)) } + +func TestJsonOldAndNewAPI(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +{ + "query": "{q(func: eq(works_for, \"company1\")) {u as uid}}", + "set": [ + { + "uid": "uid(u)", + "color": "red" + } + ], + "cond": "@if(gt(len(u), 0))", + "mutations": [ + { + "set": [ + { + "uid": "uid(u)", + "works_with": { + "uid": "0x01" + } + } + ], + "cond": "@if(gt(len(u), 0))" + } + ] +}` + mr, err := mutationWithTs(m1, "application/json", false, true, 0) + require.NoError(t, err) + require.True(t, len(mr.keys) == 0) + require.Equal(t, []string{"1-color", "1-works_with"}, mr.preds) + result := QueryResult{} + require.NoError(t, json.Unmarshal(mr.data, &result)) + require.Equal(t, 2, len(result.Q)) + q2 := ` + { + q(func: eq(works_for, "%s")) { + name + works_for + color + works_with { + uid + } + } + }` + res, _, err := queryWithTs(fmt.Sprintf(q2, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, ` + { + "data": { + "q": [ + { + "name": "user1", + "works_for": "company1", + "color": "red", + "works_with": [ + { + "uid": "0x1" + } + ] + }, + { + "name": "user2", + "works_for": "company1", + "color": "red", + "works_with": [ + { + "uid": "0x1" + } + ] + } + ] + } + }`, res) +} + +func TestJsonNewAPI(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + m1 := ` +{ + "query": "{q(func: eq(works_for, \"company1\")) {u as uid}}", + "mutations": [ + { + "set": [ + { + "uid": "uid(u)", + "works_with": { + "uid": "0x01" + } + } + ], + "cond": "@if(gt(len(u), 0))" + }, + { + "set": [ + { + "uid": "uid(u)", + "color": "red" + } + ], + "cond": "@if(gt(len(u), 0))" + } + ] +}` + mr, err := mutationWithTs(m1, "application/json", false, true, 0) + require.NoError(t, err) + require.True(t, len(mr.keys) == 0) + require.Equal(t, []string{"1-color", "1-works_with"}, mr.preds) + result := QueryResult{} + require.NoError(t, json.Unmarshal(mr.data, &result)) + require.Equal(t, 2, len(result.Q)) + q2 := ` + { + q(func: eq(works_for, "%s")) { + name + works_for + color + works_with { + uid + } + } + }` + res, _, err := queryWithTs(fmt.Sprintf(q2, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, ` + { + "data": { + "q": [ + { + "name": "user1", + "works_for": "company1", + "color": "red", + "works_with": [ + { + "uid": "0x1" + } + ] + }, + { + "name": "user2", + "works_for": "company1", + "color": "red", + "works_with": [ + { + "uid": "0x1" + } + ] + } + ] + } + }`, res) +} + +func TestUpsertMultiValueJson(t *testing.T) { + require.NoError(t, dropAll()) + populateCompanyData(t) + + // add color to all employees of company1 + m2 := ` +{ + "query": "{q(func: eq(works_for, \"company1\")) {u as uid}}", + "mutations": [ + { + "set": [ + { + "uid": "uid(u)", + "color": "red" + } + ], + "cond": "@if(gt(len(u), 0))" + } + ] +}` + + mr, err := mutationWithTs(m2, "application/json", false, true, 0) + require.NoError(t, err) + require.True(t, len(mr.keys) == 0) + require.Equal(t, []string{"1-color"}, mr.preds) + result := QueryResult{} + require.NoError(t, json.Unmarshal(mr.data, &result)) + require.Equal(t, 2, len(result.Q)) + q2 := ` +{ + q(func: eq(works_for, "%s")) { + name + works_for + color + works_with + } +}` + res, _, err := queryWithTs(fmt.Sprintf(q2, "company1"), "application/graphql+-", "", 0) + require.NoError(t, err) + testutil.CompareJSON(t, `{"data":{"q":[{"name":"user1","works_for":"company1","color":"red"},`+ + `{"name":"user2","works_for":"company1","color":"red"}]}}`, res) + + // delete color for employess of company1 and set color for employees of company2 + m3 := ` +{ + "query": "{user1(func: eq(works_for, \"company1\")) {c1 as uid} user2(func: eq(works_for, \"company2\")) {c2 as uid}}", + "mutations": [ + { + "delete": [ + { + "uid": "uid(c1)", + "color": null + } + ], + "cond": "@if(le(len(c1), 100) AND lt(len(c2), 100))" + }, + { + "set": [ + { + "uid": "uid(c2)", + "color": "blue" + } + ], + "cond": "@if(le(len(c1), 100) AND lt(len(c2), 100))" + } + ] +}` + mr, err = mutationWithTs(m3, "application/json", false, true, 0) + require.NoError(t, err) + result = QueryResult{} + require.NoError(t, json.Unmarshal(mr.data, &result)) + require.Equal(t, 2, len(result.User1)) + require.Equal(t, 2, len(result.User2)) +}