Feat(GraphQL): Add support for GraphQL Upsert Mutations

[vmrajas]

This PR adds support for GraphQL Upsert Mutations
Related Discuss Post: https://discuss.dgraph.io/t/rfc-upsert-mutations/12736

Testing:

1. yaml tests (auth and common)
2. e2e tests (auth and common)

---

This change is 

[abhimanyusinghgaur]

Reviewed 63 of 63 files at r1.
Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @pawanrawal and @vmrajas)

---

graphql/e2e/auth/add_mutation_test.go, line 1135 at r1 (raw file):

	for _, tcase := range testCases {
		getUserParams := &common.GraphQLParams{

should we use t.Run() to run each test case? we can make the test comment its name.

---

graphql/e2e/common/mutation.go, line 5537 at r1 (raw file):

filter = map[string]interface{}{"xcode": map[string]interface{}{"eq": "S1"}}

for building xid filters, there is also a convenience function: GetXidFilter(xidKey string, xidVals []interface{}) in tests.

---

graphql/resolve/add_mutation_test.yaml, line 777 at r1 (raw file):

upsert: false

Nice idea to test the false condition!

---

graphql/resolve/add_mutation_test.yaml, line 810 at r1 (raw file):

    }

-

I am not sure, but do we need to add tests for the remaining 2 upsert cases where:

1. state exists, but its country doesn't
2. state doesn't exist but its country does

or would that be already handled by normal mutation rewriting?

Also, should we add one case where there is deep data too, like a country has more fields apart from uid, and that doesn't get updated?

---

graphql/resolve/auth_add_test.yaml, line 1261 at r1 (raw file):

    }

- name: "Upsert Add Mutation with RBAC false"

when RBAC is false, should we optimize it later to not do even the first dgquery?

---

graphql/resolve/auth_add_test.yaml, line 1345 at r1 (raw file):

"Country1": "0x123"

was this supposed to be "0x456" because in auth query that is the id or it doesn't matter in tests?
should it matter?

---

graphql/resolve/auth_add_test.yaml, line 1350 at r1 (raw file):

Quoted 5 lines of code…

  json: |
    {
      "Column4":  [ { "uid": "0x799" } ],
      "Column4.auth": [ { "uid": "0x799" } ]
    }

does this block not matter? It has Column not Country

---

graphql/schema/gqlschema.go, line 2022 at r1 (raw file):

Quoted 4 lines of code…

			{
				Name: "upsert",
				Type: &ast.Type{NamedType: "Boolean"},
			},

should we add it only in the cases when the defn has a field with @id?

[vmrajas]

Reviewable status: all files reviewed, 8 unresolved discussions (waiting on @abhimanyusinghgaur, @id, and @pawanrawal)

---

graphql/e2e/auth/add_mutation_test.go, line 1135 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

should we use t.Run() to run each test case? we can make the test comment its name.

Done.

---

graphql/e2e/common/mutation.go, line 5537 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

filter = map[string]interface{}{"xcode": map[string]interface{}{"eq": "S1"}}

for building xid filters, there is also a convenience function: GetXidFilter(xidKey string, xidVals []interface{}) in tests.

Done.

---

graphql/resolve/add_mutation_test.yaml, line 777 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

upsert: false

Nice idea to test the false condition!

Thanks. Just wanted to confirm that explicitly setting upsert to true also works.

---

graphql/resolve/add_mutation_test.yaml, line 810 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

I am not sure, but do we need to add tests for the remaining 2 upsert cases where:

1. state exists, but its country doesn't
2. state doesn't exist but its country does

or would that be already handled by normal mutation rewriting?

Also, should we add one case where there is deep data too, like a country has more fields apart from uid, and that doesn't get updated?

1. As Country is referenced by UID. This will throw an error. This is handled in normal mutation rewriting.
2. This is also a case which is handled by normal mutation rewriting as the state now needs to be created and linked to existing country.

I feel even that is taken care by other cases in normal mutation rewriting. Once UID is supplied, other data won't be processed.

---

graphql/resolve/auth_add_test.yaml, line 1261 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

when RBAC is false, should we optimize it later to not do even the first dgquery?

This optimization can be done. Althought, I doubt it will save much time. The effort will include separating out the RBAC and doing RBAC authorization along with RewriteQueries. I will write a TODO and it can be picked later on.

---

graphql/resolve/auth_add_test.yaml, line 1345 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

"Country1": "0x123"

was this supposed to be "0x456" because in auth query that is the id or it doesn't matter in tests?
should it matter?

Yes. I have fixed this test. The problem with Execute function of auth_test is that the asserts are in the mock function Execute. In case something goes wrong as it happened in this test case, the test does not fail, because no call to Execute was made.

---

graphql/resolve/auth_add_test.yaml, line 1350 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

  json: |
    {
      "Column4":  [ { "uid": "0x799" } ],
      "Column4.auth": [ { "uid": "0x799" } ]
    }

does this block not matter? It has Column not Country

Fixed it.

---

graphql/schema/gqlschema.go, line 2022 at r1 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

			{
				Name: "upsert",
				Type: &ast.Type{NamedType: "Boolean"},
			},

should we add it only in the cases when the defn has a field with @id?

Yeah. I think that will be fair. Updating code to only have upsert in case of @id directive.

[abhimanyusinghgaur]

Reviewed 10 of 52 files at r2.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @pawanrawal)