Skip to content

Commit

Permalink
feat(graphql): allow updatable and nullable @id fields. (#7736)
Browse files Browse the repository at this point in the history 
Currently, @id fields are not updatable and also we enforce them to be non-null.
In this PR we are allowing @id fields to be updatable and nullable.

RFC:
 https://discuss.dgraph.io/t/rfc-mutable-and-null-able-id-field-in-graphql/13663

Related Posts:
 https://discuss.dgraph.io/t/nullable-id-field/13296
 https://discuss.dgraph.io/t/editable-id-fields/13295
 https://discuss.dgraph.io/t/rfc-allow-multiple-unique-fields-in-graphql-schema/12008
  • Loading branch information
@mangalaman93
mangalaman93 committed Dec 6, 2023
1 parent 283fe63 commit 5cc1ddb
Show file tree
Hide file tree
Showing 32 changed files with 2,189 additions and 570 deletions.
149 changes: 148 additions & 1 deletion graphql/e2e/auth/add_mutation_test.go
View file
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 Tweets already exists for field id inside type tweet1")
" GraphQL debug: id tweet1 already exists for field id inside type Tweets")
} else {
common.RequireNoGQLErrors(t, gqlResponse)
require.JSONEq(t, tcase.result, string(gqlResponse.Data))
Expand Down Expand Up @@ -1164,3 +1164,150 @@ func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
// cleanup
common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
}

func TestUpdateMutationWithIDFields(t *testing.T) {
addEmployerParams := &common.GraphQLParams{
Query: `mutation addEmployer($input: [AddEmployerInput!]!) {
addEmployer(input: $input, upsert: false) {
numUids
}
}`,
Variables: map[string]interface{}{"input": []interface{}{
map[string]interface{}{
"company": "ABC tech",
"name": "ABC",
"worker": map[string]interface{}{
"empId": "E01",
"regNo": 101,
},
}, map[string]interface{}{
"company": " XYZ tech",
"name": "XYZ",
"worker": map[string]interface{}{
"empId": "E02",
"regNo": 102,
},
},
},
},
}

gqlResponse := addEmployerParams.ExecuteAsPost(t, common.GraphqlURL)
common.RequireNoGQLErrors(t, gqlResponse)
var resultEmployer struct {
AddEmployer struct {
NumUids int
}
}
err := json.Unmarshal(gqlResponse.Data, &resultEmployer)
require.NoError(t, err)
require.Equal(t, 4, resultEmployer.AddEmployer.NumUids)

// errors while updating node should be returned in debug mode,
// if type have auth rules defined on it

tcases := []struct {
name string
query string
variables string
error string
}{{
name: "update mutation gives error when multiple nodes are selected in filter",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": [
"ABC",
"XYZ"
]
}
},
"set": {
"name": "MNO",
"company": "MNO tech"
}
}
}`,
error: "mutation updateEmployer failed because GraphQL debug: only one node is allowed" +
" in the filter while updating fields with @id directive",
}, {
name: "update mutation gives error when given @id field already exist in some node",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"company": "ABC tech"
}
}
}`,
error: "couldn't rewrite mutation updateEmployer because failed to rewrite mutation" +
" payload because GraphQL debug: id ABC tech already exists for field company" +
" inside type Employer",
},
{
name: "update mutation gives error when multiple nodes are found at nested level" +
"while linking rot object to nested object",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"name": "JKL",
"worker":{
"empId":"E01",
"regNo":102
}
}
}
}`,
error: "couldn't rewrite mutation updateEmployer because failed to rewrite mutation" +
" payload because multiple nodes found for given xid values, updation not possible",
},
}

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 := &common.GraphQLParams{
Query: tcase.query,
Variables: vars,
}

resp := params.ExecuteAsPost(t, common.GraphqlURL)
require.Equal(t, tcase.error, resp.Errors[0].Error())
})
}

// cleanup
filterEmployer := map[string]interface{}{"name": map[string]interface{}{"in": []string{"ABC", "XYZ"}}}
filterWorker := map[string]interface{}{"empId": map[string]interface{}{"in": []string{"E01", "E02"}}}
common.DeleteGqlType(t, "Employer", filterEmployer, 2, nil)
common.DeleteGqlType(t, "Worker", filterWorker, 2, nil)
}
2 changes: 1 addition & 1 deletion graphql/e2e/auth/auth_test.go
View file
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 Tweets already exists for field id inside type tweet1")
"GraphQL debug: id tweet1 already exists for field id inside type Tweets")

// Clear the tweet.
tweet.DeleteByID(t, user, metaInfo)
Expand Down
148 changes: 146 additions & 2 deletions graphql/e2e/auth/debug_off/debugoff_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -142,8 +142,6 @@ func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
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
Expand All @@ -153,6 +151,8 @@ func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
require.NoError(t, err)
require.Equal(t, 1, resultLibraryMember.AddLibraryMember.NumUids)

// add SportsMember 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) {
Expand Down Expand Up @@ -184,6 +184,150 @@ func TestAddMutationWithAuthOnIDFieldHavingInterfaceArg(t *testing.T) {
common.DeleteGqlType(t, "LibraryMember", map[string]interface{}{}, 1, nil)
}

func TestUpdateMutationWithIDFields(t *testing.T) {
addEmployerParams := &common.GraphQLParams{
Query: `mutation addEmployer($input: [AddEmployerInput!]!) {
addEmployer(input: $input, upsert: false) {
numUids
}
}`,
Variables: map[string]interface{}{"input": []interface{}{
map[string]interface{}{
"company": "ABC tech",
"name": "ABC",
"worker": map[string]interface{}{
"empId": "E01",
"regNo": 101,
},
}, map[string]interface{}{
"company": " XYZ tech",
"name": "XYZ",
"worker": map[string]interface{}{
"empId": "E02",
"regNo": 102,
},
},
},
},
}

gqlResponse := addEmployerParams.ExecuteAsPost(t, common.GraphqlURL)
common.RequireNoGQLErrors(t, gqlResponse)
type resEmployer struct {
AddEmployer struct {
NumUids int
}
}
var resultEmployer resEmployer
err := json.Unmarshal(gqlResponse.Data, &resultEmployer)
require.NoError(t, err)
require.Equal(t, 4, resultEmployer.AddEmployer.NumUids)

// errors while updating node should be returned in debug mode,
// if type have auth rules defined on it

tcases := []struct {
name string
query string
variables string
error string
}{{
name: "update mutation gives error when multiple nodes are selected in filter",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": [
"ABC",
"XYZ"
]
}
},
"set": {
"name": "MNO",
"company": "MNO tech"
}
}
}`,
}, {
name: "update mutation gives error when given @id field already exist in some node",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"company": "ABC tech"
}
}
}`,
},
{
name: "update mutation gives error when multiple nodes are found at nested level" +
"while linking rot object to nested object",
query: `mutation update($patch: UpdateEmployerInput!) {
updateEmployer(input: $patch) {
numUids
}
}`,
variables: `{
"patch": {
"filter": {
"name": {
"in": "ABC"
}
},
"set": {
"name": "JKL",
"worker":{
"empId":"E01",
"regNo":102
}
}
}
}`,
},
}

for _, tcase := range tcases {
t.Run(tcase.name, func(t *testing.T) {
var vars map[string]interface{}
var resultEmployerErr resEmployer
if tcase.variables != "" {
err := json.Unmarshal([]byte(tcase.variables), &vars)
require.NoError(t, err)
}
params := &common.GraphQLParams{
Query: tcase.query,
Variables: vars,
}

resp := params.ExecuteAsPost(t, common.GraphqlURL)
err := json.Unmarshal(resp.Data, &resultEmployerErr)
require.NoError(t, err)
require.Equal(t, 0, resultEmployerErr.AddEmployer.NumUids)
})
}

// cleanup
filterEmployer := map[string]interface{}{"name": map[string]interface{}{"in": []string{"ABC", "XYZ"}}}
filterWorker := map[string]interface{}{"empId": map[string]interface{}{"in": []string{"E01", "E02"}}}
common.DeleteGqlType(t, "Employer", filterEmployer, 2, nil)
common.DeleteGqlType(t, "Worker", filterWorker, 2, nil)
}

func TestMain(m *testing.M) {
schemaFile := "../schema.graphql"
schema, err := os.ReadFile(schemaFile)
Expand Down
15 changes: 15 additions & 0 deletions graphql/e2e/auth/schema.graphql
View file
Original file line number Diff line number Diff line change
Expand Up @@ -864,6 +864,21 @@ type Person
name: String!
}

type Employer@auth(
query: { rule: "{$ROLE: { eq: \"ADMIN\" } }" },
) {
company: String! @id
companyId: String @id
name: String @id
worker:[Worker]
}

type Worker {
regNo: Int @id
uniqueId: Int @id
empId: String! @id
}

interface Member @auth(
query: { rule: "{$ROLE: { eq: \"ADMIN\" } }" },
){
Expand Down
3 changes: 2 additions & 1 deletion graphql/e2e/common/common.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -930,7 +930,8 @@ 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)
t.Run("mutation with @id field and interface arg", addMutationWithIDFieldHavingInterfaceArg)
t.Run("xid update and nullable tests", xidUpdateAndNullableTests)

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

0 comments on commit 5cc1ddb

Please sign in to comment.