Skip to content

Commit

Permalink
Avoid problems with val being undefined in the federation template. (
Browse files Browse the repository at this point in the history
…#1760)

* Avoid problems with `val` being undefined in the federation template.

When running gqlgen over our schema, we were seeing errors like:
```
assignments/generated/graphql/service.go:300:4: val declared but not used
```

The generated code looks like this:
```
func entityResolverNameForMobileNavigation(ctx context.Context, rep map[string]interface{}) (string, error) {
        for {
                var (
                        m   map[string]interface{}
                        val interface{}
                        ok  bool
                )
                m = rep
                if _, ok = m["kaid"]; !ok {
                        break
                }
                m = rep
                if _, ok = m["language"]; !ok {
                        break
                }
                return "findMobileNavigationByKaidAndLanguage", nil
        }
        return "", fmt.Errorf("%w for MobileNavigation", ErrTypeNotFound)
}
```

Looking at the code, it's pretty clear that this happens when there
are multiple key-fields, but each of them has only one keyField.Field
entry.  This is because the old code looked at `len(keyFields)` to
decide whether to declare the `val` variable, but looks at
`len(keyField.Field)` for each keyField to decide whether to use the
`val` variable.

The easiest solution, and the one I do in this PR, is to just declare
`val` all the time, and use a null-assignment to quiet the compiler
when it's not used.

* run go generate to update generated files

* run go generate to update moar generated files

* Adding a test for verify that this fixes the issue.

From `plugins/federation`, run the following command and verify that no errors are produced

```
go run github.com/99designs/gqlgen --config testdata/entityresolver/gqlgen.yml
```

Co-authored-by: MiguelCastillo <miguel@khanacademy.org>
  • Loading branch information
csilvers and MiguelCastillo authored Jan 12, 2022
1 parent 47015f1 commit b2a832d
Show file tree
Hide file tree
Showing 10 changed files with 398 additions and 38 deletions.
12 changes: 8 additions & 4 deletions example/federation/accounts/graph/generated/federation.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 9 additions & 4 deletions example/federation/products/graph/generated/federation.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

7 changes: 5 additions & 2 deletions example/federation/reviews/graph/generated/federation.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

5 changes: 2 additions & 3 deletions plugin/federation/federation.gotpl
Original file line number Diff line number Diff line change
Expand Up @@ -233,11 +233,10 @@ func (ec *executionContext) __resolve_entities(ctx context.Context, representati
for {
var (
m map[string]interface{}
{{- if gt (len .KeyFields) 1 }}
val interface{}
{{- end }}
val interface{}
ok bool
)
_ = val
{{- range $_, $keyField := .KeyFields }}
m = rep
{{- range $i, $field := .Field }}
Expand Down
26 changes: 18 additions & 8 deletions plugin/federation/testdata/allthethings/generated/federation.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions plugin/federation/testdata/entityresolver/entity.resolvers.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ func (r *entityResolver) FindHelloByName(ctx context.Context, name string) (*gen
}, nil
}

func (r *entityResolver) FindHelloMultiSingleKeysByKey1AndKey2(ctx context.Context, key1 string, key2 string) (*generated.HelloMultiSingleKeys, error) {
panic(fmt.Errorf("not implemented"))
}

func (r *entityResolver) FindHelloWithErrorsByName(ctx context.Context, name string) (*generated.HelloWithErrors, error) {
if name == "inject error" {
return nil, generated.ErrResolvingHelloWithErrorsByName
Expand Down
Loading

0 comments on commit b2a832d

Please sign in to comment.