From 9f8e2b262fc908a9e4846dedb96fa82e07c1bb72 Mon Sep 17 00:00:00 2001 From: Craig Silverstein Date: Mon, 26 Feb 2024 11:32:48 -0800 Subject: [PATCH] Fix non-deterministic generated code involving interfaces and fragments. (#323) In #209, @vikstrous2 added a bugfix to sort the list of interfaces when generating code, so the generated output was stable. This worked great, but there was one more case where we need to do that, involving interfaces in fragments. I have: - [x] Written a clear PR title and description (above) - [x] Signed the [Khan Academy CLA](https://www.khanacademy.org/r/cla) - [x] Added tests covering my changes, if applicable - [ ] Included a link to the issue fixed, if applicable - [ ] Included documentation, for new features - [x] Added an entry to the changelog --- docs/CHANGELOG.md | 3 ++- generate/convert.go | 2 ++ ...s.graphql-ComplexNamedFragments.graphql.go | 22 +++++++++---------- ...enerate-Flatten.graphql-Flatten.graphql.go | 22 +++++++++---------- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index ab9b287c..62016434 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -36,8 +36,9 @@ Note that genqlient is now tested from Go 1.20 through Go 1.22. ### Bug fixes: - The presence of negative pointer directives, i.e., `# @genqlient(pointer: false)` are now respected even in the when `optional: pointer` is set in the configuration file. - Made name collisions between query/mutation arguments and local function variables less likely. -- Fix generation issue related to golang type implementation of complex graphql union fragments +- Fix generation issue related to golang type implementation of complex graphql union fragments. - Bind correctly to types in the same package as the generated code. +- Fix non-deterministic generated code when querying graphql interfaces via named fragments. ## v0.6.0 diff --git a/generate/convert.go b/generate/convert.go index b7e6a6d2..3e3dc03f 100644 --- a/generate/convert.go +++ b/generate/convert.go @@ -841,6 +841,8 @@ func (g *generator) convertNamedFragment(fragment *ast.FragmentDefinition) (goTy return goType, nil case ast.Interface, ast.Union: implementationTypes := g.schema.GetPossibleTypes(typ) + // Make sure we generate stable output by sorting the types by name when we get them + sort.Slice(implementationTypes, func(i, j int) bool { return implementationTypes[i].Name < implementationTypes[j].Name }) goType := &goInterfaceType{ GoName: fragment.Name, SharedFields: fields, diff --git a/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go b/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go index 7888b2e8..b7fa016d 100644 --- a/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-ComplexNamedFragments.graphql-ComplexNamedFragments.graphql.go @@ -273,8 +273,8 @@ func (v *ComplexNamedFragmentsWithInlineUnionUser) __premarshalJSON() (*__premar // // ContentFields is implemented by the following types: // ContentFieldsArticle -// ContentFieldsVideo // ContentFieldsTopic +// ContentFieldsVideo type ContentFields interface { implementsGraphQLInterfaceContentFields() // GetName returns the interface-field "name" from its implementation. @@ -284,8 +284,8 @@ type ContentFields interface { } func (v *ContentFieldsArticle) implementsGraphQLInterfaceContentFields() {} -func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields() {} func (v *ContentFieldsTopic) implementsGraphQLInterfaceContentFields() {} +func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields() {} func __unmarshalContentFields(b []byte, v *ContentFields) error { if string(b) == "null" { @@ -304,12 +304,12 @@ func __unmarshalContentFields(b []byte, v *ContentFields) error { case "Article": *v = new(ContentFieldsArticle) return json.Unmarshal(b, *v) - case "Video": - *v = new(ContentFieldsVideo) - return json.Unmarshal(b, *v) case "Topic": *v = new(ContentFieldsTopic) return json.Unmarshal(b, *v) + case "Video": + *v = new(ContentFieldsVideo) + return json.Unmarshal(b, *v) case "": return fmt.Errorf( "response was missing Content.__typename") @@ -331,20 +331,20 @@ func __marshalContentFields(v *ContentFields) ([]byte, error) { *ContentFieldsArticle }{typename, v} return json.Marshal(result) - case *ContentFieldsVideo: - typename = "Video" + case *ContentFieldsTopic: + typename = "Topic" result := struct { TypeName string `json:"__typename"` - *ContentFieldsVideo + *ContentFieldsTopic }{typename, v} return json.Marshal(result) - case *ContentFieldsTopic: - typename = "Topic" + case *ContentFieldsVideo: + typename = "Video" result := struct { TypeName string `json:"__typename"` - *ContentFieldsTopic + *ContentFieldsVideo }{typename, v} return json.Marshal(result) case nil: diff --git a/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go b/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go index c05e913d..76499bc7 100644 --- a/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go +++ b/generate/testdata/snapshots/TestGenerate-Flatten.graphql-Flatten.graphql.go @@ -30,8 +30,8 @@ func (v *ChildVideoFields) GetName() string { return v.Name } // // ContentFields is implemented by the following types: // ContentFieldsArticle -// ContentFieldsVideo // ContentFieldsTopic +// ContentFieldsVideo type ContentFields interface { implementsGraphQLInterfaceContentFields() // GetName returns the interface-field "name" from its implementation. @@ -41,8 +41,8 @@ type ContentFields interface { } func (v *ContentFieldsArticle) implementsGraphQLInterfaceContentFields() {} -func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields() {} func (v *ContentFieldsTopic) implementsGraphQLInterfaceContentFields() {} +func (v *ContentFieldsVideo) implementsGraphQLInterfaceContentFields() {} func __unmarshalContentFields(b []byte, v *ContentFields) error { if string(b) == "null" { @@ -61,12 +61,12 @@ func __unmarshalContentFields(b []byte, v *ContentFields) error { case "Article": *v = new(ContentFieldsArticle) return json.Unmarshal(b, *v) - case "Video": - *v = new(ContentFieldsVideo) - return json.Unmarshal(b, *v) case "Topic": *v = new(ContentFieldsTopic) return json.Unmarshal(b, *v) + case "Video": + *v = new(ContentFieldsVideo) + return json.Unmarshal(b, *v) case "": return fmt.Errorf( "response was missing Content.__typename") @@ -88,20 +88,20 @@ func __marshalContentFields(v *ContentFields) ([]byte, error) { *ContentFieldsArticle }{typename, v} return json.Marshal(result) - case *ContentFieldsVideo: - typename = "Video" + case *ContentFieldsTopic: + typename = "Topic" result := struct { TypeName string `json:"__typename"` - *ContentFieldsVideo + *ContentFieldsTopic }{typename, v} return json.Marshal(result) - case *ContentFieldsTopic: - typename = "Topic" + case *ContentFieldsVideo: + typename = "Video" result := struct { TypeName string `json:"__typename"` - *ContentFieldsTopic + *ContentFieldsVideo }{typename, v} return json.Marshal(result) case nil: