feat(Apollo): Expose mutations to the gateway. #7358

minhaj-shakeel · 2021-01-22T15:38:31Z

Fixes GRAPHQL-968.
This PR does two things.
1 - It exposes the mutations to the apollo gateway which were earlier restricted. It exposes mutations for both type definitions as well as type extensions. Whereas queries for the extended types are not exposed as they are resolved through entity resolver.
2 - It adds support for add mutation on extended type having field with ID type as @key field. In this case the field is stored internally in the dgraph as string type so its value needs to be given as an argument inside the mutation.

This change is

…directive

abhimanyusinghgaur

Nice PR!

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

pawanrawal

Reviewable status: 6 of 9 files reviewed, 5 unresolved discussions (waiting on @abhimanyusinghgaur and @minhaj-shakeel)

graphql/e2e/common/query.go, line 455 at r3 (raw file):

func addMutationOnExtendedTypeWithIDasKeyField(t *testing.T) {
	addAstronautParams := &GraphQLParams{
		Query: `mutation addAstronaut($id1: ID!, $missionId1: String!, $id2: ID!, $missionId2: String! ) {

Do the mission and astronaut with the specified ids already exist in this service?

graphql/resolve/query_rewriter.go, line 263 at r3 (raw file):

	//	}

	// If key field is of ID type but it is an external field,

This comment looks like it belongs in the else block.

So basically if it's not external, then we query using the uid otherwise we treat it as a string and query using eq function.

graphql/schema/gqlschema.go, line 993 at r3 (raw file):

		addFieldFilters(sch, defn, apolloServiceQuery)
		addAggregationResultType(sch, defn)
		// Don't expose queries for the @extends type to the gateway

What about all the other things like filters and such which the query uses. Should they also not be exposed?

graphql/schema/wrappers.go, line 511 at r3 (raw file):

			// If key field is of ID type but it is an external field,
			// then it is stored in Dgraph as string type with Hash index.
			// So we need the predicate mapping in this case.

Can we add a dgraph schema gen test for key field which is an ID but is an external field that shows the type for it in Dgraph schema?

graphql/schema/testdata/apolloservice/output/extended-types.graphql, line 357 at r3 (raw file):

}

input AstronautFilter {

If the astronaut query isn't generated then why do we have AstronautFilter and ORder.

pawanrawal

Reviewable status: 6 of 9 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur and @minhaj-shakeel)

graphql/e2e/common/query.go, line 466 at r3 (raw file):

			}
		}`,
		Variables: map[string]interface{}{

Can you add a test where you add a new mission and link to an astronaut that doesn't exist yet? Does that return an error or work properly?

pawanrawal

Reviewed 6 of 7 files at r1, 3 of 3 files at r3.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @minhaj-shakeel)

minhaj-shakeel

Reviewable status: 6 of 12 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur and @pawanrawal)

graphql/e2e/common/query.go, line 466 at r3 (raw file):