Merge pull request #985 from Khan/no-key-needed

Correctly generate a federated schema when no entity has a `@key`.
99designs · Jan 24, 2020 · f7a6772 · f7a6772
2 parents 36aae4a + fa88499
commit f7a6772
Show file tree

Hide file tree

Showing 6 changed files with 96 additions and 29 deletions.
diff --git a/plugin/federation/federation.go b/plugin/federation/federation.go
@@ -56,9 +56,11 @@ func (f *federation) MutateConfig(cfg *config.Config) error {
 		"_Any": {
 			Model: config.StringList{"github.com/99designs/gqlgen/graphql.Map"},
 		},
-		"Entity": {
+	}
+	if len(entityFields) > 0 {
+		builtins["Entity"] = config.TypeMapEntry{
 			Fields: entityFields,
-		},
+		}
 	}
 	for typeName, entry := range builtins {
 		if cfg.Models.Exists(typeName) {
@@ -79,7 +81,14 @@ func (f *federation) MutateConfig(cfg *config.Config) error {
 // the fields that had the @key directive
 func (f *federation) InjectSources(cfg *config.Config) {
 	cfg.AdditionalSources = append(cfg.AdditionalSources, f.getSource(false))
+
 	f.setEntities(cfg)
+	if len(f.Entities) == 0 {
+		// It's unusual for a service not to have any entities, but
+		// possible if it only exports top-level queries and mutations.
+		return
+	}
+
 	s := "type Entity {\n"
 	for _, e := range f.Entities {
 		s += fmt.Sprintf("\t%s(%s: %s): %s!\n", e.ResolverName, e.Field.Name, e.Field.Type.String(), e.Def.Name)
@@ -88,9 +97,9 @@ func (f *federation) InjectSources(cfg *config.Config) {
 	cfg.AdditionalSources = append(cfg.AdditionalSources, &ast.Source{Name: "entity.graphql", Input: s, BuiltIn: true})
 }
 
-// MutateSchema creates types and query declarations
-// that are required by the federation spec.
-func (f *federation) MutateSchema(s *ast.Schema) error {
+// addEntityToSchema adds the _Entity Union and _entities query to schema.
+// This is part of MutateSchema.
+func (f *federation) addEntityToSchema(s *ast.Schema) {
 	// --- Set _Entity Union ---
 	union := &ast.Definition{
 		Name:        "_Entity",
@@ -124,8 +133,11 @@ func (f *federation) MutateSchema(s *ast.Schema) error {
 		s.Types["Query"] = s.Query
 	}
 	s.Query.Fields = append(s.Query.Fields, fieldDef)
+}
 
-	// --- set _Service type ---
+// addServiceToSchema adds the _Service type and _service query to schema.
+// This is part of MutateSchema.
+func (f *federation) addServiceToSchema(s *ast.Schema) {
 	typeDef := &ast.Definition{
 		Kind: ast.Object,
 		Name: "_Service",
@@ -144,6 +156,17 @@ func (f *federation) MutateSchema(s *ast.Schema) error {
 		Type: ast.NonNullNamedType("_Service", nil),
 	}
 	s.Query.Fields = append(s.Query.Fields, _serviceDef)
+}
+
+// MutateSchema creates types and query declarations
+// that are required by the federation spec.
+func (f *federation) MutateSchema(s *ast.Schema) error {
+	// It's unusual for a service not to have any entities, but
+	// possible if it only exports top-level queries and mutations.
+	if len(f.Entities) > 0 {
+		f.addEntityToSchema(s)
+	}
+	f.addServiceToSchema(s)
 	return nil
 }
 
@@ -196,18 +219,20 @@ func (f *federation) GenerateCode(data *codegen.Data) error {
 		return err
 	}
 	f.SDL = sdl
-	data.Objects.ByName("Entity").Root = true
-	for _, e := range f.Entities {
-		obj := data.Objects.ByName(e.Def.Name)
-		for _, f := range obj.Fields {
-			if f.Name == e.Field.Name {
-				e.FieldTypeGo = f.TypeReference.GO.String()
-			}
-			for _, r := range e.Requires {
-				for _, rf := range r.Fields {
-					if rf.Name == f.Name {
-						rf.TypeReference = f.TypeReference
-						rf.NameGo = f.GoFieldName
+	if len(f.Entities) > 0 {
+		data.Objects.ByName("Entity").Root = true
+		for _, e := range f.Entities {
+			obj := data.Objects.ByName(e.Def.Name)
+			for _, f := range obj.Fields {
+				if f.Name == e.Field.Name {
+					e.FieldTypeGo = f.TypeReference.GO.String()
+				}
+				for _, r := range e.Requires {
+					for _, rf := range r.Fields {
+						if rf.Name == f.Name {
+							rf.TypeReference = f.TypeReference
+							rf.NameGo = f.GoFieldName
+						}
 					}
 				}
 			}

diff --git a/plugin/federation/federation.gotpl b/plugin/federation/federation.gotpl
@@ -12,6 +12,7 @@ func (ec *executionContext) __resolve__service(ctx context.Context) (federation.
 	}, nil
 }
 
+{{if .Entities}}
 func (ec *executionContext) __resolve_entities(ctx context.Context, representations []map[string]interface{}) ([]_Entity, error) {
 	list := []_Entity{}
 	for _, rep := range representations {
@@ -46,3 +47,4 @@ func (ec *executionContext) __resolve_entities(ctx context.Context, representati
 	}
 	return list, nil
 }
+{{end}}
diff --git a/plugin/federation/federation_test.go b/plugin/federation/federation_test.go
@@ -10,11 +10,12 @@ import (
 )
 
 func TestInjectSources(t *testing.T) {
-	var cfg config.Config
+	cfg, err := config.LoadConfig("test_data/gqlgen.yml")
+	require.NoError(t, err)
 	f := &federation{}
-	f.InjectSources(&cfg)
+	f.InjectSources(cfg)
 	if len(cfg.AdditionalSources) != 2 {
-		t.Fatalf("expected an additional source but got %v", len(cfg.AdditionalSources))
+		t.Fatalf("expected 2 additional sources but got %v", len(cfg.AdditionalSources))
 	}
 }
 
@@ -30,10 +31,9 @@ func TestMutateSchema(t *testing.T) {
 	if gqlErr != nil {
 		t.Fatal(gqlErr)
 	}
+
 	err := f.MutateSchema(schema)
-	if err != nil {
-		t.Fatal(err)
-	}
+	require.NoError(t, err)
 }
 
 func TestGetSDL(t *testing.T) {
@@ -53,3 +53,31 @@ func TestMutateConfig(t *testing.T) {
 	err = f.MutateConfig(cfg)
 	require.NoError(t, err)
 }
+
+func TestInjectSourcesNoKey(t *testing.T) {
+	cfg, err := config.LoadConfig("test_data/nokey.yml")
+	require.NoError(t, err)
+	f := &federation{}
+	f.InjectSources(cfg)
+	if len(cfg.AdditionalSources) != 1 {
+		t.Fatalf("expected an additional source but got %v", len(cfg.AdditionalSources))
+	}
+}
+
+func TestGetSDLNoKey(t *testing.T) {
+	cfg, err := config.LoadConfig("test_data/nokey.yml")
+	require.NoError(t, err)
+	f := &federation{}
+	_, err = f.getSDL(cfg)
+	require.NoError(t, err)
+}
+
+func TestMutateConfigNoKey(t *testing.T) {
+	cfg, err := config.LoadConfig("test_data/nokey.yml")
+	require.NoError(t, err)
+	require.NoError(t, cfg.Check())
+
+	f := &federation{}
+	err = f.MutateConfig(cfg)
+	require.NoError(t, err)
+}
diff --git a/plugin/federation/test_data/nokey.graphql b/plugin/federation/test_data/nokey.graphql
@@ -0,0 +1,8 @@
+type Hello {
+  name: String!
+}
+
+type Query {
+  hello: Hello!
+}
+
diff --git a/plugin/federation/test_data/nokey.yml b/plugin/federation/test_data/nokey.yml
@@ -0,0 +1,3 @@
+schema:
+  - "test_data/nokey.graphql"
+federated: true
diff --git a/plugin/federation/test_data/schema.graphql b/plugin/federation/test_data/schema.graphql
@@ -1,6 +1,7 @@
 type Hello @key(fields: "name") {
-        	name: String!
-        }
-        type Query {
-        	hello: Hello!
-        }
+  name: String!
+}
+
+type Query {
+  hello: Hello!
+}