Feat(GraphQL): This PR allows @id field in interface to be unique acr…

…oss all the implementing types. (#7710)

Currently, @id fields in the interface are unique only in one implementing type. But there are several use cases that require @id field to be unique across all the implementation types. Also currently. get a query on the interface can result in unexpected errors as we can have multiple implementing types have the same value for that @id field.

Now we are allowing the @id field in the interface to be unique across all the implementing types. In order to do this, we have added a new argument interface of boolean type in the @id field.

Whenever a @id field in interface type has an interface argument set then its value will be unique across all the implementing types. Users will get errors if they try to add a node with such a field and there is already a node with the same value of that field even in some other implementing types. This is true for other scenarios like adding nested value or while using upserts.

If the interface argument is not present or its value is false then that field will be unique only for one implementing type.
But such fields won't be allowed in argument to get query on interface in the future, see this PR also #7602

Example Schema,

 interface LibraryItem {
    refID: String! @id                   //  This field is unique only for one implementing type
    itemID: String! @id(interface:true)    // This field will be unique over all the implementing types inheriting this interface
}

type Book implements LibraryItem {
    title: String
    author: String
}
Related discuss Post: https://discuss.dgraph.io/t/uniqueness-for-id-fields-on-interface/13294

graphql/e2e/auth/add_mutation_test.go

@@ -1035,7 +1035,7 @@ func TestUpsertMutationsWithRBAC(t *testing.T) {
 				require.Error(t, gqlResponse.Errors)
 				require.Equal(t, len(gqlResponse.Errors), 1)
 				require.Contains(t, gqlResponse.Errors[0].Error(),
-					"GraphQL debug: id tweet1 already exists for field id inside type Tweets")
+					" GraphQL debug: id Tweets already exists for field id inside type tweet1")
 			} else {
 				common.RequireNoGQLErrors(t, gqlResponse)
 				require.JSONEq(t, tcase.result, string(gqlResponse.Data))
@@ -1152,3 +1152,48 @@ func TestUpsertWithDeepAuth(t *testing.T) {
 	filter = map[string]interface{}{"code": map[string]interface{}{"eq": "UK"}}
 	common.DeleteGqlType(t, "State", filter, 1, nil)
 }
+
+func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
+	// add Library Member
+	addLibraryMemberParams := &common.GraphQLParams{
+		Query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
+                         addLibraryMember(input: $input, upsert: false) {
+                          numUids
+                         }
+                        }`,
+		Variables: map[string]interface{}{"input": []interface{}{
+			map[string]interface{}{
+				"refID":     "101",
+				"name":      "Alice",
+				"readHours": "4d2hr",
+			}},
+		},
+	}
+
+	gqlResponse := addLibraryMemberParams.ExecuteAsPost(t, common.GraphqlURL)
+	common.RequireNoGQLErrors(t, gqlResponse)
+	// add sports member should return error but in debug mode
+	// because interface type have auth rules defined on it
+	addSportsMemberParams := &common.GraphQLParams{
+		Query: `mutation addSportsMember($input: [AddSportsMemberInput!]!) {
+                         addSportsMember(input: $input, upsert: false) {
+                          numUids
+                         }
+                        }`,
+		Variables: map[string]interface{}{"input": []interface{}{
+			map[string]interface{}{
+				"refID": "101",
+				"name":  "Bob",
+				"plays": "football and cricket",
+			}},
+		},
+	}
+
+	gqlResponse = addSportsMemberParams.ExecuteAsPost(t, common.GraphqlURL)
+	require.Contains(t, gqlResponse.Errors[0].Error(),
+		" GraphQL debug: id 101 already exists for field refID in some other"+
+			" implementing type of interface Member")
+
+	// cleanup
+	common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
+}

graphql/e2e/auth/auth_test.go

@@ -352,7 +352,7 @@ func TestAddMutationWithXid(t *testing.T) {
 	require.Error(t, gqlResponse.Errors)
 	require.Equal(t, len(gqlResponse.Errors), 1)
 	require.Contains(t, gqlResponse.Errors[0].Error(),
-		"GraphQL debug: id tweet1 already exists for field id inside type Tweets")
+		"GraphQL debug: id Tweets already exists for field id inside type tweet1")
 
 	// Clear the tweet.
 	tweet.DeleteByID(t, user, metaInfo)

graphql/e2e/auth/debug_off/debugoff_test.go

@@ -124,6 +124,68 @@ func TestAddMutationWithXid(t *testing.T) {
 	tweet.DeleteByID(t, user, metaInfo)
 }
 
+func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
+
+	// add Library Member
+	addLibraryMemberParams := &common.GraphQLParams{
+		Query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
+                         addLibraryMember(input: $input, upsert: false) {
+                          numUids
+                         }
+                        }`,
+		Variables: map[string]interface{}{"input": []interface{}{
+			map[string]interface{}{
+				"refID":     "101",
+				"name":      "Alice",
+				"readHours": "4d2hr",
+			}},
+		},
+	}
+
+	gqlResponse := addLibraryMemberParams.ExecuteAsPost(t, common.GraphqlURL)
+	common.RequireNoGQLErrors(t, gqlResponse)
+
+	// add SportsMember should return error but in debug mode
+	// because interface type have auth rules defined on it
+	var resultLibraryMember struct {
+		AddLibraryMember struct {
+			NumUids int
+		}
+	}
+	err := json.Unmarshal(gqlResponse.Data, &resultLibraryMember)
+	require.NoError(t, err)
+	require.Equal(t, 1, resultLibraryMember.AddLibraryMember.NumUids)
+
+	addSportsMemberParams := &common.GraphQLParams{
+		Query: `mutation addSportsMember($input: [AddSportsMemberInput!]!) {
+                         addSportsMember(input: $input, upsert: false) {
+                          numUids
+                         }
+                        }`,
+		Variables: map[string]interface{}{"input": []interface{}{
+			map[string]interface{}{
+				"refID": "101",
+				"name":  "Bob",
+				"plays": "football and cricket",
+			}},
+		},
+	}
+
+	gqlResponse = addSportsMemberParams.ExecuteAsPost(t, common.GraphqlURL)
+	common.RequireNoGQLErrors(t, gqlResponse)
+	var resultSportsMember struct {
+		AddSportsMember struct {
+			NumUids int
+		}
+	}
+	err = json.Unmarshal(gqlResponse.Data, &resultSportsMember)
+	require.NoError(t, err)
+	require.Equal(t, 0, resultSportsMember.AddSportsMember.NumUids)
+
+	// cleanup
+	common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
+}
+
 func TestMain(m *testing.M) {
 	schemaFile := "../schema.graphql"
 	schema, err := ioutil.ReadFile(schemaFile)

graphql/e2e/auth/schema.graphql

@@ -848,6 +848,25 @@ type State @auth(
     country: Country
 }
 
+interface Member @auth(
+    query: { rule: "{$ROLE: { eq: \"ADMIN\" } }" },
+){
+    refID: String! @id (interface:true)
+    name:  String! @id (interface:false)
+}
+
+type SportsMember implements Member {
+    plays: String
+    playerRating: Int
+}
+
+type LibraryMember implements Member {
+    interests: [String]
+    readHours: String
+}
+
+
+
 # union testing - start
 enum AnimalCategory {
     Fish

graphql/e2e/common/common.go

@@ -837,6 +837,7 @@ func RunAll(t *testing.T) {
 	t.Run("query id directive with int64", idDirectiveWithInt64)
 	t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)
 	t.Run("query multiple language Fields", queryMultipleLangFields)
+	t.Run("query @id field with interface arg on interface", queryWithIDFieldAndInterfaceArg)
 
 	// mutation tests
 	t.Run("add mutation", addMutation)
@@ -897,6 +898,7 @@ func RunAll(t *testing.T) {
 	t.Run("multiple external Id's tests", multipleXidsTests)
 	t.Run("Upsert Mutation Tests", upsertMutationTests)
 	t.Run("Update language tag fields", updateLangTagFields)
+	t.Run("add mutation with @id field and interface arg", addMutationWithIDFieldHavingInterfaceArg)
 
 	// error tests
 	t.Run("graphql completion on", graphQLCompletionOn)

graphql/e2e/common/mutation.go

@@ -6086,3 +6086,206 @@ func updateLangTagFields(t *testing.T) {
 	testutil.CompareJSON(t, queryPersonExpected, string(gqlResponse.Data))
 	DeleteGqlType(t, "Person", map[string]interface{}{}, 1, nil)
 }
+
+func addMutationWithIDFieldHavingInterfaceArg(t *testing.T) {
+
+	// add data successfully for different implementing types
+	tcases := []struct {
+		name      string
+		query     string
+		variables string
+		error     string
+	}{
+		{
+			name: "adding new Library member shouldn't return any error",
+			query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
+                      addLibraryMember(input: $input, upsert: false) {
+                        libraryMember {
+                          refID
+                        }
+                      }
+                    }`,
+			variables: `{
+                          "input": {
+                              "refID": "101",
+                              "name": "Alice",
+                              "itemsIssued": [
+                                  "Intro to Go",
+                                  "Parallel Programming"
+                              ],
+                              "readHours": "4d2hr"
+                          }
+                       }`,
+		}, {
+			name: "update existing library member using upsert shouldn't return any error",
+			query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
+                      addLibraryMember(input: $input, upsert: true) {
+                        libraryMember {
+                          refID
+                        }
+                      }
+                    }`,
+			variables: `{
+                          "input": {
+                              "refID": "101",
+                              "name": "Alice",
+                              "itemsIssued": [
+                                  "Intro to Go",
+                                  "Parallel Programming",
+                                  "Computer Architecture"
+                              ],
+                              "readHours": "5d3hr"
+                          }
+						}`,
+		}, {
+			name: "adding new Sports Member shouldn't return any error",
+			query: `mutation addSportsMember($input: [AddSportsMemberInput!]!) {
+                      addSportsMember(input: $input, upsert: false) {
+                        sportsMember {
+                          refID
+                        }
+                      }
+                    }`,
+			variables: `{
+                          "input": {
+                              "refID": "102",
+                              "name": "Bob",
+                              "teamID": "T01",
+                              "teamName": "GraphQL",
+                              "itemsIssued": [
+                                  "2-Bats",
+                                  "1-football"
+                              ],
+                              "plays": "football and cricket"
+                          }
+						}`,
+		}, {
+			name: "adding new Cricket Team shouldn't return any error",
+			query: `mutation addCricketTeam($input: [AddCricketTeamInput!]!) {
+                      addCricketTeam(input: $input, upsert: false) {
+                        cricketTeam {
+                          teamID
+                        }
+                      }
+					}`,
+			variables: `{
+                          "input": {
+                              "teamID": "T02",
+                              "teamName": "Dgraph",
+                              "numOfBatsmen": 5,
+                              "numOfBowlers": 3
+                          }
+						}`,
+		}, {
+			name: "add new LibraryManager,linking to existing library Member",
+			query: `mutation addLibraryManager($input: [AddLibraryManagerInput!]!) {
+                      addLibraryManager(input: $input, upsert: false) {
+                        libraryManager {
+                          name
+                        }
+                      }
+                   }`,
+			variables: `{
+                           "input": {
+                               "name": "Juliet",
+                               "manages": {
+                                   "refID": "101"
+                               }
+                           }
+                       }`,
+		}, {
+			name: "adding new Library member returns error as given id already exist in other node of type" +
+				" SportsMember which implements same interface",
+			query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
+                       addLibraryMember(input: $input, upsert: false) {
+                         libraryMember {
+                           refID
+                         }
+                       }
+                     }`,
+			variables: `{
+                         "input": {
+                             "refID": "102",
+                             "name": "James",
+                             "itemsIssued": [
+                                 "Intro to C"
+                             ],
+                             "readHours": "1d2hr"
+                         }
+                     }`,
+			error: "couldn't rewrite mutation addLibraryMember because failed to rewrite mutation" +
+				" payload because id 102 already exists for field refID in some other implementing" +
+				" type of interface Member",
+		}, {
+			name: "adding new Cricket Team with upsert returns error as given id already exist" +
+				" in other node of type SportsMember which implements same interface",
+			query: `mutation addCricketTeam($input: [AddCricketTeamInput!]!) {
+                      addCricketTeam(input: $input, upsert: true) {
+                        cricketTeam {
+                          teamID
+                        }
+                      }
+                    }`,
+			variables: `{
+                         "input": {
+                             "teamID": "T01",
+                             "teamName": "Slash",
+                             "numOfBatsmen": 5,
+                             "numOfBowlers": 4
+                         }
+                     }`,
+			error: "couldn't rewrite mutation addCricketTeam because failed to rewrite mutation" +
+				" payload because id T01 already exists for field teamID in some other" +
+				" implementing type of interface Team",
+		}, {
+			name: "adding new Library manager returns error when it try to links to LibraryMember" +
+				" but got id of some other implementing type which implements " +
+				"same interface as LibraryMember",
+			query: `mutation addLibraryManager($input: [AddLibraryManagerInput!]!) {
+                      addLibraryManager(input: $input, upsert: false) {
+                        libraryManager {
+                          name
+                        }
+                      }
+                    }`,
+			variables: `{
+                          "input": {
+                              "name": "John",
+                              "manages": {
+                                  "refID": "102"
+                              }
+                          }
+                       }`,
+			error: "couldn't rewrite mutation addLibraryManager because failed to rewrite mutation" +
+				" payload because id 102 already exists for field refID in some other implementing" +
+				" type of interface Member",
+		},
+	}
+
+	for _, tcase := range tcases {
+		t.Run(tcase.name, func(t *testing.T) {
+			var vars map[string]interface{}
+			if tcase.variables != "" {
+				err := json.Unmarshal([]byte(tcase.variables), &vars)
+				require.NoError(t, err)
+			}
+			params := &GraphQLParams{
+				Query:     tcase.query,
+				Variables: vars,
+			}
+			resp := params.ExecuteAsPost(t, GraphqlURL)
+			if tcase.error != "" {
+				require.Equal(t, tcase.error, resp.Errors[0].Error())
+			} else {
+				RequireNoGQLErrors(t, resp)
+			}
+
+		})
+	}
+
+	// Cleanup
+	DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
+	DeleteGqlType(t, "SportsMember", map[string]interface{}{}, 1, nil)
+	DeleteGqlType(t, "CricketTeam", map[string]interface{}{}, 1, nil)
+	DeleteGqlType(t, "LibraryManager", map[string]interface{}{}, 1, nil)
+}