Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cannot map Go interface type to schema interface/union type #42

Closed
ereyes01 opened this issue Mar 7, 2018 · 4 comments
Closed

Cannot map Go interface type to schema interface/union type #42

ereyes01 opened this issue Mar 7, 2018 · 4 comments
Labels
bug Something isn't working

Comments

@ereyes01
Copy link

ereyes01 commented Mar 7, 2018

I am hitting an issue where I cannot get the gqlgen generated code to build when I map a schema interface type to a Go interface.

Below is an example of what I'm talking about. Full code and detailed instructions to reproduce are at https://github.com/ereyes01/gqlgen-interface-bug

Suppose I have this schema:

schema {
    query: Query
}

type Query {
    shapes(): [Shape]
}

interface Shape {
    area(): Float
}

type Circle implements Shape {
    radius: Float
    area() : Float
}

type Rectangle implements Shape {
    length: Float
    width: Float
    area(): Float
}

And suppose I have the following corresponding Go types:

package shapes

import "math"

type Shape interface {
	Area() float64
}

type Circle struct {
	Radius float64
}

func (c *Circle) Area() float64 {
	return c.Radius * math.Pi * math.Pi
}

type Rectangle struct {
	Length, Width float64
}

func (r *Rectangle) Area() float64 {
	return r.Length * r.Width
}

So let's map our types as follows:

{
    "Shape": "github.com/ereyes01/gqlgen-interface-bug/shapes.Shape",
    "Circle": "github.com/ereyes01/gqlgen-interface-bug/shapes.Circle",
    "Rectangle": "github.com/ereyes01/gqlgen-interface-bug/shapes.Rectangle"
}

NOTE: ^^ This uses the path to my repository given above as if I'd obtained it via go get.

So from this we can now create our generated.go as follows:

$ gqlgen -out generated.go -package shapes -typemap types.json schema.graphql

... and that works fine. We now implement the needed resolver:

package shapes

import "context"

type ShapeResolver struct{}

func (r *ShapeResolver) Query_shapes(ctx context.Context) ([]Shape, error) {
	return []Shape{
		&Circle{Radius: 10.0},
		&Rectangle{Length: 1.0, Width: 10.0},
		&Rectangle{Length: 10.0, Width: 10.0},
	}, nil
}

... And I confirmed that the code generated for Shape.area() looks correct- it just calls the Area() Go method on the Go type.

So now let's say that inside shapes/server we place a simple Playground server:

package main

import (
	"log"
	"net/http"

	"github.com/ereyes01/gqlgen-interface-bug/shapes"
	"github.com/vektah/gqlgen/handler"
)

func main() {
	http.Handle("/", handler.Playground("Shapes", "/query"))
	http.Handle("/query", handler.GraphQL(shapes.MakeExecutableSchema(new(shapes.ShapeResolver))))

	log.Fatal(http.ListenAndServe(":9090", nil))
}

If I try to build that via go install I get a compile error in the generated code:

$ go install github.com/ereyes01/gqlgen-interface-bug/shapes/server
# github.com/ereyes01/gqlgen-interface-bug/shapes
shapes/generated.go:711:2: impossible type switch case: *obj (type Shape) cannot have dynamic type Circle (Area method has pointer receiver)
shapes/generated.go:716:2: impossible type switch case: *obj (type Shape) cannot have dynamic type Rectangle (Area method has pointer receiver)

If we go have a look at that part of the generated code, we have:

func (ec *executionContext) _Shape(sel []query.Selection, obj *Shape) graphql.Marshaler {
	switch obj := (*obj).(type) {
	case nil:
		return graphql.Null
	case Circle:
		return ec._Circle(sel, &obj)

	case *Circle:
		return ec._Circle(sel, obj)
	case Rectangle:
		return ec._Rectangle(sel, &obj)

	case *Rectangle:
		return ec._Rectangle(sel, obj)
	default:
		panic(fmt.Errorf("unexpected type %T", obj))
	}
}

Of course, the case Circle and case Rectangle are not valid when type-switching on the Shape type, as those types do not implement the Shape interface (the pointers to those types do).

I could work around this by commenting out the case Circle and case Rectangle sections, and then everything works fine.

I investigated why this code was generated, and I found that in the template for the interface type, it just blindly writes out a case for both the Go type and its pointer:

{{- $interface := . }}

func (ec *executionContext) _{{$interface.GQLType}}(sel []query.Selection, obj *{{$interface.FullName}}) graphql.Marshaler {
	switch obj := (*obj).(type) {
	case nil:
		return graphql.Null
	{{- range $implementor := $interface.Implementors }}
	case {{$implementor.FullName}}:
		return ec._{{$implementor.GQLType}}(sel, &obj)

	case *{{$implementor.FullName}}:
		return ec._{{$implementor.GQLType}}(sel, obj)

	{{- end }}
	default:
		panic(fmt.Errorf("unexpected type %T", obj))
	}
}

This generated will only ever work if:

  • The matching Go interface type is the empty interface (interface{})
  • Both the type and its pointer implement the matching Go interface

... which doesn't sound right if you're going to allow the interface type to be matched to your own type.

I'm not immediately sure what the best way to fix this might be. One way might be to look into whether the Go type system can tell you which are all the implementing types of an interface (I haven't really investigated that rabbit hole).

Another approach might be to allow us to tell you the implementing types in the types.json file. Of course, that will further complicate the format of that JSON file, but it seems otherwise robust at first glance.

Thanks for taking a look.

@vektah
Copy link
Collaborator

vektah commented Mar 8, 2018

Of course, the case Circle and case Rectangle are not valid when type-switching on the Shape type, as those types do not implement the Shape interface (the pointers to those types do).

in this case they aren't, but if they accepted non pointer receivers, eg:

func (c Circle) Area() float64 {
	return c.Radius * math.Pi * math.Pi
}

then it would be valid to switch on case Circle:

I'm not immediately sure what the best way to fix this might be. One way might be to look into whether the Go type system can tell you which are all the implementing types of an interface (I haven't really investigated that rabbit hole).

I think this has to be the answer.

Another approach might be to allow us to tell you the implementing types in the types.json file. Of course, that will further complicate the format of that JSON file, but it seems otherwise robust at first glance.

You already know the implementing types, the schema maps them out for you.

What probably needs to be added is a bindInterface pass after buildInterface.

eg for Objects

And the interface implementor struct probably needs a custom type to carry this extra info into the template.

If you want to continue your deep dive, a PR it would be appreciated. Otherwise I'll take a look when I get a chance.

@ereyes01
Copy link
Author

Thanks @vektah, really appreciate the fix! This was still a few items deep in my todo list...

However, while the issue seems to have been fixed for the graphql interface type, it seems to still persist for the union type. I have added a union type mapped to a Go interface in my sample repo that reproduces this problem with the latest code: ereyes01/gqlgen-interface-bug@ad9648b

Let me know if you will reopen this, or if you need a new issue. Thanks!

@vektah
Copy link
Collaborator

vektah commented Mar 17, 2018

Should be fixed in 15b3af2

@ereyes01
Copy link
Author

That did it, thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants