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

feat: add WithHeader & WithCookie #48

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
/vendor
coverage.txt
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@ install:
script:
- go get -t -v ./...
- diff -u <(echo -n) <(gofmt -d -s .)
- go tool vet .
- go vet .
- go test -v -race ./...
8 changes: 8 additions & 0 deletions go.mod
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
module github.com/shurcooL/graphql

go 1.13

require (
github.com/graph-gophers/graphql-go v0.0.0-20191115155744-f33e81362277
github.com/stretchr/testify v1.4.0 // indirect
)
15 changes: 15 additions & 0 deletions go.sum
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
github.com/davecgh/go-spew v1.1.0 h1:ZDRjVQ15GmhC3fiQ8ni8+OwkZQO4DARzQgrnXU1Liz8=
github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/graph-gophers/graphql-go v0.0.0-20191115155744-f33e81362277 h1:E0whKxgp2ojts0FDgUA8dl62bmH0LxKanMoBr6MDTDM=
github.com/graph-gophers/graphql-go v0.0.0-20191115155744-f33e81362277/go.mod h1:9CQHMSxwO4MprSdzoIEobiHpoLtHm77vfxsvsIN5Vuc=
github.com/opentracing/opentracing-go v1.1.0 h1:pWlfV3Bxv7k65HYwkikxat0+s3pV4bsqf19k25Ur8rU=
github.com/opentracing/opentracing-go v1.1.0/go.mod h1:UkNAQd3GIcIGf0SeVgPpRdFStlNbqXla1AfSYxPUl2o=
github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM=
github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4=
github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME=
github.com/stretchr/testify v1.4.0 h1:2E4SXV/wtOkTonXsotYi4li6zVWxYlZuYNCXe9XRJyk=
github.com/stretchr/testify v1.4.0/go.mod h1:j7eGeouHqKxXV5pUuKE4zz7dFj8WfuZ+81PSLYec5m4=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405 h1:yhCVgyC4o1eVCa2tZl7eS0r+SDo693bJlVdllGtEeKM=
gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0=
gopkg.in/yaml.v2 v2.2.2 h1:ZCJp+EgiOT7lHqUV2J862kp8Qj64Jo6az82+3Td9dZw=
gopkg.in/yaml.v2 v2.2.2/go.mod h1:hI93XBmqTisBFMUTm0b8Fm+jr3Dg1NNxqwp+5A1VGuI=
64 changes: 59 additions & 5 deletions graphql.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,25 +9,66 @@ import (
"net/http"

"github.com/shurcooL/graphql/internal/jsonutil"
"golang.org/x/net/context/ctxhttp"
)

func getDefaultClientHeaders() map[string]string {
return map[string]string{
"Content-Type": "application/json",
}
}

// ClientOptFunc graphql client option
type ClientOptFunc func(*Client)

// WithHeader set graphql client header
func WithHeader(key, val string) ClientOptFunc {
return func(c *Client) {
c.headers[key] = val
}
}

// WithCookie set graphql client cookie
func WithCookie(key, val string) ClientOptFunc {
return func(c *Client) {
if c.cookies == nil {
c.cookies = map[string]string{}
}
c.cookies[key] = val
}
}

// Client is a GraphQL client.
type Client struct {
url string // GraphQL server URL.
httpClient *http.Client

headers,
cookies map[string]string
}

// NewClient creates a GraphQL client targeting the specified GraphQL server URL.
// If httpClient is nil, then http.DefaultClient is used.
func NewClient(url string, httpClient *http.Client) *Client {
Copy link

Choose a reason for hiding this comment

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

Changing a function's signature is a very bad idea for backward compatibility. It's better to create a new function and have the old call the new. A blog entry from the Go team discusses this here: https://blog.golang.org/module-compatibility

Copy link
Author

Choose a reason for hiding this comment

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

just add some option args, not change the function's behavior.

Copy link

Choose a reason for hiding this comment

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

Correct, you changed the function's signature. By adding args to the function, you broke backward compatibility. See the blog post from the Go team that I linked in the original message. You should never change a function's signature in a module unless you're doing a major version release with breaking changes, and even then, try to avoid it.

Copy link
Author

Choose a reason for hiding this comment

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

you broke backward compatibility

it not broke the backward compatibility. I add option args, you can just ignore these args.

Choose a reason for hiding this comment

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

The change here is to add a variadic, old implementations will not break. How is this a breaking change?

Copy link

Choose a reason for hiding this comment

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

Laisky, again, you changed the function's signature. Please see the provided reference on how it is a breaking change. The blog post from the Go team explains how it's a breaking change when you add args to an exported function. From the provided link "Often, breaking changes come in the form of new arguments to a function." https://blog.golang.org/module-compatibility they also cover how to add arguments to an exported function without making it a breaking change. Don't change a function's signature if you don't need to..

Copy link
Author

Choose a reason for hiding this comment

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

I add an new method called NewClientWithOptions

func NewClient(url string, httpClient *http.Client) (c *Client) {
if httpClient == nil {
httpClient = http.DefaultClient
}
return &Client{
c = &Client{
headers: getDefaultClientHeaders(),
url: url,
httpClient: httpClient,
}

return c
}

// NewClientWithOptions creates a GraphQL client, same as NewClient but with some options can used to set HTTP Header
func NewClientWithOptions(url string, httpClient *http.Client, opts ...ClientOptFunc) (c *Client) {
c = NewClient(url, httpClient)
for _, optf := range opts {
optf(c)
}

return c
}

// Query executes a single GraphQL query request,
Expand Down Expand Up @@ -65,8 +106,21 @@ func (c *Client) do(ctx context.Context, op operationType, v interface{}, variab
if err != nil {
return err
}
resp, err := ctxhttp.Post(ctx, c.httpClient, c.url, "application/json", &buf)
if err != nil {
var (
req *http.Request
resp *http.Response
)
// fmt.Println(buf.String())
if req, err = http.NewRequest("POST", c.url, &buf); err != nil {
return err
}
for k, v := range c.headers {
req.Header.Set(k, v)
}
for k, v := range c.cookies {
req.AddCookie(&http.Cookie{Name: k, Value: v})
}
if resp, err = c.httpClient.Do(req.WithContext(ctx)); err != nil {
return err
}
defer resp.Body.Close()
Expand Down