Implicit external check (#2449)

* Prevent entity resolver generation for stub types.
In Federation 2 key fields are implicitly external

* Add more comments to "isResolvable"

* Check that no resolvers are set for stub "Hello"

* Run generate with go 1.16

* Simplify implicit external check

* Add stricter federation version check.
Update comment on expected behavior of the resolvable argument.
Add comment to documentation about external directive.

* Preallocate keyFields slice

* Add non stub type to federation v2 test

* Do not append to preallocated slice

* Add test coverage for multiple fields in key

* Fix typo in comment

docs/content/recipes/federation.md

@@ -47,12 +47,12 @@ type Review {
 }
 
 extend type User @key(fields: "id") {
-  id: ID! @external
+  id: ID! @external # External directive not required for key fields in federation v2
   reviews: [Review]
 }
 
 extend type Product @key(fields: "upc") {
-  upc: String! @external
+  upc: String! @external # External directive not required for key fields in federation v2
   reviews: [Review]
 }
 ```

plugin/federation/entity.go

@@ -0,0 +1,109 @@
+package federation
+
+import (
+	"github.com/99designs/gqlgen/codegen/config"
+	"github.com/99designs/gqlgen/plugin/federation/fieldset"
+	"github.com/vektah/gqlparser/v2/ast"
+)
+
+// Entity represents a federated type
+// that was declared in the GQL schema.
+type Entity struct {
+	Name      string // The same name as the type declaration
+	Def       *ast.Definition
+	Resolvers []*EntityResolver
+	Requires  []*Requires
+	Multi     bool
+}
+
+type EntityResolver struct {
+	ResolverName string      // The resolver name, such as FindUserByID
+	KeyFields    []*KeyField // The fields declared in @key.
+	InputType    string      // The Go generated input type for multi entity resolvers
+}
+
+type KeyField struct {
+	Definition *ast.FieldDefinition
+	Field      fieldset.Field        // len > 1 for nested fields
+	Type       *config.TypeReference // The Go representation of that field type
+}
+
+// Requires represents an @requires clause
+type Requires struct {
+	Name  string                // the name of the field
+	Field fieldset.Field        // source Field, len > 1 for nested fields
+	Type  *config.TypeReference // The Go representation of that field type
+}
+
+func (e *Entity) allFieldsAreExternal(federationVersion int) bool {
+	for _, field := range e.Def.Fields {
+		if !e.isFieldImplicitlyExternal(field, federationVersion) && field.Directives.ForName("external") == nil {
+			return false
+		}
+	}
+	return true
+}
+
+// In federation v2, key fields are implicitly external.
+func (e *Entity) isFieldImplicitlyExternal(field *ast.FieldDefinition, federationVersion int) bool {
+	// Key fields are only implicitly external in Federation 2
+	if federationVersion != 2 {
+		return false
+	}
+	// TODO: From the spec, it seems like if an entity is not resolvable then it should not only not have a resolver, but should not appear in the _Entitiy union.
+	// The current implementation is a less drastic departure from the previous behavior, but should probably be reviewed.
+	// See https://www.apollographql.com/docs/federation/subgraph-spec/
+	if e.isResolvable() {
+		return false
+	}
+	// If the field is a key field, it is implicitly external
+	if e.isKeyField(field) {
+		return true
+	}
+
+	return false
+}
+
+// Determine if the entity is resolvable.
+func (e *Entity) isResolvable() bool {
+	key := e.Def.Directives.ForName("key")
+	if key == nil {
+		// If there is no key directive, the entity is resolvable.
+		return true
+	}
+	resolvable := key.Arguments.ForName("resolvable")
+	if resolvable == nil {
+		// If there is no resolvable argument, the entity is resolvable.
+		return true
+	}
+	// only if resolvable: false has been set on the @key directive do we consider the entity non-resolvable.
+	return resolvable.Value.Raw != "false"
+}
+
+// Determine if a field is part of the entities key.
+func (e *Entity) isKeyField(field *ast.FieldDefinition) bool {
+	for _, keyField := range e.keyFields() {
+		if keyField == field.Name {
+			return true
+		}
+	}
+	return false
+}
+
+// Get the key fields for this entity.
+func (e *Entity) keyFields() []string {
+	key := e.Def.Directives.ForName("key")
+	if key == nil {
+		return []string{}
+	}
+	fields := key.Arguments.ForName("fields")
+	if fields == nil {
+		return []string{}
+	}
+	fieldSet := fieldset.New(fields.Value.Raw, nil)
+	keyFields := make([]string, len(fieldSet))
+	for i, field := range fieldSet {
+		keyFields[i] = field[0]
+	}
+	return keyFields
+}

plugin/federation/federation.go

@@ -198,44 +198,6 @@ type Entity {
 	}
 }
 
-// Entity represents a federated type
-// that was declared in the GQL schema.
-type Entity struct {
-	Name      string // The same name as the type declaration
-	Def       *ast.Definition
-	Resolvers []*EntityResolver
-	Requires  []*Requires
-	Multi     bool
-}
-
-type EntityResolver struct {
-	ResolverName string      // The resolver name, such as FindUserByID
-	KeyFields    []*KeyField // The fields declared in @key.
-	InputType    string      // The Go generated input type for multi entity resolvers
-}
-
-type KeyField struct {
-	Definition *ast.FieldDefinition
-	Field      fieldset.Field        // len > 1 for nested fields
-	Type       *config.TypeReference // The Go representation of that field type
-}
-
-// Requires represents an @requires clause
-type Requires struct {
-	Name  string                // the name of the field
-	Field fieldset.Field        // source Field, len > 1 for nested fields
-	Type  *config.TypeReference // The Go representation of that field type
-}
-
-func (e *Entity) allFieldsAreExternal() bool {
-	for _, field := range e.Def.Fields {
-		if field.Directives.ForName("external") == nil {
-			return false
-		}
-	}
-	return true
-}
-
 func (f *federation) GenerateCode(data *codegen.Data) error {
 	if len(f.Entities) > 0 {
 		if data.Objects.ByName("Entity") != nil {
@@ -323,7 +285,7 @@ func (f *federation) setEntities(schema *ast.Schema) {
 		//    extend TypeDefinedInOtherService @key(fields: "id") {
 		//       id: ID @external
 		//    }
-		if !e.allFieldsAreExternal() {
+		if !e.allFieldsAreExternal(f.Version) {
 			for _, dir := range keys {
 				if len(dir.Arguments) > 2 {
 					panic("More than two arguments provided for @key declaration.")

plugin/federation/federation_test.go

@@ -121,6 +121,12 @@ func TestCodeGenerationFederation2(t *testing.T) {
 	err := f.MutateConfig(cfg)
 
 	require.NoError(t, err)
+	require.Equal(t, "ExternalExtension", f.Entities[0].Name)
+	require.Len(t, f.Entities[0].Resolvers, 1)
+	require.Equal(t, "Hello", f.Entities[1].Name)
+	require.Empty(t, f.Entities[1].Resolvers)
+	require.Equal(t, "World", f.Entities[2].Name)
+	require.Empty(t, f.Entities[2].Resolvers)
 
 	data, err := codegen.BuildData(cfg)
 	if err != nil {

plugin/federation/testdata/federation2/federation2.graphql

@@ -10,6 +10,16 @@ type Hello @key(fields:"name", resolvable: false) {
   name: String!
 }
 
+type World @key(fields: "foo bar", resolvable: false) {
+    foo: String!
+    bar: Int!
+}
+
+extend type ExternalExtension @key(fields: "  upc    ") {
+    upc: String!
+    reviews: [Hello]
+}
+
 type CustomQuery {
   hello: Hello!
 }