Skip to content

Commit

Permalink
Feat(GraphQL): This PR allows @id field in interface to be unique acr…
Browse files Browse the repository at this point in the history
…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
  • Loading branch information
JatinDev543 authored and Harshil Goel committed Jun 14, 2023
1 parent 845593a commit 4ea4bb5
Show file tree
Hide file tree
Showing 78 changed files with 2,572 additions and 564 deletions.
47 changes: 46 additions & 1 deletion graphql/e2e/auth/add_mutation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1002,7 +1002,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))
Expand Down Expand Up @@ -1119,3 +1119,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)
}
2 changes: 1 addition & 1 deletion graphql/e2e/auth/auth_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,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)
Expand Down
62 changes: 62 additions & 0 deletions graphql/e2e/auth/debug_off/debugoff_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,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 := os.ReadFile(schemaFile)
Expand Down
95 changes: 57 additions & 38 deletions graphql/e2e/auth/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -620,43 +620,43 @@ type Author {
interface Post @secret(field: "pwd") @auth(
password: { rule: "{$ROLE: { eq: \"Admin\" } }"},
query: { rule: """
query($USER: String!) {
query($USER: String!) {
queryPost{
author(filter: {name: {eq: $USER}}){
name
}
}
}
}""" },
add: { rule: """
query($USER: String!) {
query($USER: String!) {
queryPost{
author(filter: {name: {eq: $USER}}){
name
}
}
}
}""" },
delete: { rule: """
query($USER: String!) {
query($USER: String!) {
queryPost{
author(filter: {name: {eq: $USER}}){
name
}
}
}
}""" },
update: { rule: """
query($USER: String!) {
query($USER: String!) {
queryPost{
author(filter: {name: {eq: $USER}}){
name
}
}
}
}""" }
){
id: ID!
text: String! @search(by: [exact])
topic: String
datePublished: DateTime @search
author: Author!
author: Author!
}

interface MsgPost @auth(
Expand All @@ -678,28 +678,28 @@ type Question implements Post @auth(
}
}""" },
query:{ rule: """
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
}""" },
add:{ rule: """
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
}""" },
delete:{ rule: """
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
}""" },
update:{ rule: """
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryQuestion(filter: { answered: $ANS } ) {
id
}
}""" },
){
answered: Boolean @search
Expand All @@ -726,7 +726,7 @@ type Answer implements Post {
interface A {
id: ID!
fieldA: String @search(by: [exact])
random: String
random: String
}

type B implements A {
Expand All @@ -735,16 +735,16 @@ type B implements A {

type C implements A @auth(
query:{ rule: """
query($ANS: Boolean!) {
queryC(filter: { fieldC: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryC(filter: { fieldC: $ANS } ) {
id
}
}""" },
delete:{ rule: """
query($ANS: Boolean!) {
queryC(filter: { fieldC: $ANS } ) {
id
}
query($ANS: Boolean!) {
queryC(filter: { fieldC: $ANS } ) {
id
}
}""" }
){
fieldC: Boolean @search
Expand Down Expand Up @@ -780,10 +780,10 @@ type Book @auth(

type Mission @key(fields: "id") @auth(
query:{ rule: """
query($USER: String!) {
queryMission(filter: { supervisorName: {eq: $USER} } ) {
id
}
query($USER: String!) {
queryMission(filter: { supervisorName: {eq: $USER} } ) {
id
}
}""" }
){
id: String! @id
Expand Down Expand Up @@ -864,6 +864,25 @@ type Person
name: String!
}

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
Expand Down
2 changes: 2 additions & 0 deletions graphql/e2e/common/common.go
Original file line number Diff line number Diff line change
Expand Up @@ -868,6 +868,7 @@ func RunAll(t *testing.T) {
t.Run("query id directive with int", idDirectiveWithInt)
t.Run("query id directive with int64", idDirectiveWithInt64)
t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)
t.Run("query @id field with interface arg on interface", queryWithIDFieldAndInterfaceArg)

// mutation tests
t.Run("add mutation", addMutation)
Expand Down Expand Up @@ -927,6 +928,7 @@ func RunAll(t *testing.T) {
t.Run("input coercion to list", inputCoerciontoList)
t.Run("multiple external Id's tests", multipleXidsTests)
t.Run("Upsert Mutation Tests", upsertMutationTests)
t.Run("add mutation with @id field and interface arg", addMutationWithIDFieldHavingInterfaceArg)

// error tests
t.Run("graphql completion on", graphQLCompletionOn)
Expand Down
Loading

0 comments on commit 4ea4bb5

Please sign in to comment.