Skip to content

Commit

Permalink
Add support for interfaces, part 1: the simplest cases
Browse files Browse the repository at this point in the history
In this commit I begin the journey to add the long-awaited support for
interfaces (part of #8).  Well, it's not the beginning: I already had
some half-written broken code around.  But it's the first fully
functional support, and especially, the first *tested* support; it's
probably best to review the nontrivially-changed code as if it were new.

Conceptually, the code so far is pretty simple: we generate an interface
type, and the implementations.  (That code is in fact mostly unchanged.)
The complexity comes in because encoding/json doesn't know how to
unmarshal that.  So we have to add an UnmarshalJSON method, which
actually has to be on the types with interface-type fields, that knows
how.  I factored it into two methods, such that that UnmarshalJSON
method is just glue, and then there's a separate function, corresponding
to each interface-type, that actually does all the work.  (If only one
could just write it as an actual method!)  The method uses the same
trick suggested to me by a few others in another context to deserialize
all but one field, then handle that field specially, which is discussed
in the code.

This still has some limitations, which will be lifted in future commits:
- it doesn't allow for list-of-interface fields
- it requires that you manually ask for `__typename`
- it doesn't support fragments, i.e. you can only query for interface
  fields, not concrete-type-specific ones
But it works, even in integration tests, which is progress!

As a part of this, I added a proper config option for the "allow broken
features" flag, since I need to be able to set it from the integration
tests which are in a separate package (and actually shell out via `go
generate`).  I also renamed what was to be the first case
(InterfaceNoFragments), and replaced it with a further-simplified
version (avoiding list-of-interface fields.

[1] https://github.com/benjaminjkraft/notes/blob/master/go-json-interfaces.md

Issue: #8

Test plan: make tesc
  • Loading branch information
benjaminjkraft committed Aug 23, 2021
1 parent c58bb03 commit 7496cbf
Show file tree
Hide file tree
Showing 31 changed files with 1,109 additions and 186 deletions.
7 changes: 7 additions & 0 deletions generate/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,13 @@ type Config struct {
// and UnmarshalJSON methods, or otherwise be convertible to JSON.
Scalars map[string]string `yaml:"scalars"`

// Set to true to use features that aren't fully ready to use.
//
// This is primarily intended for genqlient's own tests. These features
// are likely BROKEN and come with NO EXPECTATION OF COMPATIBBILITY. Use
// them at your own risk!
AllowBrokenFeatures bool `yaml:"allow_broken_features"`

// Set automatically to the filename of the config file itself.
configFilename string
}
Expand Down
17 changes: 15 additions & 2 deletions generate/convert.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (g *generator) baseTypeForOperation(operation ast.Operation) (*ast.Definiti
case ast.Mutation:
return g.schema.Mutation, nil
case ast.Subscription:
if !allowBrokenFeatures {
if !g.Config.AllowBrokenFeatures {
return nil, errorf(nil, "genqlient does not yet support subscriptions")
}
return g.schema.Subscription, nil
Expand Down Expand Up @@ -255,10 +255,23 @@ func (g *generator) convertDefinition(
return goType, nil

case ast.Interface, ast.Union:
if !allowBrokenFeatures {
if !g.Config.AllowBrokenFeatures {
return nil, errorf(pos, "not implemented: %v", def.Kind)
}

// We need to request __typename so we know which concrete type to use.
hasTypename := false
for _, selection := range selectionSet {
field, ok := selection.(*ast.Field)
if ok && field.Name == "__typename" {
hasTypename = true
}
}
if !hasTypename {
// TODO(benkraft): Instead, modify the query to add __typename.
return nil, errorf(pos, "union/interface type %s must request __typename", def.Name)
}

implementationTypes := g.schema.GetPossibleTypes(def)
goType := &goInterfaceType{
GoName: name,
Expand Down
5 changes: 1 addition & 4 deletions generate/generate.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,6 @@ import (
"golang.org/x/tools/imports"
)

// Set to true to test features that aren't yet really ready.
var allowBrokenFeatures = false

// generator is the context for the codegen process (and ends up getting passed
// to the template).
type generator struct {
Expand Down Expand Up @@ -230,7 +227,7 @@ func Generate(config *Config) (map[string][]byte, error) {
strings.Join(config.Operations, ", "))
}

if len(document.Fragments) > 0 && !allowBrokenFeatures {
if len(document.Fragments) > 0 && !config.AllowBrokenFeatures {
return nil, errorf(document.Fragments[0].Position,
"genqlient does not yet support fragments")
}
Expand Down
14 changes: 5 additions & 9 deletions generate/generate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,6 @@ const (
// update the snapshots. Make sure to check that the output is sensible; the
// snapshots don't even get compiled!
func TestGenerate(t *testing.T) {
// we can test parts of features even if they're not done yet!
allowBrokenFeatures = true

files, err := ioutil.ReadDir(dataDir)
if err != nil {
t.Fatal(err)
Expand All @@ -59,6 +56,7 @@ func TestGenerate(t *testing.T) {
"Junk": "interface{}",
"ComplexJunk": "[]map[string]*[]*map[string]interface{}",
},
AllowBrokenFeatures: true,
})
if err != nil {
t.Fatal(err)
Expand All @@ -73,9 +71,9 @@ func TestGenerate(t *testing.T) {
t.Run("Build", func(t *testing.T) {
if testing.Short() {
t.Skip("skipping build due to -short")
} else if sourceFilename == "InterfaceNesting.graphql" ||
sourceFilename == "InterfaceNoFragments.graphql" ||
sourceFilename == "Omitempty.graphql" {
} else if sourceFilename == "InterfaceNesting.graphql" || // #8
sourceFilename == "InterfaceListField.graphql" || // #8
sourceFilename == "Omitempty.graphql" { // #43
t.Skip("TODO: enable these once they build")
}

Expand Down Expand Up @@ -109,9 +107,6 @@ func TestGenerate(t *testing.T) {
}

func TestGenerateErrors(t *testing.T) {
// we can test parts of features even if they're not done yet!
allowBrokenFeatures = true

files, err := ioutil.ReadDir(errorsDir)
if err != nil {
t.Fatal(err)
Expand All @@ -138,6 +133,7 @@ func TestGenerateErrors(t *testing.T) {
"ValidScalar": "string",
"InvalidScalar": "bogus",
},
AllowBrokenFeatures: true,
})
if err == nil {
t.Fatal("expected an error")
Expand Down
5 changes: 5 additions & 0 deletions generate/testdata/errors/MissingTypeName.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
package errors

const _ = `# @genqlient
query MyQuery { i { f } }
`
1 change: 1 addition & 0 deletions generate/testdata/errors/MissingTypeName.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
query MyQuery { i { f } }
11 changes: 11 additions & 0 deletions generate/testdata/errors/MissingTypeName.schema.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
type Query {
i: I
}

type T implements I {
f: String!
}

interface I {
f: String!
}
11 changes: 11 additions & 0 deletions generate/testdata/queries/InterfaceListField.graphql
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
query InterfaceNoFragmentsQuery {
root {
id
name
children {
__typename
id
name
}
}
}
3 changes: 3 additions & 0 deletions generate/testdata/queries/InterfaceNesting.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,13 @@ query InterfaceNesting {
root {
id
children {
__typename
id
parent {
__typename
id
children {
__typename
id
}
}
Expand Down
10 changes: 2 additions & 8 deletions generate/testdata/queries/InterfaceNoFragments.graphql
Original file line number Diff line number Diff line change
@@ -1,10 +1,4 @@
query InterfaceNoFragmentsQuery {
root {
id
name
children {
id
name
}
}
root { id name } # (make sure sibling fields work)
randomItem { __typename id name }
}
1 change: 1 addition & 0 deletions generate/testdata/queries/schema.graphql
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ type Query {
"""usersWithRole looks a user up by role."""
usersWithRole(role: Role!): [User!]!
root: Topic!
randomItem: Content!
randomLeaf: LeafContent!
convert(dt: DateTime!, tz: String): DateTime!
maybeConvert(dt: DateTime, tz: String): DateTime
Expand Down

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"operations": [
{
"operationName": "InterfaceNoFragmentsQuery",
"query": "\nquery InterfaceNoFragmentsQuery {\n\troot {\n\t\tid\n\t\tname\n\t\tchildren {\n\t\t\t__typename\n\t\t\tid\n\t\t\tname\n\t\t}\n\t}\n}\n",
"sourceLocation": "testdata/queries/InterfaceListField.graphql"
}
]
}
Loading

0 comments on commit 7496cbf

Please sign in to comment.