Skip to content

Commit

Permalink
Do not require a resolver for "empty" extended types.
Browse files Browse the repository at this point in the history
Summary:
If our schema has a field with a type defined in another service, then
we need to define an "empty extend" of that type in this service, so
this service knows what the type is like.  But the graphql-server will
never ask us to actually resolve this "empty extend", so we don't
require a resolver function for it.  Example:
```
   type MyType {
      myvar: TypeDefinedInOtherService
   }

   // Federation needs this type, but it doesn't need a resolver for
   // it!  graphql-server will never ask *us* to resolve a
   // TypeDefinedInOtherService; it will ask the other service.
   extend TypeDefinedInOtherService @key(fields: "id") {
      id: ID @extends
   }
```

Test Plan:
I manually tested this on a service (`assignments`) that we have that
fell afoul of this problem.  But I had a hard time adding tests inside
gqlgen because the error happens at validation-time, and the
federation tests are not set up to go that far down the processing
path.

Reviewers: benkraft, lizfaubell, dhruv

Subscribers: #graphql

Differential Revision: https://phabricator.khanacademy.org/D61883
  • Loading branch information
csilvers committed Mar 28, 2020
1 parent a1a0261 commit 7e3f584
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 25 deletions.
41 changes: 35 additions & 6 deletions plugin/federation/federation.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,11 +95,13 @@ func (f *federation) InjectSourceLate(schema *ast.Schema) *ast.Source {
}
entities += e.Name

resolverArgs := ""
for _, field := range e.KeyFields {
resolverArgs += fmt.Sprintf("%s: %s,", field.Field.Name, field.Field.Type.String())
if e.ResolverName != "" {
resolverArgs := ""
for _, field := range e.KeyFields {
resolverArgs += fmt.Sprintf("%s: %s,", field.Field.Name, field.Field.Type.String())
}
resolvers += fmt.Sprintf("\t%s(%s): %s!\n", e.ResolverName, resolverArgs, e.Def.Name)
}
resolvers += fmt.Sprintf("\t%s(%s): %s!\n", e.ResolverName, resolverArgs, e.Def.Name)

}

Expand Down Expand Up @@ -162,6 +164,10 @@ type RequireField struct {
TypeReference *config.TypeReference // The Go representation of that field type
}

func (e *Entity) allFieldsAreKeyFields() bool {
return len(e.Def.Fields) == len(e.KeyFields)
}

func (f *federation) GenerateCode(data *codegen.Data) error {
if len(f.Entities) > 0 {
data.Objects.ByName("Entity").Root = true
Expand Down Expand Up @@ -251,13 +257,36 @@ func (f *federation) setEntities(schema *ast.Schema) {

}

f.Entities = append(f.Entities, &Entity{
e := &Entity{
Name: schemaType.Name,
KeyFields: keyFields,
Def: schemaType,
ResolverName: resolverName,
Requires: requires,
})
}
// If our schema has a field with a type defined in
// another service, then we need to define an "empty
// extend" of that type in this service, so this service
// knows what the type is like. But the graphql-server
// will never ask us to actually resolve this "empty
// extend", so we don't require a resolver function for
// it. (Well, it will never ask in practice; it's
// unclear whether the spec guarantees this. See
// https://github.com/apollographql/apollo-server/issues/3852
// ). Example:
// type MyType {
// myvar: TypeDefinedInOtherService
// }
// // Federation needs this type, but
// // it doesn't need a resolver for it!
// extend TypeDefinedInOtherService @key(fields: "id") {
// id: ID @extends
// }
if (e.allFieldsAreKeyFields()) {
e.ResolverName = ""
}

f.Entities = append(f.Entities, e)
}
}
}
Expand Down
40 changes: 21 additions & 19 deletions plugin/federation/federation.gotpl
Original file line number Diff line number Diff line change
Expand Up @@ -34,29 +34,31 @@ func (ec *executionContext) __resolve_entities(ctx context.Context, representati
}
switch typeName {
{{ range .Entities }}
case "{{.Def.Name}}":
{{ range $i, $keyField := .KeyFields -}}
id{{$i}}, err := ec.{{.TypeReference.UnmarshalFunc}}(ctx, rep["{{$keyField.Field.Name}}"])
if err != nil {
return nil, errors.New(fmt.Sprintf("Field %s undefined in schema.", "{{$keyField.Field.Name}}"))
}
{{end}}

entity, err := ec.resolvers.Entity().{{.ResolverName | go}}(ctx,
{{ range $i, $_ := .KeyFields -}} id{{$i}}, {{end}})
if err != nil {
return nil, err
}

{{ range .Requires }}
{{ range .Fields}}
entity.{{.NameGo}}, err = ec.{{.TypeReference.UnmarshalFunc}}(ctx, rep["{{.Name}}"])
{{ if .ResolverName }}
case "{{.Def.Name}}":
{{ range $i, $keyField := .KeyFields -}}
id{{$i}}, err := ec.{{.TypeReference.UnmarshalFunc}}(ctx, rep["{{$keyField.Field.Name}}"])
if err != nil {
return nil, err
return nil, errors.New(fmt.Sprintf("Field %s undefined in schema.", "{{$keyField.Field.Name}}"))
}
{{end}}

entity, err := ec.resolvers.Entity().{{.ResolverName | go}}(ctx,
{{ range $i, $_ := .KeyFields -}} id{{$i}}, {{end}})
if err != nil {
return nil, err
}

{{ range .Requires }}
{{ range .Fields}}
entity.{{.NameGo}}, err = ec.{{.TypeReference.UnmarshalFunc}}(ctx, rep["{{.Name}}"])
if err != nil {
return nil, err
}
{{ end }}
{{ end }}
list = append(list, entity)
{{ end }}
list = append(list, entity)
{{ end }}
default:
return nil, errors.New("unknown type: "+typeName)
Expand Down

0 comments on commit 7e3f584

Please sign in to comment.