diff --git a/ee/acl/acl_test.go b/ee/acl/acl_test.go index a06258a9145..fcf2c07c5b7 100644 --- a/ee/acl/acl_test.go +++ b/ee/acl/acl_test.go @@ -86,7 +86,7 @@ func checkUserCount(t *testing.T, resp []byte, expected int) { require.Equal(t, expected, len(r.AddUser.User)) } -func deleteUser(t *testing.T, accessToken, username string) { +func deleteUser(t *testing.T, accessToken, username string, confirmDeletion bool) { // TODO - Verify that only one uid got deleted once numUids are returned as part of the payload. delUser := ` mutation deleteUser($name: String!) { @@ -104,7 +104,9 @@ func deleteUser(t *testing.T, accessToken, username string) { resp := makeRequest(t, accessToken, params) resp.RequireNoGraphQLErrors(t) - require.JSONEq(t, `{"deleteUser":{"msg":"Deleted"}}`, string(resp.Data)) + if confirmDeletion { + require.JSONEq(t, `{"deleteUser":{"msg":"Deleted"}}`, string(resp.Data)) + } } func deleteGroup(t *testing.T, accessToken, name string) { @@ -164,20 +166,14 @@ func TestPasswordReturn(t *testing.T) { } func TestGetCurrentUser(t *testing.T) { - accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{ - Endpoint: adminEndpoint, - UserID: "groot", - Passwd: "password", - }) - require.NoError(t, err, "login failed") - + accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint) currentUser := getCurrentUser(t, accessJwt) currentUser.RequireNoGraphQLErrors(t) require.Equal(t, string(currentUser.Data), `{"getCurrentUser":{"name":"groot"}}`) // clean up the user to allow repeated running of this test userid := "hamilton" - deleteUser(t, accessJwt, userid) + deleteUser(t, accessJwt, userid, false) glog.Infof("cleaned up db user state") resp := createUser(t, accessJwt, userid, userpassword) @@ -197,23 +193,11 @@ func TestGetCurrentUser(t *testing.T) { } func TestCreateAndDeleteUsers(t *testing.T) { - accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{ - Endpoint: adminEndpoint, - UserID: "groot", - Passwd: "password", - }) - require.NoError(t, err, "login failed") - - // clean up the user to allow repeated running of this test - deleteUser(t, accessJwt, userid) - glog.Infof("cleaned up db user state") - - resp := createUser(t, accessJwt, userid, userpassword) - resp.RequireNoGraphQLErrors(t) - checkUserCount(t, resp.Data, 1) + resetUser(t) // adding the user again should fail - resp = createUser(t, accessJwt, userid, userpassword) + accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint) + resp := createUser(t, accessJwt, userid, userpassword) require.Equal(t, x.GqlErrorList{{ Message: "couldn't rewrite query for mutation addUser because id alice already exists" + " for type User", @@ -221,7 +205,7 @@ func TestCreateAndDeleteUsers(t *testing.T) { checkUserCount(t, resp.Data, 0) // delete the user - deleteUser(t, accessJwt, userid) + deleteUser(t, accessJwt, userid, true) resp = createUser(t, accessJwt, userid, userpassword) resp.RequireNoGraphQLErrors(t) @@ -230,15 +214,10 @@ func TestCreateAndDeleteUsers(t *testing.T) { } func resetUser(t *testing.T) { - accessJwt, _, err := testutil.HttpLogin(&testutil.LoginParams{ - Endpoint: adminEndpoint, - UserID: "groot", - Passwd: "password", - }) - require.NoError(t, err, "login failed") + accessJwt, _ := testutil.GrootHttpLogin(adminEndpoint) // clean up the user to allow repeated running of this test - deleteUser(t, accessJwt, userid) + deleteUser(t, accessJwt, userid, false) glog.Infof("deleted user") resp := createUser(t, accessJwt, userid, userpassword) @@ -1599,7 +1578,7 @@ func TestDeleteUserShouldDeleteUserFromGroup(t *testing.T) { }) require.NoError(t, err, "login failed") - deleteUser(t, accessJwt, userid) + deleteUser(t, accessJwt, userid, true) gqlQuery := ` query { diff --git a/graphql/e2e/auth/auth_test.go b/graphql/e2e/auth/auth_test.go index 9f415696676..497e46300ca 100644 --- a/graphql/e2e/auth/auth_test.go +++ b/graphql/e2e/auth/auth_test.go @@ -566,7 +566,7 @@ func TestDeleteAuthRule(t *testing.T) { "anyofterms": "Sensitive information", }, }, - result: `{"deleteUserSecret":{"msg":"Deleted","numUids":0}}`, + result: `{"deleteUserSecret":{"msg":"No nodes were deleted","numUids":0}}`, }, } query := ` @@ -666,7 +666,7 @@ func TestDeleteDeepAuthRule(t *testing.T) { "anyofterms": "Ticket2", }, }, - result: `{"deleteTicket":{"msg":"Deleted","numUids":0}}`, + result: `{"deleteTicket":{"msg":"No nodes were deleted","numUids":0}}`, }, { name: "ticket with edit permission", diff --git a/graphql/e2e/common/mutation.go b/graphql/e2e/common/mutation.go index c0c5f30d625..5fc6e0e33e9 100644 --- a/graphql/e2e/common/mutation.go +++ b/graphql/e2e/common/mutation.go @@ -381,6 +381,7 @@ func deepMutationsTest(t *testing.T, executeRequest requestExecutor) { name } posts { + postID title text tags @@ -429,7 +430,7 @@ func deepMutationsTest(t *testing.T, executeRequest requestExecutor) { cleanUp(t, []*country{newCountry, anotherCountry}, []*author{newAuth}, - []*post{newAuth.Posts[0], newAuth.Posts[0], patchSet.Posts[0]}) + []*post{newAuth.Posts[0], newAuth.Posts[1], result.UpdateAuthor.Author[0].Posts[1]}) } func testMultipleMutations(t *testing.T) { @@ -1073,9 +1074,8 @@ func deleteMutationWithMultipleIds(t *testing.T) { country := addCountry(t, postExecutor) anotherCountry := addCountry(t, postExecutor) t.Run("delete Country", func(t *testing.T) { - deleteCountryExpected := `{"deleteCountry" : { "msg": "Deleted" } }` filter := map[string]interface{}{"id": []string{country.ID, anotherCountry.ID}} - deleteCountry(t, filter, deleteCountryExpected, nil) + deleteCountry(t, filter, 2, nil) }) t.Run("check Country is deleted", func(t *testing.T) { @@ -1088,9 +1088,8 @@ func deleteMutationWithSingleID(t *testing.T) { newCountry := addCountry(t, postExecutor) anotherCountry := addCountry(t, postExecutor) t.Run("delete Country", func(t *testing.T) { - deleteCountryExpected := `{"deleteCountry" : { "msg": "Deleted" } }` filter := map[string]interface{}{"id": []string{newCountry.ID}} - deleteCountry(t, filter, deleteCountryExpected, nil) + deleteCountry(t, filter, 1, nil) }) // In this case anotherCountry shouldn't be deleted. @@ -1110,14 +1109,13 @@ func deleteMutationByName(t *testing.T) { } updateCountry(t, filter, anotherCountry.Name, true) - deleteCountryExpected := `{"deleteCountry" : { "msg": "Deleted" } }` t.Run("delete Country", func(t *testing.T) { filter := map[string]interface{}{ "name": map[string]interface{}{ "regexp": "/" + newCountry.Name + "/", }, } - deleteCountry(t, filter, deleteCountryExpected, nil) + deleteCountry(t, filter, 1, nil) }) // In this case anotherCountry shouldn't be deleted. @@ -1437,7 +1435,7 @@ func deleteMutationSingleReference(t *testing.T, executeRequest requestExecutor) require.NoError(t, err) filter := map[string]interface{}{"id": []string{addResult.AddCountry.Country[0].ID}} - deleteCountry(t, filter, `{"deleteCountry" : { "msg": "Deleted" } }`, nil) + deleteCountry(t, filter, 1, nil) // the state doesn't belong to a country getCatParams := &GraphQLParams{ @@ -1484,7 +1482,7 @@ func deleteMutationMultipleReferences(t *testing.T, executeRequest requestExecut }} requireAuthor(t, newAuthor.ID, newAuthor, executeRequest) - deletePost(t, newPost.PostID, `{"deletePost" : { "msg": "Deleted" } }`, nil) + deletePost(t, newPost.PostID, 1, nil) // the post isn't in the author's list of posts newAuthor.Posts = []*post{} @@ -1511,72 +1509,27 @@ func deleteMutationMultipleReferences(t *testing.T, executeRequest requestExecut func deleteCountry( t *testing.T, filter map[string]interface{}, - deleteCountryExpected string, + expectedNumUids int, expectedErrors x.GqlErrorList) { - - deleteCountryParams := &GraphQLParams{ - Query: `mutation deleteCountry($filter: CountryFilter!) { - deleteCountry(filter: $filter) { msg } - }`, - Variables: map[string]interface{}{"filter": filter}, - } - - gqlResponse := deleteCountryParams.ExecuteAsPost(t, graphqlURL) - require.JSONEq(t, deleteCountryExpected, string(gqlResponse.Data)) - - if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" { - t.Errorf("errors mismatch (-want +got):\n%s", diff) - } + deleteGqlType(t, "Country", filter, expectedNumUids, expectedErrors) } func deleteAuthor( t *testing.T, authorID string, - deleteAuthorExpected string, + expectedNumUids int, expectedErrors x.GqlErrorList) { - - deleteAuthorParams := &GraphQLParams{ - Query: `mutation deleteAuthor($filter: AuthorFilter!) { - deleteAuthor(filter: $filter) { msg } - }`, - Variables: map[string]interface{}{ - "filter": map[string]interface{}{ - "id": []string{authorID}, - }, - }, - } - - gqlResponse := deleteAuthorParams.ExecuteAsPost(t, graphqlURL) - - require.JSONEq(t, deleteAuthorExpected, string(gqlResponse.Data)) - - if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" { - t.Errorf("errors mismatch (-want +got):\n%s", diff) - } + filter := map[string]interface{}{"id": []string{authorID}} + deleteGqlType(t, "Author", filter, expectedNumUids, expectedErrors) } func deletePost( t *testing.T, postID string, - deletePostExpected string, + expectedNumUids int, expectedErrors x.GqlErrorList) { - - deletePostParams := &GraphQLParams{ - Query: `mutation deletePost($filter: PostFilter!) { - deletePost(filter: $filter) { msg } - }`, - Variables: map[string]interface{}{"filter": map[string]interface{}{ - "postID": []string{postID}, - }}, - } - - gqlResponse := deletePostParams.ExecuteAsPost(t, graphqlURL) - - require.JSONEq(t, deletePostExpected, string(gqlResponse.Data)) - - if diff := cmp.Diff(expectedErrors, gqlResponse.Errors); diff != "" { - t.Errorf("errors mismatch (-want +got):\n%s", diff) - } + filter := map[string]interface{}{"postID": []string{postID}} + deleteGqlType(t, "Post", filter, expectedNumUids, expectedErrors) } func deleteWrongID(t *testing.T) { @@ -1584,7 +1537,7 @@ func deleteWrongID(t *testing.T) { newAuthor := addAuthor(t, newCountry.ID, postExecutor) expectedData := `{ "deleteCountry": { - "msg": "Deleted", + "msg": "No nodes were deleted", "numUids": 0 } }` @@ -1954,16 +1907,16 @@ func manyMutationsWithQueryError(t *testing.T) { func cleanUp(t *testing.T, countries []*country, authors []*author, posts []*post) { t.Run("cleaning up", func(t *testing.T) { for _, post := range posts { - deletePost(t, post.PostID, `{"deletePost" : { "msg": "Deleted" } }`, nil) + deletePost(t, post.PostID, 1, nil) } for _, author := range authors { - deleteAuthor(t, author.ID, `{"deleteAuthor" : { "msg": "Deleted" } }`, nil) + deleteAuthor(t, author.ID, 1, nil) } for _, country := range countries { filter := map[string]interface{}{"id": []string{country.ID}} - deleteCountry(t, filter, `{"deleteCountry" : { "msg": "Deleted" } }`, nil) + deleteCountry(t, filter, 1, nil) } }) } @@ -2295,55 +2248,17 @@ func queryInterfaceAfterAddMutation(t *testing.T) { func cleanupStarwars(t *testing.T, starshipID, humanID, droidID string) { // Delete everything - multiMutationParams := &GraphQLParams{ - Query: `mutation cleanup($starshipFilter: StarshipFilter!, $humanFilter: HumanFilter!, - $droidFilter: DroidFilter!) { - deleteStarship(filter: $starshipFilter) { msg } - - deleteHuman(filter: $humanFilter) { msg } - - deleteDroid(filter: $droidFilter) { msg } - }`, - Variables: map[string]interface{}{ - "starshipFilter": map[string]interface{}{ - "id": []string{starshipID}, - }, - "humanFilter": map[string]interface{}{ - "id": []string{humanID}, - }, - "droidFilter": map[string]interface{}{ - "id": []string{droidID}, - }, - }, + if starshipID != "" { + starshipFilter := map[string]interface{}{"id": []string{starshipID}} + deleteGqlType(t, "Starship", starshipFilter, 1, nil) } - multiMutationExpected := `{ - "deleteStarship": { "msg": "Deleted" }, - "deleteHuman" : { "msg": "Deleted" }, - "deleteDroid": { "msg": "Deleted" } -}` - - gqlResponse := multiMutationParams.ExecuteAsPost(t, graphqlURL) - RequireNoGQLErrors(t, gqlResponse) - - var expected, result struct { - DeleteStarhip struct { - Msg string - } - DeleteHuman struct { - Msg string - } - DeleteDroid struct { - Msg string - } + if humanID != "" { + humanFilter := map[string]interface{}{"id": []string{humanID}} + deleteGqlType(t, "Human", humanFilter, 1, nil) } - - err := json.Unmarshal([]byte(multiMutationExpected), &expected) - require.NoError(t, err) - err = json.Unmarshal([]byte(gqlResponse.Data), &result) - require.NoError(t, err) - - if diff := cmp.Diff(expected, result); diff != "" { - t.Errorf("result mismatch (-want +got):\n%s", diff) + if droidID != "" { + droidFilter := map[string]interface{}{"id": []string{droidID}} + deleteGqlType(t, "Droid", droidFilter, 1, nil) } } @@ -2451,8 +2366,17 @@ func deleteGqlType( deleteField := fmt.Sprintf(`delete%s`, typeName) deleteType := result[deleteField].(map[string]interface{}) - require.Equal(t, "Deleted", deleteType["msg"]) - require.Equal(t, expectedNumUids, int(deleteType["numUids"].(float64))) + gotNumUids := int(deleteType["numUids"].(float64)) + require.Equal(t, expectedNumUids, gotNumUids, + "numUids mismatch while deleting %s (filter: %v) want: %d, got: %d", typeName, filter, + expectedNumUids, gotNumUids) + if expectedNumUids == 0 { + require.Equal(t, "No nodes were deleted", deleteType["msg"], + "while deleting %s (filter: %v)", typeName, filter) + } else { + 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) @@ -2846,8 +2770,8 @@ func testNumUids(t *testing.T) { require.Equal(t, deleteResult.DeletePost.Msg, "Deleted") }) - cleanUp(t, []*country{newCountry}, result.AddAuthor.Author, - result.AddAuthor.Author[0].Posts) + // no need to delete author and posts as they would be already deleted by above test + cleanUp(t, []*country{newCountry}, nil, nil) } func checkUser(t *testing.T, userObj, expectedObj *user) { @@ -2878,21 +2802,7 @@ func checkUser(t *testing.T, userObj, expectedObj *user) { } func deleteUser(t *testing.T, userObj user) { - deletePostParams := &GraphQLParams{ - Query: `mutation deleteUser($filter: UserFilter!) { - deleteUser(filter: $filter) { msg } - }`, - Variables: map[string]interface{}{"filter": map[string]interface{}{ - "name": map[string]interface{}{ - "eq": userObj.Name, - }, - }}, - } - - gqlResponse := deletePostParams.ExecuteAsPost(t, graphqlURL) - - RequireNoGQLErrors(t, gqlResponse) - require.JSONEq(t, `{"deleteUser": {"msg": "Deleted"}}`, string(gqlResponse.Data)) + deleteGqlType(t, "User", getXidFilter("name", []string{userObj.Name}), 1, nil) } func passwordTest(t *testing.T) { diff --git a/graphql/resolve/mutation.go b/graphql/resolve/mutation.go index 3e18f0ce101..60a309b3fcd 100644 --- a/graphql/resolve/mutation.go +++ b/graphql/resolve/mutation.go @@ -257,11 +257,7 @@ func (mr *dgraphResolver) rewriteAndExecute( extQ := &schema.Extensions{TouchedUids: qryResp.GetMetrics().GetNumUids()[touchedUidsKey]} // merge the extensions we got from Mutate and Query into extM - if extM == nil { - extM = extQ - } else { - extM.Merge(extQ) - } + extM.Merge(extQ) numUids := getNumUids(mutation, mutResp.Uids, result) @@ -298,6 +294,9 @@ func deleteCompletion() CompletionFunc { if fld, ok := resolved.Data.(map[string]interface{}); ok { if rsp, ok := fld[resolved.Field.Name()].(map[string]interface{}); ok { rsp["msg"] = "Deleted" + if rsp[schema.NumUid] == 0 { + rsp["msg"] = "No nodes were deleted" + } } } })