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

Add option for including operationName as a query parameter #298

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ When releasing a new version:

### New features:

- The new `graphql.WithOperationNameParam` allows clients to be created that include `operationName` as a query parameter.
- The new `optional: generic` allows using a generic type to represent optionality. See the [documentation](genqlient.yaml) for details.
- For schemas with enum values that differ only in casing, it's now possible to disable smart-casing in genqlient.yaml; see the [documentation](genqlient.yaml) for `casing` for details.

Expand Down
13 changes: 13 additions & 0 deletions docs/FAQ.md
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,19 @@ The URL requested will be:

The client does not support mutations, and will return an error if passed a request that attempts one.

### … include operationName as a query parameter?

`graphql.NewClient` support the option `graphql.WithOperationNameParam`, which will add `?operationName=...` to the request's query parameters.

```go
ctx := context.Background()
client := graphql.NewClient("https://api.github.com/graphql", http.DefaultClient, graphql.WithOperationNameParam)
resp, err := getUser(ctx, client, "benjaminjkraft")
fmt.Println(resp.User.Name, err)
```



### … use an API that requires authentication?

When you call `graphql.NewClient`, pass in an HTTP client that adds whatever authentication headers you need (typically by wrapping the client's `Transport`). For example:
Expand Down
47 changes: 39 additions & 8 deletions graphql/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,15 @@ type Client interface {
}

type client struct {
httpClient Doer
endpoint string
method string
httpClient Doer
endpoint string
method string
withOperationNameParam bool
}

// WithOperationNameParam allows operationName to be included as a query parameter.
func WithOperationNameParam(c *client) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please add a named type for the options so they show up nicely in the documentation

c.withOperationNameParam = true
}

// NewClient returns a [Client] which makes requests to the given endpoint,
Expand All @@ -58,8 +64,8 @@ type client struct {
// example.
//
// [example/main.go]: https://github.com/Khan/genqlient/blob/main/example/main.go#L12-L20
func NewClient(endpoint string, httpClient Doer) Client {
return newClient(endpoint, httpClient, http.MethodPost)
func NewClient(endpoint string, httpClient Doer, options ...func(*client)) Client {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically a breaking change but probably one we can get away with

return newClient(endpoint, httpClient, http.MethodPost, options...)
}

// NewClientUsingGet returns a [Client] which makes GET requests to the given
Expand All @@ -83,11 +89,19 @@ func NewClientUsingGet(endpoint string, httpClient Doer) Client {
return newClient(endpoint, httpClient, http.MethodGet)
}

func newClient(endpoint string, httpClient Doer, method string) Client {
func newClient(endpoint string, httpClient Doer, method string, options ...func(*client)) Client {
if httpClient == nil || httpClient == (*http.Client)(nil) {
httpClient = http.DefaultClient
}
return &client{httpClient, endpoint, method}
c := &client{
httpClient: httpClient,
endpoint: endpoint,
method: method,
}
for _, opt := range options {
opt(c)
}
return c
}

// Doer encapsulates the methods from [*http.Client] needed by [Client].
Expand Down Expand Up @@ -178,14 +192,31 @@ func (c *client) MakeRequest(ctx context.Context, req *Request, resp *Response)
}

func (c *client) createPostRequest(req *Request) (*http.Request, error) {
parsedURL, err := url.Parse(c.endpoint)
if err != nil {
return nil, err
}

queryParams := parsedURL.Query()
queryUpdated := false

if c.withOperationNameParam && req.OpName != "" {
queryParams.Set("operationName", req.OpName)
queryUpdated = true
}

if queryUpdated {
parsedURL.RawQuery = queryParams.Encode()
}

body, err := json.Marshal(req)
if err != nil {
return nil, err
}

httpReq, err := http.NewRequest(
c.method,
c.endpoint,
parsedURL.String(),
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is parse/unparse guaranteed to roundtrip a valid URL without changes? Unless we're sure, it's probably a tad safer if we change queryUpdated to urlUpdated and use the URL exactly as specified if we don't need to change it.

bytes.NewReader(body))
if err != nil {
return nil, err
Expand Down
1 change: 1 addition & 0 deletions internal/integration/integration_test.go
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add at least a quick test (either unit or integration, not sure which will be easier to wire up) that the option actually does something!

Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ func TestVariables(t *testing.T) {
// worry about it.
clients := []graphql.Client{
graphql.NewClient(server.URL, http.DefaultClient),
graphql.NewClient(server.URL, http.DefaultClient, graphql.WithOperationNameParam),
graphql.NewClientUsingGet(server.URL, http.DefaultClient),
}

Expand Down
16 changes: 15 additions & 1 deletion internal/integration/roundtrip.go
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,11 @@ func (c *roundtripClient) MakeRequest(ctx context.Context, req *graphql.Request,
}

func newRoundtripClients(t *testing.T, endpoint string) []graphql.Client {
return []graphql.Client{newRoundtripClient(t, endpoint), newRoundtripGetClient(t, endpoint)}
return []graphql.Client{
newRoundtripClient(t, endpoint),
newRoundtripClientWithOptions(t, endpoint),
newRoundtripGetClient(t, endpoint),
}
}

func newRoundtripClient(t *testing.T, endpoint string) graphql.Client {
Expand All @@ -117,6 +121,16 @@ func newRoundtripClient(t *testing.T, endpoint string) graphql.Client {
}
}

func newRoundtripClientWithOptions(t *testing.T, endpoint string) graphql.Client {
transport := &lastResponseTransport{wrapped: http.DefaultTransport}
httpClient := &http.Client{Transport: transport}
return &roundtripClient{
wrapped: graphql.NewClient(endpoint, httpClient, graphql.WithOperationNameParam),
transport: transport,
t: t,
}
}

func newRoundtripGetClient(t *testing.T, endpoint string) graphql.Client {
transport := &lastResponseTransport{wrapped: http.DefaultTransport}
httpClient := &http.Client{Transport: transport}
Expand Down
Loading