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

VTN-29032, create interface to graphql client #20

Merged
merged 1 commit into from Aug 29, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
57 changes: 31 additions & 26 deletions graphql.go
Expand Up @@ -48,25 +48,26 @@ import (
"github.com/pkg/errors"
)

type Client interface {

Choose a reason for hiding this comment

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

This change caused a problem in my repo where I was using a * to graphql.Client...!!

Choose a reason for hiding this comment

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

Same issue with edge-output-writer https://github.com/veritone/edge-output-writer/blob/master/api/graphql.go#L78 where I was using

Copy link
Author

Choose a reason for hiding this comment

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

The clients of graphql need to be updated to use the new API.

Run(ctx context.Context, req *Request, resp interface{}) error
SetLogger(func(string))
}

// Client is a client for interacting with a GraphQL API.

Choose a reason for hiding this comment

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

This comment should be describing the interface on line 51

Copy link
Author

Choose a reason for hiding this comment

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

Unfortunately, I didn't catch your comment prior to the merge. I would prefer deleting something as obvious as this.

type Client struct {
type clientImp struct {
endpoint string
httpClient *http.Client
useMultipartForm bool
retryConfig RetryConfig
defaultHeaders map[string]string

// Log is called with various debug information.
// To log to standard out, use:
// client.Log = func(s string) { log.Println(s) }
Log func(s string)
log func(s string)
}

// NewClient makes a new Client capable of making GraphQL requests.
func NewClient(endpoint string, opts ...ClientOption) *Client {
c := &Client{
func NewClient(endpoint string, opts ...ClientOption) Client {
c := &clientImp{
endpoint: endpoint,
Log: func(string) {},
log: func(string) {},
}
for _, optionFunc := range opts {
optionFunc(c)
Expand All @@ -80,8 +81,12 @@ func NewClient(endpoint string, opts ...ClientOption) *Client {
return c
}

func (c *Client) logf(format string, args ...interface{}) {
c.Log(fmt.Sprintf(format, args...))
func (c *clientImp) SetLogger(cb func(string)) {
c.log = cb
}

func (c *clientImp) logf(format string, args ...interface{}) {
c.log(fmt.Sprintf(format, args...))
}

// RetryConfig defines possible fields that client can supply for their retry strategies
Expand Down Expand Up @@ -131,7 +136,7 @@ var (
)

// Wrapper method to send request while optionally applying retry policy
func (c *Client) sendRequest(retryConfig *RetryConfig, req *http.Request) (*http.Response, error) {
func (c *clientImp) sendRequest(retryConfig *RetryConfig, req *http.Request) (*http.Response, error) {
// Client did not specify retry config
if retryConfig.Policy == "" {
resp, err := c.httpClient.Do(req)
Expand Down Expand Up @@ -230,35 +235,35 @@ func (config *RetryConfig) isValid() bool {

// WithRetryConfig allows consumer to assign their retryConfig to the client's private retryConfig
func WithRetryConfig(config RetryConfig) ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.retryConfig = config
}
}

// WithDefaultLinearRetryConfig provides a default set of value for linear policy
func WithDefaultLinearRetryConfig() ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.retryConfig = defaultLinearRetryConfig
}
}

// WithDefaultExponentialRetryConfig provides a default set of value for exponential backoff policy
func WithDefaultExponentialRetryConfig() ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.retryConfig = defaultExponentialRetryConfig
}
}

// WithBeforeRetryHandler provides a handler for beforeRetry
func WithBeforeRetryHandler(beforeRetryHandler func(*http.Request, *http.Response, error, int)) ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.retryConfig.BeforeRetry = beforeRetryHandler
}
}

// WithDefaultHeaders provides a default set of header values
func WithDefaultHeaders(defaultHeaders map[string]string) ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.defaultHeaders = defaultHeaders
}
}
Expand All @@ -268,7 +273,7 @@ func WithDefaultHeaders(defaultHeaders map[string]string) ClientOption {
// Pass in a nil response object to skip response parsing.
// If the request fails or the server returns an error, the first error
// will be returned.
func (c *Client) Run(ctx context.Context, req *Request, resp interface{}) error {
func (c *clientImp) Run(ctx context.Context, req *Request, resp interface{}) error {
// TODO: validate retryConfig

select {
Expand All @@ -285,7 +290,7 @@ func (c *Client) Run(ctx context.Context, req *Request, resp interface{}) error
return c.runWithJSON(ctx, req, resp)
}

func (c *Client) getTracer() *httptrace.ClientTrace {
func (c *clientImp) getTracer() *httptrace.ClientTrace {
trace := &httptrace.ClientTrace{
GotConn: func(connInfo httptrace.GotConnInfo) {
c.logf("Connection reused? %t", connInfo.Reused)
Expand Down Expand Up @@ -323,7 +328,7 @@ func (c *Client) getTracer() *httptrace.ClientTrace {
return trace
}

func (c *Client) runWithJSON(ctx context.Context, req *Request, resp interface{}) error {
func (c *clientImp) runWithJSON(ctx context.Context, req *Request, resp interface{}) error {
var requestBody bytes.Buffer
requestBodyObj := struct {
Query string `json:"query"`
Expand Down Expand Up @@ -376,7 +381,7 @@ func getGraphQLResp(reader io.ReadCloser, schema interface{}) error {
return nil
}

func (c *Client) executeRequest(gr *graphResponse, r *http.Request) error {
func (c *clientImp) executeRequest(gr *graphResponse, r *http.Request) error {
gqlRetryConfig := c.retryConfig
var body io.Reader = r.Body
tryCount := 0
Expand Down Expand Up @@ -415,7 +420,7 @@ func (c *Client) executeRequest(gr *graphResponse, r *http.Request) error {
}

} else {
c.Log("debug return error")
c.log("debug return error")
return getAggrErr(gr.Errors)
}

Expand All @@ -428,7 +433,7 @@ func (c *Client) executeRequest(gr *graphResponse, r *http.Request) error {
return fmt.Errorf("Client has retried %d times but unable to get a successful response. Error: %s", tryCount, getAggrErr(gr.Errors))
}

func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp interface{}) error {
func (c *clientImp) runWithPostFields(ctx context.Context, req *Request, resp interface{}) error {
var requestBody bytes.Buffer
writer := multipart.NewWriter(&requestBody)
if err := writer.WriteField("query", req.q); err != nil {
Expand Down Expand Up @@ -488,22 +493,22 @@ func (c *Client) runWithPostFields(ctx context.Context, req *Request, resp inter
// making requests.
// NewClient(endpoint, WithHTTPClient(specificHTTPClient))
func WithHTTPClient(httpclient *http.Client) ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.httpClient = httpclient
}
}

// UseMultipartForm uses multipart/form-data and activates support for
// files.
func UseMultipartForm() ClientOption {
return func(client *Client) {
return func(client *clientImp) {
client.useMultipartForm = true
}
}

// ClientOption are functions that are passed into NewClient to
// modify the behaviour of the Client.
type ClientOption func(*Client)
type ClientOption func(*clientImp)

type graphResponse struct {
Data interface{}
Expand Down
32 changes: 16 additions & 16 deletions graphql_retry_test.go
Expand Up @@ -27,9 +27,9 @@ func TestLinearPolicy(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL, WithDefaultLinearRetryConfig())
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(10))
defer cancel()
Expand All @@ -50,9 +50,9 @@ func TestNilRespStatus200(t *testing.T) {
defer srv.Close()

client := NewClient(srv.URL, WithDefaultLinearRetryConfig())
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

var responseData map[string]interface{}
err := client.Run(context.Background(), &Request{q: "query {}"}, &responseData)
Expand All @@ -79,9 +79,9 @@ func TestNoPolicySpecified(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL)
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(1))
defer cancel()
Expand Down Expand Up @@ -109,9 +109,9 @@ func TestCustomRetryStatus(t *testing.T) {
RetryStatus: retryStatus,
}
client := NewClient(srv.URL, WithRetryConfig(retryConfig))
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(1))
defer cancel()
Expand All @@ -132,9 +132,9 @@ func TestExponentialBackoffPolicy(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL, WithDefaultExponentialRetryConfig(), WithBeforeRetryHandler(logHandler(t)))
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(31))
defer cancel()
Expand Down Expand Up @@ -164,9 +164,9 @@ func TestExponentialBackoffPolicyMultiPart(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL, WithDefaultExponentialRetryConfig(), WithBeforeRetryHandler(logHandler(t)), UseMultipartForm())
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(31))
defer cancel()
Expand Down Expand Up @@ -214,9 +214,9 @@ func TestErrCodeRetry(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL, WithDefaultExponentialRetryConfig(), WithBeforeRetryHandler(logHandler(t)), UseMultipartForm())
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(31))
defer cancel()
Expand Down Expand Up @@ -257,9 +257,9 @@ func TestErrCodeNoRetry(t *testing.T) {

ctx := context.Background()
client := NewClient(srv.URL, WithDefaultExponentialRetryConfig(), WithBeforeRetryHandler(logHandler(t)), UseMultipartForm())
client.Log = func(str string) {
client.SetLogger(func(str string) {
t.Log(str)
}
})

ctx, cancel := context.WithTimeout(ctx, getTestDuration(31))
defer cancel()
Expand Down
31 changes: 31 additions & 0 deletions mocks/Client.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.