Add support for named GraphQL operations

[mislav]

For purposes of both server-side metrics and stubbing of HTTP requests in client-side tests, we need all GraphQL operations to be named; e.g.

query FindUser{...}
mutation AddReaction{...}

This library did not support any facilities to name queries. Furthermore, adding an extra name argument to current methods would compromise backwards compatibility. #12

This proposes a declarative, backwards-compatible interface for naming queries:

// old, unnamed queries
client.Query(context.Background(), &q, vars)

// named queries
client.Query(context.Background(), graphql.NamedOperation("FindUser", &q), vars)
client.Mutate(context.Background(), graphql.NamedOperation("AddReaction", &m), vars)

[dmitshur]

Thanks for the PR. I'll try to get to reviewing it, but it may not happen right away.

I hope you don't mind continuing to use your fork in the meantime, and please let me know if you learn anything more about this API and approach as you use it.

[mislav]

Sure, take your time! We are already using a fork in GitHub CLI.

[anagrius]

LGTM

[jdoss]

Can we perchance get this merged? GitHub CLI is using a fork of this and it is making things difficult w/r/t packaging the GitHub CLI in Fedora.

[parkr]

LGTM. @dmitshur, are you happy with this PR?

[bdentino]

I'm having a hard time getting this to work, but I am a golang novice so I'm not sure if it's something I'm doing wrong. It seems that client.Query(ctx, &q, vars) is still trying to unmarshal the GraphQL response data directly into the 2nd function argument, which doesn't work when you pass in a NamedOperation. Am I missing something?

[mislav]

@bdentino Apologies; the deserialization was never actually tested. I've mostly submitted it as a suggestion of a potential approach.

I've just pushed a commit that deserializes to the correct struct. Try it out!

[obukhov]

This one would be nice to have merged. I think it is general GraphQL best practice to have operations named.

[rocktavious]

+1 on merging this - i need this functionality

[m3libea]

+1 on merging this!

[fillup]

@dmitshur any chance this could be merged soon? Atlassian will soon be requiring named operations for calls to their GraphQL APIs which will break our integrations and force us to maintain/use a fork of this or do something else. It sounds like from the comments here that this is well tested and ready to go.

All responses from Atlassian's GraphQL API right now include:

{
  "errors": [
    {
      "message": "Warning : You MUST provide a query operation name.  In the future operations without names will be banned.",
      "extensions": {
        "statusCode": 400,
        "classification": "ExecutionAborted"
      }
    }
  ]
}

Fortunately they still return the response data I'm asking for so I'm able to inspect the error and ignore it in this case, but that's an ugly band aid that won't last for very long.