Skip to content

Commit a38fff0

Browse files
authored
Feat(GraphQL): This PR allows @id field in interface to be unique across all implementing types (#8876)
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 Cherry picked from: #7710
1 parent 47111b1 commit a38fff0

File tree

78 files changed

+2584
-562
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

78 files changed

+2584
-562
lines changed

graphql/e2e/auth/add_mutation_test.go

+46-1
Original file line numberDiff line numberDiff line change
@@ -1002,7 +1002,7 @@ func TestUpsertMutationsWithRBAC(t *testing.T) {
10021002
require.Error(t, gqlResponse.Errors)
10031003
require.Equal(t, len(gqlResponse.Errors), 1)
10041004
require.Contains(t, gqlResponse.Errors[0].Error(),
1005-
"GraphQL debug: id tweet1 already exists for field id inside type Tweets")
1005+
" GraphQL debug: id Tweets already exists for field id inside type tweet1")
10061006
} else {
10071007
common.RequireNoGQLErrors(t, gqlResponse)
10081008
require.JSONEq(t, tcase.result, string(gqlResponse.Data))
@@ -1119,3 +1119,48 @@ func TestUpsertWithDeepAuth(t *testing.T) {
11191119
filter = map[string]interface{}{"code": map[string]interface{}{"eq": "UK"}}
11201120
common.DeleteGqlType(t, "State", filter, 1, nil)
11211121
}
1122+
1123+
func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
1124+
// add Library Member
1125+
addLibraryMemberParams := &common.GraphQLParams{
1126+
Query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
1127+
addLibraryMember(input: $input, upsert: false) {
1128+
numUids
1129+
}
1130+
}`,
1131+
Variables: map[string]interface{}{"input": []interface{}{
1132+
map[string]interface{}{
1133+
"refID": "101",
1134+
"name": "Alice",
1135+
"readHours": "4d2hr",
1136+
}},
1137+
},
1138+
}
1139+
1140+
gqlResponse := addLibraryMemberParams.ExecuteAsPost(t, common.GraphqlURL)
1141+
common.RequireNoGQLErrors(t, gqlResponse)
1142+
// add sports member should return error but in debug mode
1143+
// because interface type have auth rules defined on it
1144+
addSportsMemberParams := &common.GraphQLParams{
1145+
Query: `mutation addSportsMember($input: [AddSportsMemberInput!]!) {
1146+
addSportsMember(input: $input, upsert: false) {
1147+
numUids
1148+
}
1149+
}`,
1150+
Variables: map[string]interface{}{"input": []interface{}{
1151+
map[string]interface{}{
1152+
"refID": "101",
1153+
"name": "Bob",
1154+
"plays": "football and cricket",
1155+
}},
1156+
},
1157+
}
1158+
1159+
gqlResponse = addSportsMemberParams.ExecuteAsPost(t, common.GraphqlURL)
1160+
require.Contains(t, gqlResponse.Errors[0].Error(),
1161+
" GraphQL debug: id 101 already exists for field refID in some other"+
1162+
" implementing type of interface Member")
1163+
1164+
// cleanup
1165+
common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
1166+
}

graphql/e2e/auth/auth_test.go

+1-1
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ func TestAddMutationWithXid(t *testing.T) {
354354
require.Error(t, gqlResponse.Errors)
355355
require.Equal(t, len(gqlResponse.Errors), 1)
356356
require.Contains(t, gqlResponse.Errors[0].Error(),
357-
"GraphQL debug: id tweet1 already exists for field id inside type Tweets")
357+
"GraphQL debug: id Tweets already exists for field id inside type tweet1")
358358

359359
// Clear the tweet.
360360
tweet.DeleteByID(t, user, metaInfo)

graphql/e2e/auth/debug_off/debugoff_test.go

+62
Original file line numberDiff line numberDiff line change
@@ -122,6 +122,68 @@ func TestAddMutationWithXid(t *testing.T) {
122122
tweet.DeleteByID(t, user, metaInfo)
123123
}
124124

125+
func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
126+
// add Library Member
127+
addLibraryMemberParams := &common.GraphQLParams{
128+
Query: `mutation addLibraryMember($input: [AddLibraryMemberInput!]!) {
129+
addLibraryMember(input: $input, upsert: false) {
130+
numUids
131+
}
132+
}`,
133+
Variables: map[string]interface{}{"input": []interface{}{
134+
map[string]interface{}{
135+
"refID": "101",
136+
"name": "Alice",
137+
"readHours": "4d2hr",
138+
}},
139+
},
140+
}
141+
142+
gqlResponse := addLibraryMemberParams.ExecuteAsPost(t, common.GraphqlURL)
143+
common.RequireNoGQLErrors(t, gqlResponse)
144+
145+
// add SportsMember should return error but in debug mode
146+
// because interface type have auth rules defined on it
147+
var resultLibraryMember struct {
148+
AddLibraryMember struct {
149+
NumUids int
150+
}
151+
}
152+
err := json.Unmarshal(gqlResponse.Data, &resultLibraryMember)
153+
require.NoError(t, err)
154+
require.Equal(t, 1, resultLibraryMember.AddLibraryMember.NumUids)
155+
156+
addSportsMemberParams := &common.GraphQLParams{
157+
Query: `mutation addSportsMember($input: [AddSportsMemberInput!]!) {
158+
addSportsMember(input: $input, upsert: false) {
159+
numUids
160+
}
161+
}`,
162+
Variables: map[string]interface{}{"input": []interface{}{
163+
map[string]interface{}{
164+
"refID": "101",
165+
"name": "Bob",
166+
"plays": "football and cricket",
167+
}},
168+
},
169+
}
170+
171+
gqlResponse = addSportsMemberParams.ExecuteAsPost(t, common.GraphqlURL)
172+
common.RequireNoGQLErrors(t, gqlResponse)
173+
var resultSportsMember struct {
174+
AddSportsMember struct {
175+
NumUids int
176+
}
177+
}
178+
err = json.Unmarshal(gqlResponse.Data, &resultSportsMember)
179+
require.NoError(t, err)
180+
// We show no error here as it could be a security violation
181+
require.Equal(t, 0, resultSportsMember.AddSportsMember.NumUids)
182+
183+
// cleanup
184+
common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
185+
}
186+
125187
func TestMain(m *testing.M) {
126188
schemaFile := "../schema.graphql"
127189
schema, err := os.ReadFile(schemaFile)

graphql/e2e/auth/schema.graphql

+57-38
Original file line numberDiff line numberDiff line change
@@ -620,43 +620,43 @@ type Author {
620620
interface Post @secret(field: "pwd") @auth(
621621
password: { rule: "{$ROLE: { eq: \"Admin\" } }"},
622622
query: { rule: """
623-
query($USER: String!) {
623+
query($USER: String!) {
624624
queryPost{
625625
author(filter: {name: {eq: $USER}}){
626626
name
627627
}
628-
}
628+
}
629629
}""" },
630630
add: { rule: """
631-
query($USER: String!) {
631+
query($USER: String!) {
632632
queryPost{
633633
author(filter: {name: {eq: $USER}}){
634634
name
635635
}
636-
}
636+
}
637637
}""" },
638638
delete: { rule: """
639-
query($USER: String!) {
639+
query($USER: String!) {
640640
queryPost{
641641
author(filter: {name: {eq: $USER}}){
642642
name
643643
}
644-
}
644+
}
645645
}""" },
646646
update: { rule: """
647-
query($USER: String!) {
647+
query($USER: String!) {
648648
queryPost{
649649
author(filter: {name: {eq: $USER}}){
650650
name
651651
}
652-
}
652+
}
653653
}""" }
654654
){
655655
id: ID!
656656
text: String! @search(by: [exact])
657657
topic: String
658658
datePublished: DateTime @search
659-
author: Author!
659+
author: Author!
660660
}
661661

662662
interface MsgPost @auth(
@@ -678,28 +678,28 @@ type Question implements Post @auth(
678678
}
679679
}""" },
680680
query:{ rule: """
681-
query($ANS: Boolean!) {
682-
queryQuestion(filter: { answered: $ANS } ) {
683-
id
684-
}
681+
query($ANS: Boolean!) {
682+
queryQuestion(filter: { answered: $ANS } ) {
683+
id
684+
}
685685
}""" },
686686
add:{ rule: """
687-
query($ANS: Boolean!) {
688-
queryQuestion(filter: { answered: $ANS } ) {
689-
id
690-
}
687+
query($ANS: Boolean!) {
688+
queryQuestion(filter: { answered: $ANS } ) {
689+
id
690+
}
691691
}""" },
692692
delete:{ rule: """
693-
query($ANS: Boolean!) {
694-
queryQuestion(filter: { answered: $ANS } ) {
695-
id
696-
}
693+
query($ANS: Boolean!) {
694+
queryQuestion(filter: { answered: $ANS } ) {
695+
id
696+
}
697697
}""" },
698698
update:{ rule: """
699-
query($ANS: Boolean!) {
700-
queryQuestion(filter: { answered: $ANS } ) {
701-
id
702-
}
699+
query($ANS: Boolean!) {
700+
queryQuestion(filter: { answered: $ANS } ) {
701+
id
702+
}
703703
}""" },
704704
){
705705
answered: Boolean @search
@@ -726,7 +726,7 @@ type Answer implements Post {
726726
interface A {
727727
id: ID!
728728
fieldA: String @search(by: [exact])
729-
random: String
729+
random: String
730730
}
731731

732732
type B implements A {
@@ -735,16 +735,16 @@ type B implements A {
735735

736736
type C implements A @auth(
737737
query:{ rule: """
738-
query($ANS: Boolean!) {
739-
queryC(filter: { fieldC: $ANS } ) {
740-
id
741-
}
738+
query($ANS: Boolean!) {
739+
queryC(filter: { fieldC: $ANS } ) {
740+
id
741+
}
742742
}""" },
743743
delete:{ rule: """
744-
query($ANS: Boolean!) {
745-
queryC(filter: { fieldC: $ANS } ) {
746-
id
747-
}
744+
query($ANS: Boolean!) {
745+
queryC(filter: { fieldC: $ANS } ) {
746+
id
747+
}
748748
}""" }
749749
){
750750
fieldC: Boolean @search
@@ -780,10 +780,10 @@ type Book @auth(
780780

781781
type Mission @key(fields: "id") @auth(
782782
query:{ rule: """
783-
query($USER: String!) {
784-
queryMission(filter: { supervisorName: {eq: $USER} } ) {
785-
id
786-
}
783+
query($USER: String!) {
784+
queryMission(filter: { supervisorName: {eq: $USER} } ) {
785+
id
786+
}
787787
}""" }
788788
){
789789
id: String! @id
@@ -864,6 +864,25 @@ type Person
864864
name: String!
865865
}
866866

867+
interface Member @auth(
868+
query: { rule: "{$ROLE: { eq: \"ADMIN\" } }" },
869+
){
870+
refID: String! @id (interface:true)
871+
name: String! @id (interface:false)
872+
}
873+
874+
type SportsMember implements Member {
875+
plays: String
876+
playerRating: Int
877+
}
878+
879+
type LibraryMember implements Member {
880+
interests: [String]
881+
readHours: String
882+
}
883+
884+
885+
867886
# union testing - start
868887
enum AnimalCategory {
869888
Fish

graphql/e2e/common/common.go

+2
Original file line numberDiff line numberDiff line change
@@ -868,6 +868,7 @@ func RunAll(t *testing.T) {
868868
t.Run("query id directive with int", idDirectiveWithInt)
869869
t.Run("query id directive with int64", idDirectiveWithInt64)
870870
t.Run("query filter ID values coercion to List", queryFilterWithIDInputCoercion)
871+
t.Run("query @id field with interface arg on interface", queryWithIDFieldAndInterfaceArg)
871872

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

931933
// error tests
932934
t.Run("graphql completion on", graphQLCompletionOn)

0 commit comments

Comments
 (0)