Use Struct Field Resolver instead of Method

[0xSalman]

This fixes graph-gophers#28. Essentially, it reduces lot of boilerplate code by making use of struct field as resolvers instead of creating methods for each struct field.

Now, methods are required only when

1. a struct implements a GraphQL interface. In this case, you create methods for fields which overlap
2. a struct exposes a GraphQL field that accepts additional arguments i.e., first: Int, last: Int etc.
3. there is GraphQL union type

By using struct fields as resolvers, one could also benefit from DRY codebase by not having a pair of a struct which look the same (one for GraphQL and one for DB)

Does this break existing API?

No. This change is completely transparent and you can continue using your existing codebase.

Can I still continue using methods as resolvers?

Yes, by default, it uses method resolvers.

How can I use struct fields as resolvers?

When invoking graphql.ParseSchema() or graphql.MustParseSchema() to parse the schema, you pass in graphql.UseFieldResolvers() as an argument. For an example, take a look at ./example/social/server/server.go

Additional Notes

• also fixes Allow returning string for an ID field? graph-gophers/graphql-go#124
• for an example, take a look at ./example/social/social.go & ./example/social/server/server.go
• Unit tests are missing

[edlau]

Visually inspected two passes, looks good! PR notes is good too!

[Niennienzz]

Digged a bit into the library yesterday for learning. I tried the method way and the struct field way, and both servers worked nicely. I just have a very subtle suggestion in the example code that:

errors.New(fmt.Sprintf( ... ))

could be replaced by a shortcut:

fmt.Errorf( ... )

at lines 160 and 168 of example/social/social.go.

We will probably use Salman's error handling library a lot, so anyways. 😃

[0xSalman]

@Niennienzz good suggestion - I will make the change

[dabfleming]

Still reviewing but so far I have one suggestion. Instead of adding config.Config, why not build on the existing functional options using SchemaOpt? This would have the advantage of not requiring any breaking changes or the addition of the config package.

For example, expand graphql.Schema like so

// Schema represents a GraphQL schema with an optional resolver.
type Schema struct {
	schema *schema.Schema
	res    *resolvable.Schema

	maxParallelism    int
	tracer            trace.Tracer
	logger            log.Logger
	useFieldResolvers bool
}

and add

// UseFieldResovlers opts in to using struct field resolvers
func UseFieldResolvers() SchemaOpt {
	return func(s *Schema) {
		s.useFieldResolvers = true
	}
}

It's simpler, non-breaking, and the zero value, useFieldResolvers == false represents the current behaviour. You could then use field resolvers by:
ParseSchema(schema, resolver, UseFieldResolvers()) similar to passing options like WithInsecure() to grpc.

[0xSalman]

@dabfleming

That's a very good suggestion. I initially explored that option. I don't remember now why I didn't continue with it. I would, however, keep Config and make it more generic because I am sure we will make use of it when we have to configure things like query's maxDepth, cost etc.

I am thinking something like this:

package graphql
...

func AddConfiguration(conf *config.Config) SchemaOpt {
    return func(s *Schema) {
        s.Config = conf
    }
}

and then

ParseSchema(schema, resolver, graphql.AddConfiguration(&config.Config{}))

[0xSalman]

David,

I went ahead and refactored the code as per your suggestion. It seems to be a better solution than my proposal of using a config struct.

[dabfleming]

@salmana1 3rd param still needs to be removed here.

[0xSalman]

fixed

[dabfleming]

Instead of adding global state, how about adding a field to schema.Schema? There's a pointer to it everywhere useFieldResolvers is read: both in this func and as a member of resolvable.execBuilder (other usages are in methods on execBuilder).

[0xSalman]

fixed

[Niennienzz]

I went through it again. I really like the way you interactively worked on this one.