From 58c3c4ff3aa8be6087a61007a6355dfb9437c766 Mon Sep 17 00:00:00 2001 From: Dan Wendorf Date: Tue, 16 Mar 2021 17:00:33 -0700 Subject: [PATCH] Reload config packages after generating models If models are generated in a package that has already been loaded, and that package refers to another package that has already been loaded, we can find ourselves in a position where it appears that a GQL `union` is not satisfied. For example, if we have: ``` union Subject = User ``` with this gqlgen.yml in github.com/wendorf/gqlgen-error/gql: ``` schema: - schema.graphql exec: filename: generated.go model: filename: models_gen.go models: User: model: github.com/wendorf/gqlgen-error/gql.User Subject: model: github.com/wendorf/gqlgen-error/models.Subject ``` Note that our User model is in the github.com/wendorf/gqlgen-error.gql package, and our models_gen.go will be generated in that same package. When we try to run gqlgen, we get this error: ``` merging type systems failed: unable to bind to interface: github.com/wendorf/gqlgen-error/gql.User does not satisfy the interface github.com/wendorf/gqlgen-error/models.Subject ``` Digging deeper, it's because we use types.Implements in codegen/interface.go, which does a shallow object comparison. Because the type has been reloaded, it refers to a _different_ interface type object than the one we're comparing against, and get a false negative. By clearing the package cache and repopulating it, the whole package cache is generated at the same time, and comparisons across packages work. To see a demo of this, check out https://github.com/wendorf/gqlgen-error and try the following: 1. Checkout the works-with-v0.10.2 branch and `go generate ./...` to see that it works 2. Checkout the breaks-with-v0.13.0 branch (or run go get github.com/99designs/gqlgen@v0.13.0 yourself) and `go generate ./...` to see errors 3. Checkout the works-with-pull-request branch and `go generate ./...` to see that it works again. This branch adds a go.mod replace directive to use the gqlgen code in this PR. The demo starts at v0.10.2 since it is the last release without this problem. https://github.com/99designs/gqlgen/pull/1020 introduces the code that fails in this scenario. --- codegen/config/config.go | 23 +++++++++++++++-------- internal/code/packages.go | 7 +++++++ plugin/modelgen/models.go | 11 ++++++++++- 3 files changed, 32 insertions(+), 9 deletions(-) diff --git a/codegen/config/config.go b/codegen/config/config.go index ba939fcf59a..a0cffacdee1 100644 --- a/codegen/config/config.go +++ b/codegen/config/config.go @@ -194,15 +194,8 @@ func (c *Config) Init() error { } c.injectBuiltins() - // prefetch all packages in one big packages.Load call - pkgs := []string{ - "github.com/99designs/gqlgen/graphql", - "github.com/99designs/gqlgen/graphql/introspection", - } - pkgs = append(pkgs, c.Models.ReferencedPackages()...) - pkgs = append(pkgs, c.AutoBind...) - c.Packages.LoadAll(pkgs...) + c.Packages.LoadAll(c.packageList()...) // check everything is valid on the way out err = c.check() @@ -213,6 +206,20 @@ func (c *Config) Init() error { return nil } +func (c *Config) packageList() []string { + pkgs := []string{ + "github.com/99designs/gqlgen/graphql", + "github.com/99designs/gqlgen/graphql/introspection", + } + pkgs = append(pkgs, c.Models.ReferencedPackages()...) + pkgs = append(pkgs, c.AutoBind...) + return pkgs +} + +func (c *Config) ReloadAllPackages() { + c.Packages.ReloadAll(c.packageList()...) +} + func (c *Config) injectTypesFromSchema() error { c.Directives["goModel"] = DirectiveConfig{ SkipRuntime: true, diff --git a/internal/code/packages.go b/internal/code/packages.go index b14c45ad276..6a88361f724 100644 --- a/internal/code/packages.go +++ b/internal/code/packages.go @@ -26,6 +26,13 @@ type Packages struct { numNameCalls int // stupid test steam. ignore. } +// ReloadAll will call LoadAll after clearing the package cache, so we can reload +// packages in the case that the packages have changed +func (p *Packages) ReloadAll(importPaths ...string) []*packages.Package { + p.packages = nil + return p.LoadAll(importPaths...) +} + // LoadAll will call packages.Load and return the package data for the given packages, // but if the package already have been loaded it will return cached values instead. func (p *Packages) LoadAll(importPaths ...string) []*packages.Package { diff --git a/plugin/modelgen/models.go b/plugin/modelgen/models.go index e0ca186632f..abaac1d2c80 100644 --- a/plugin/modelgen/models.go +++ b/plugin/modelgen/models.go @@ -214,13 +214,22 @@ func (m *Plugin) MutateConfig(cfg *config.Config) error { b = m.MutateHook(b) } - return templates.Render(templates.Options{ + err := templates.Render(templates.Options{ PackageName: cfg.Model.Package, Filename: cfg.Model.Filename, Data: b, GeneratedHeader: true, Packages: cfg.Packages, }) + if err != nil { + return err + } + + // We may have generated code in a package we already loaded, so we reload all packages + // to allow packages to be compared correctly + cfg.ReloadAllPackages() + + return nil } func isStruct(t types.Type) bool {