graphql: Add extensions

[abhimanyusinghgaur]

Description.

This PR adds extensions in GraphQL responses. Currently, only the total number of touched UIDs is sent in extensions as touched_uids.

GitHub Issue or Jira number.

#GRAPHQL-327

Other components or 3rd party tools affected (or regression areas).

None

Affected releases.

> 20.03.0

Changelog tags.

added extensions in GraphQL

Please indicate if this is a breaking change.

no

Please indicate if this is an enterprise feature.

no

Please indicate if documentation needs to be updated.

Yes

Please indicate if end to end testing is needed.

No

---

This change is 

Docs Preview:

[MichaelJCompton]

Looks like this touches too many things. Can we do it without updating all the interfaces everywhere.

For example, all the admin endpoints don't really need this, so I'd like to see them not change.

I think it's be much smaller changes and easier to deal with if the resolvers do it internally and not expose it to all the interfaces.

Reviewable status: 0 of 19 files reviewed, 2 unresolved discussions (waiting on @abhimanyusinghgaur, @manishrjain, and @MichaelJCompton)

---

graphql/resolve/query.go, line 124 at r2 (raw file):

	}

	resp, ext, err := qr.queryExecutor.Query(ctx, dgQuery)

I'd be more comfortable - and I think will result in much smaller changes - if the queryResolver handles this internally. So it's resolve function returns a Resolved, that now has the extensions in it, it should be able to compute it without any interfaces changing.

Can't the metrics be gotten from resp here anyway, so why does the query executor's interface need to change?

Let's just calculate it internally in the resolver, and the only things that need to change then are the two Resolve functions for query and mutation rewrites and the rewriteAndExecute functions.

[MichaelJCompton]

Really nice PR. Just added a couple of things to be changed.

Reviewable status: 0 of 19 files reviewed, 9 unresolved discussions (waiting on @abhimanyusinghgaur and @manishrjain)

---

ee/acl/acl_test.go, line 105 at r3 (raw file):

	}
	resp := makeRequest(t, accessToken, params)
	resp.RequireNoGraphQLErrors(t)

nice

---

graphql/e2e/subscription/subscription_test.go, line 124 at r3 (raw file):

	err = json.Unmarshal(res, &subscriptionResp)
	require.NoError(t, err)
	require.Nil(t, subscriptionResp.Errors)

should we be checking that subscriptions come in with extensions as well?

---

graphql/resolve/resolver_error_test.go, line 463 at r3 (raw file):

	require.NotNil(t, resp)
	require.Equal(t, expectedExtensions, resp.Extensions)
}

can we add tests that it gets summed properly if there's multiple queries/ mutations in the request

---

graphql/schema/response.go, line 40 at r3 (raw file):

// NewExtensions builds GraphQL extensions from *api.Response
func NewExtensions(resp *api.Response) *Extensions {
	return &Extensions{TouchedUids: resp.GetMetrics().GetNumUids()[TouchedUidsKey]}

this is the only bit of the PR I don't like ... why does the resolve package have to take a dependency on the schema package, which now also has to take a dependency on protos/api to pass in a dgraph protos api object to get out an int ?

the TouchedUids has to be exported anyway. I think it's better to just inline this creation at the two points where it's used.

---

protos/pb.proto, line 523 at r3 (raw file):

service Worker {
    // Data serving RPCs.

something strange here ... let's not change this file if it's unrelated to the PR

---

systest/online-restore/docker-compose.yml, line 12 at r3 (raw file):

      cluster: test
    ports:
      - 8180:8180

don't change unless needed

---

systest/online-restore/backup/dgraph.20200323.231232.407/manifest.json, line 2 at r3 (raw file):

{
  "type": "full",

again don't change this unless it's related to the PR

---

testutil/graphql.go, line 54 at r3 (raw file):

}

func (resp *GraphQLResponse) RequireNoGraphQLErrors(t *testing.T) {

I really like this. We have some other bits in the GraphQL e2e tests, including a version of this. I've put up a ticket to revamp the e2e tests so that all the helper bits get moved into here.

[MichaelJCompton]

Looks good ... it just has some merge conflicts. So fix those and make sure it passes CI before merging.

Reviewable status: 0 of 15 files reviewed, 9 unresolved discussions (waiting on @abhimanyusinghgaur and @manishrjain)