Support specifying operation names.

[dmitshur]

Operation names are described at http://graphql.org/learn/queries/#operation-name:

The operation name is a meaningful and explicit name for your operation. It can be very useful for debugging and server-side logging reasons. When something goes wrong either in your network logs or your GraphQL server, it is easier to identify a query in your codebase by name instead of trying to decipher the contents. Think of this just like a function name in your favorite programming language. For example, in JavaScript we can easily work only with anonymous functions, but when we give a function a name, it's easier to track it down, debug our code, and log when it's called. In the same way, GraphQL query and mutation names, along with fragment names, can be a useful debugging tool on the server side to identify different GraphQL requests.

Notably, it's most useful to be able to provide operation names for server-side logging and debugging reasons.

We should support this.

[dmitshur]

@seanpile has already created PR #8 that resolves this issue. In it, he mentioned:

This is a backward compatible change, although you could argue that operation name should be a first class citizen on the Query/Mutate methods, but that would break existing clients.

This package is still in an early period where we can change the API (see README), so what we need to do is determine which would be the overall best API.

During the process of reviewing that PR, I've considered these two alternatives:

1. Single method that takes name string parameter:

   // Query executes a GraphQL query operation,
   // with the query derived from q, populating the response into it.
   // q should be a pointer to a struct that corresponds to the GraphQL schema.
   //
   // A meaningful operation name is useful for server-side logging, but can be left empty.
   func (c *Client) Query(ctx context.Context, name string, q interface{}, variables map[string]interface{}) error { ... }

   As the documentation says, name can be left empty, which reproduces today's behavior of making queries without a name.

2. Two separate methods. One for nameless queries, and another for named ones:

   // Query executes a GraphQL query operation,
   // with the query derived from q, populating the response into it.
   // q should be a pointer to a struct that corresponds to the GraphQL schema.
   func (c *Client) Query(ctx context.Context, q interface{}, variables map[string]interface{}) error { ... }
   
   // NamedQuery executes a named GraphQL query operation,
   // with the query derived from q, populating the response into it.
   // q should be a pointer to a struct that corresponds to the GraphQL schema.
   //
   // A meaningful operation name is useful for server-side logging.
   func (c *Client) NamedQuery(ctx context.Context, name string, q interface{}, variables map[string]interface{}) error { ... }

   This option raises the question of what should happen if NamedQuery is called with an empty string name parameter. Should that be accepted and behave just like Query? Or should it return an error saying name must not be empty?

Same decision would apply to Mutate method.

At this point, I'm quite torn between the two options. Which is a better API? Feedback appreciated.

[seanpile]

👍 for case 1, documenting that an empty string will generate a query without a name. Perhaps an exported const (e.g., AnonymousQuery). If the API was frozen, I'd opt for 2 but since it's not, and since the change for clients is minimal, I would go option 1.

[eric-am]

I'd like to put a third option on the table: an "options" vararg.

func (c *Client) Query(ctx context.Context, q interface{}, variables map[string]interface{}, opts Option...) error { ... }

Usage is then roughly like:

client.Query(ctx, &structure, variables) // no name
client.Query(ctx, &structure, variables, graphql.NamedQuery("foobaz")) // named

The Cheney-style "functional options" pattern is one nice way of implementing this. An interface with an unexported marker function, a handful of structs implementing it, and a type-switch inside the Query method has the same effect; they're just coded out a little differently.

There are two reasons I like this option for this particular issue: because naming queries is indeed optional; and also, naming a query is not a dramatic change in how the function behaves (and thus, making a whole new function seems to communicate more significance than there is).