feat(Apollo): Add "_entities" resolver.

[minhaj-shakeel]

Fixes GRAPHQL-922.
This PR adds entity resolver which resolves _entities(representation: [Any]!) queries which are sent by the gateway to the graphql service.

---

This change is 

[abhimanyusinghgaur]

Reviewable status: 0 of 2 files reviewed, 12 unresolved discussions (waiting on @minhaj-shakeel and @pawanrawal)

---

graphql/resolve/resolver.go, line 301 at r2 (raw file):

					return &Resolved{
						Err: fmt.Errorf("Got Error in Unmarshalling Input Argument"),
					}

return EmptyResult(q, fmt.Errorf("Error unmarshalling `_representations` argument"))

---

graphql/resolve/resolver.go, line 307 at r2 (raw file):

					return &Resolved{
						Err: fmt.Errorf("Unable to extract __typename from input"),
					}

use EmptyResult() here

Unable to extract __typename from `_representations` argument

---

graphql/resolve/resolver.go, line 316 at r2 (raw file):

					return &Resolved{
						Err: err,
					}

EmptyResult()

---

graphql/resolve/resolver.go, line 324 at r2 (raw file):

					return &Resolved{
						Err: fmt.Errorf("Unable to Extract value for %s", keyFieldValue),
					}

Emptyresult()
error message could be a bit more verbose:

Unable to extract value for key field `%s` from `_representations` argument

---

graphql/resolve/resolver.go, line 335 at r2 (raw file):

Quoted 7 lines of code…

			for _, typ := range typeNames {
				if typ != typeNames[0] {
					return &Resolved{
						Err: fmt.Errorf("Expecting All the representations of the type %s, Got %s", typeNames[0], typ),
					}
				}
			}

we could just use a map[string]bool to store typenames, and check here that if the len(map)>1, we report error with all the keys in the map.
Error message could say:

Expected only one unique typename in `_representations` argument, got: %v

where %v is the list containing all the keys from the map.

---

graphql/resolve/resolver.go, line 344 at r2 (raw file):

// 2. We are using filter Query to resolve the Fields so filter queries must not be turned off in the Schema

Once we get it all working, then later we should do it by adding a new rewriter, that way we won't have to depend on @generate directive turning off the filter query.

---

graphql/resolve/resolver.go, line 347 at r2 (raw file):

				return &Resolved{
					Err: err,
				}

EmptyResult()

---

graphql/schema/wrappers.go, line 2642 at r2 (raw file):

Alias string, op Operation, selection []Field

instead of passing these 3 args separately, we could just pass one argument q Query and then figure out these from that.
Alias would then be q.Alias(), better than hardcoding _entities.

---

graphql/schema/wrappers.go, line 2642 at r2 (raw file):

ConstructFilterQueryFromKeyField

maybe rename it to:

ConstructEntitiesQuery

---

graphql/schema/wrappers.go, line 2649 at r2 (raw file):

oprn.inSchema.schema.Types[typeName]

typeDefn

---

graphql/schema/wrappers.go, line 2653 at r2 (raw file):

"ID"

use the IDType const

---

graphql/schema/wrappers.go, line 2674 at r2 (raw file):

Quoted 21 lines of code…

	filterValue := &ast.Value{
		Children: idChildren,
		Kind:     ast.ListValue,
	}
	// If the key field is not of "ID" type, then it must having
	// @id directive on it. Construct filter with using `in` filter
	// in that case. For eg:-
	// 		queryProduct(filter: { keyfldName: {in: [val1, ...]}}){
	// 			...
	// 			...
	// 		}
	if keyFldTypeName != "ID" {
		filterValue = &ast.Value{
			Kind: ast.ObjectValue,
			Children: ast.ChildValueList{
				&ast.ChildValue{
					Name:  "in",
					Value: filterValue,
				}},
		}
	}

var filterValue *ast.Value
if keyFldTypeName == IDType {
...
} else {
...
}

and maybe name filterValue as fieldFilterValue, because this value is for filtering a particular field and not the value of the whole filter argument.

---

graphql/schema/wrappers.go, line 2722 at r2 (raw file):

sel: nil

sel: fld

[minhaj-shakeel]

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

---

graphql/resolve/resolver.go, line 301 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

					return &Resolved{
						Err: fmt.Errorf("Got Error in Unmarshalling Input Argument"),
					}

return EmptyResult(q, fmt.Errorf("Error unmarshalling `_representations` argument"))

Done.

---

graphql/resolve/resolver.go, line 307 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

					return &Resolved{
						Err: fmt.Errorf("Unable to extract __typename from input"),
					}

use EmptyResult() here

Unable to extract __typename from `_representations` argument

Done.

---

graphql/resolve/resolver.go, line 316 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

					return &Resolved{
						Err: err,
					}

EmptyResult()

Done.

---

graphql/resolve/resolver.go, line 324 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

					return &Resolved{
						Err: fmt.Errorf("Unable to Extract value for %s", keyFieldValue),
					}

Emptyresult()
error message could be a bit more verbose:

Unable to extract value for key field `%s` from `_representations` argument

Done.

---

graphql/resolve/resolver.go, line 347 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

				return &Resolved{
					Err: err,
				}

EmptyResult()

Done.

---

graphql/schema/wrappers.go, line 2642 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

Alias string, op Operation, selection []Field

instead of passing these 3 args separately, we could just pass one argument q Query and then figure out these from that.
Alias would then be q.Alias(), better than hardcoding _entities.

Done.

---

graphql/schema/wrappers.go, line 2642 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

ConstructFilterQueryFromKeyField

maybe rename it to:

ConstructEntitiesQuery

Done.

---

graphql/schema/wrappers.go, line 2649 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

oprn.inSchema.schema.Types[typeName]

typeDefn

Don't understand what this means.

---

graphql/schema/wrappers.go, line 2653 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

"ID"

use the IDType const

Done.

---

graphql/schema/wrappers.go, line 2674 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

	filterValue := &ast.Value{
		Children: idChildren,
		Kind:     ast.ListValue,
	}
	// If the key field is not of "ID" type, then it must having
	// @id directive on it. Construct filter with using `in` filter
	// in that case. For eg:-
	// 		queryProduct(filter: { keyfldName: {in: [val1, ...]}}){
	// 			...
	// 			...
	// 		}
	if keyFldTypeName != "ID" {
		filterValue = &ast.Value{
			Kind: ast.ObjectValue,
			Children: ast.ChildValueList{
				&ast.ChildValue{
					Name:  "in",
					Value: filterValue,
				}},
		}
	}

var filterValue *ast.Value
if keyFldTypeName == IDType {
...
} else {
...
}

and maybe name filterValue as fieldFilterValue, because this value is for filtering a particular field and not the value of the whole filter argument.

Done.

---

graphql/schema/wrappers.go, line 2722 at r2 (raw file):

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

sel: nil

sel: fld

Done.

[abhimanyusinghgaur]

looking good, just got some nitpicks and good to merge after that.

Reviewed 10 of 10 files at r3.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @minhaj-shakeel and @pawanrawal)

---

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

to resolve `extended` types.

not only the types extended by this service, but also the ones defined in this service

---

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

representations := field.ArgValue("representations").([]interface{})

representations, ok := field.ArgValue("representations").([]interface{})
if !ok {
  return nil, fmt.Errorf("Error parsing `_representations` argument")
}

---

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

unmarshelling

parsing

---

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

an Array

a map

---

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

Quoted 6 lines of code…

		keyFieldname, isIDType, err := field.KeyField(typename)
		if err != nil {
			return nil, err
		}
		keyFieldIsID = isIDType
		keyFieldName = keyFieldname

we should just declare var err error outside of this for loop, so that we don't have to use := here. That would remove the need for last two statements.

---

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

	// representation List should be same, we need to validate it and throw error
	// if represenation of more than one type exists.
	if len(typeNames) > 1 {

lets also add a check above this that if len(typeNames) == 0 we return an error, because we always want only one typeName, neither less nor more.

---

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

keyFieldsValue

keyFieldsIsID

---

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

Quoted 5 lines of code…

		ids := convertIDs(keyFieldValueList)
		dgQuery.Func = &gql.Function{
			Name: "uid",
			UID:  ids,
		}

addUIDFunc(dgquery, convertIDs(keyFieldValueList))

---

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

Quoted 8 lines of code…

		args := []gql.Arg{{Value: typeDefn.DgraphPredicate(keyFieldName)}}
		for _, v := range keyFieldValueList {
			args = append(args, gql.Arg{Value: maybeQuoteArg("eq", v)})
		}
		dgQuery.Func = &gql.Function{
			Name: "eq",
			Args: args,
		}

lets add a function addEqFunc(q *gql.GraphQuery, dgPred string, values []interface{}) which encapsulates this piece of code.

---

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

	// we don't need to query uid for auth queries, as they always have at least one field in their
	// selection set.

stale comment

---

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

field.BuildType(typeName)

we already saved this in typeDefn, lets reuse that here

---

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

	KeyField(typeName string) (string, bool, error)
	BuildType(typeName string) Type

let's add comments here about what each of these functions does and what is the meaning of the returned values.

---

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

t.NamedType = q.op.inSchema.schema.Types[typeName].Name

won't this be same as:

t.NamedType = typeName

---

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

		typ: &ast.Type{
			Elem: t,
		},

If typeName were Astronaut, this would mean: [Astronaut] (i.e. a list of Astronaut).
I think we just want it to be Astronaut, as we just want to refer to a type defined in the schema and not the return type of a field.
So, we should keep it like:

typ: t

So, overall this function would just be one return statement:

return &astType{
  typ: &ast.Type{NamedType: typeName},
  ...
  ...
}

---

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

// Todo , modify to return isIDType

stale comment

[minhaj-shakeel]

Reviewable status: 8 of 10 files reviewed, 15 unresolved discussions (waiting on @abhimanyusinghgaur, @minhaj-shakeel, and @pawanrawal)

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

to resolve `extended` types.

not only the types extended by this service, but also the ones defined in this service

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

representations := field.ArgValue("representations").([]interface{})

representations, ok := field.ArgValue("representations").([]interface{})
if !ok {
  return nil, fmt.Errorf("Error parsing `_representations` argument")
}

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

unmarshelling

parsing

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

		keyFieldname, isIDType, err := field.KeyField(typename)
		if err != nil {
			return nil, err
		}
		keyFieldIsID = isIDType
		keyFieldName = keyFieldname

we should just declare var err error outside of this for loop, so that we don't have to use := here. That would remove the need for last two statements.

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

lets also add a check above this that if len(typeNames) == 0 we return an error, because we always want only one typeName, neither less nor more.

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

keyFieldsValue

keyFieldsIsID

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

		ids := convertIDs(keyFieldValueList)
		dgQuery.Func = &gql.Function{
			Name: "uid",
			UID:  ids,
		}

addUIDFunc(dgquery, convertIDs(keyFieldValueList))

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

		args := []gql.Arg{{Value: typeDefn.DgraphPredicate(keyFieldName)}}
		for _, v := range keyFieldValueList {
			args = append(args, gql.Arg{Value: maybeQuoteArg("eq", v)})
		}
		dgQuery.Func = &gql.Function{
			Name: "eq",
			Args: args,
		}

lets add a function addEqFunc(q *gql.GraphQuery, dgPred string, values []interface{}) which encapsulates this piece of code.

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

	// we don't need to query uid for auth queries, as they always have at least one field in their
	// selection set.

stale comment

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

field.BuildType(typeName)

we already saved this in typeDefn, lets reuse that here

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

	KeyField(typeName string) (string, bool, error)
	BuildType(typeName string) Type

let's add comments here about what each of these functions does and what is the meaning of the returned values.

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

t.NamedType = q.op.inSchema.schema.Types[typeName].Name

won't this be same as:

t.NamedType = typeName

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

		typ: &ast.Type{
			Elem: t,
		},

If typeName were Astronaut, this would mean: [Astronaut] (i.e. a list of Astronaut).
I think we just want it to be Astronaut, as we just want to refer to a type defined in the schema and not the return type of a field.
So, we should keep it like:

typ: t

So, overall this function would just be one return statement:

return &astType{
  typ: &ast.Type{NamedType: typeName},
  ...
  ...
}

Got it. Makes no difference btw. Changed it.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

// Todo , modify to return isIDType

stale comment

Done.

[pawanrawal]

Reviewed 8 of 10 files at r3, 2 of 2 files at r4.
Reviewable status: all files reviewed, 24 unresolved discussions (waiting on @abhimanyusinghgaur and @minhaj-shakeel)

---

graphql/e2e/directives/schema.graphql, line 317 at r4 (raw file):

}

type Astronaut @key(fields: "id") @extends {

Add a comment here that why is this not used yet

---

graphql/resolve/query_rewriter.go, line 165 at r4 (raw file):

}

// entitiesQuery rewrites The DQL Query from the Apollo `_entities` Query which is sent

rewrites the Apollo _entities Query which is sent from the Apollo gateway to a DQL query. This query is sent to the Dgraph service to ....

---

graphql/resolve/query_rewriter.go, line 196 at r4 (raw file):

		representation, ok := rep.(map[string]interface{})
		if !ok {
			return nil, fmt.Errorf("Error parsing `_representations` argument")

parsing 5th item in the _representations argument.

---

graphql/resolve/query_rewriter.go, line 204 at r4 (raw file):

		}

		// Store all the typeNames into an Map to perfrom Validation at last.

validation with v small

---

graphql/resolve/query_rewriter.go, line 212 at r4 (raw file):

		keyFieldValue, ok := representation[keyFieldName]
		if !ok {
			return nil, fmt.Errorf("Unable to extract value for key field `%s` from `_representations` argument", keyFieldName)

can return the index of the item in the array here as well

---

graphql/resolve/query_rewriter.go, line 222 at r4 (raw file):

	}

	// Since We have Restricted that All the typeNames for the inputs in the

Since we have restricted that all the typeNames

---

graphql/resolve/query_rewriter.go, line 281 at r4 (raw file):

	dgQueries := authRw.addAuthQueries(typeDefn, []*gql.GraphQuery{dgQuery}, rbac)

	if len(selectionAuth) > 0 {

return append(dgQueries, selectionAuth...), nil

---

graphql/resolve/query_test.yaml, line 3150 at r4 (raw file):

- 
  name: "entities query for extended type having @key field of ID type"

Add a couple of unit tests with auth also, positive and negative scenarios.

---

graphql/schema/wrappers.go, line 1499 at r4 (raw file):

func (q *query) KeyField(typeName string) (string, bool, error) {
	typ := q.op.inSchema.schema.Types[typeName]

if typ == nil return error.

[minhaj-shakeel]

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

---

graphql/e2e/directives/schema.graphql, line 317 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a comment here that why is this not used yet

Done.

---

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

Previously, abhimanyusinghgaur (Abhimanyu Singh Gaur) wrote…

an Array

a map

Done.

---

graphql/resolve/query_rewriter.go, line 165 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

rewrites the Apollo _entities Query which is sent from the Apollo gateway to a DQL query. This query is sent to the Dgraph service to ....

Done.

---

graphql/resolve/query_rewriter.go, line 196 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

parsing 5th item in the _representations argument.

Done.

---

graphql/resolve/query_rewriter.go, line 204 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

validation with v small

Done.

---

graphql/resolve/query_rewriter.go, line 212 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

can return the index of the item in the array here as well

Done.

---

graphql/resolve/query_rewriter.go, line 222 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Since we have restricted that all the typeNames

Done.

---

graphql/resolve/query_rewriter.go, line 281 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

return append(dgQueries, selectionAuth...), nil

Done.

---

graphql/resolve/query_test.yaml, line 3150 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

Add a couple of unit tests with auth also, positive and negative scenarios.

Done.

---

graphql/schema/wrappers.go, line 1499 at r4 (raw file):

Previously, pawanrawal (Pawan Rawal) wrote…

if typ == nil return error.

Done.

[pawanrawal]

Reviewed 6 of 6 files at r5.
Reviewable status: all files reviewed, 15 unresolved discussions (waiting on @abhimanyusinghgaur)