From 9d4440d4b1c22fcb0939c855214bdc5fd0af7a3a Mon Sep 17 00:00:00 2001 From: Jeevanandam M Date: Tue, 12 Mar 2019 22:48:47 -0700 Subject: [PATCH] #229 resty logger becomes interface to provides choice to an user and also provides default stub to get start --- README.md | 2 +- client.go | 54 ++++++++++++++++++++++--------------------------- client_test.go | 16 +++++++-------- middleware.go | 4 ++-- request.go | 4 ++-- request_test.go | 22 ++++++++++---------- resty_test.go | 9 ++++----- util.go | 43 +++++++++++++++++++++++++++++++++++---- 8 files changed, 90 insertions(+), 64 deletions(-) diff --git a/README.md b/README.md index d988724b..483061bb 100644 --- a/README.md +++ b/README.md @@ -37,7 +37,7 @@ * Auto detects file content type * Request URL [Path Params (aka URI Params)](https://godoc.org/github.com/go-resty/resty#Request.SetPathParams) * Backoff Retry Mechanism with retry condition function [reference](retry_test.go) - * resty client HTTP & REST [Request](https://godoc.org/github.com/go-resty/resty#Client.OnBeforeRequest) and [Response](https://godoc.org/github.com/go-resty/resty#Client.OnAfterResponse) middlewares + * Resty client HTTP & REST [Request](https://godoc.org/github.com/go-resty/resty#Client.OnBeforeRequest) and [Response](https://godoc.org/github.com/go-resty/resty#Client.OnAfterResponse) middlewares * `Request.SetContext` supported * Authorization option of `BasicAuth` and `Bearer` token * Set request `ContentLength` value for all request or particular request diff --git a/client.go b/client.go index 96b35fc1..a82a9c18 100644 --- a/client.go +++ b/client.go @@ -14,12 +14,10 @@ import ( "fmt" "io" "io/ioutil" - "log" "math" "net" "net/http" "net/url" - "os" "reflect" "regexp" "runtime" @@ -87,7 +85,6 @@ type Client struct { Debug bool DisableWarn bool AllowGetMethodPayload bool - Log *log.Logger RetryCount int RetryWaitTime time.Duration RetryMaxWaitTime time.Duration @@ -103,8 +100,8 @@ type Client struct { debugBodySizeLimit int64 outputDirectory string scheme string - logPrefix string pathParams map[string]string + log Logger httpClient *http.Client proxyURL *url.URL beforeRequest []func(*Client, *Request) error @@ -368,7 +365,7 @@ func (c *Client) OnAfterResponse(m func(*Client, *Response) error) *Client { // Note: Only one pre-request hook can be registered. Use `client.OnBeforeRequest` for mutilple. func (c *Client) SetPreRequestHook(h func(*Client, *http.Request) error) *Client { if c.preReqHook != nil { - c.Log.Printf("Overwriting an existing pre-request hook: %s", functionName(h)) + c.log.Warnf("Overwriting an existing pre-request hook: %s", functionName(h)) } c.preReqHook = h return c @@ -394,7 +391,8 @@ func (c *Client) SetDebugBodyLimit(sl int64) *Client { // called before the resty actually logs the information. func (c *Client) OnRequestLog(rl func(*RequestLog) error) *Client { if c.requestLog != nil { - c.Log.Printf("Overwriting an existing on-request-log callback from=%s to=%s", functionName(c.requestLog), functionName(rl)) + c.log.Warnf("Overwriting an existing on-request-log callback from=%s to=%s", + functionName(c.requestLog), functionName(rl)) } c.requestLog = rl return c @@ -404,7 +402,8 @@ func (c *Client) OnRequestLog(rl func(*RequestLog) error) *Client { // called before the resty actually logs the information. func (c *Client) OnResponseLog(rl func(*ResponseLog) error) *Client { if c.responseLog != nil { - c.Log.Printf("Overwriting an existing on-response-log callback from=%s to=%s", functionName(c.responseLog), functionName(rl)) + c.log.Warnf("Overwriting an existing on-response-log callback from=%s to=%s", + functionName(c.responseLog), functionName(rl)) } c.responseLog = rl return c @@ -429,12 +428,10 @@ func (c *Client) SetAllowGetMethodPayload(a bool) *Client { } // SetLogger method sets given writer for logging Resty request and response details. -// Defaults to os.Stderr. -// file, _ := os.OpenFile("/Users/jeeva/go-resty.log", os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) // -// client.SetLogger(file) -func (c *Client) SetLogger(w io.Writer) *Client { - c.Log = getLogger(w) +// Compliant to interface `resty.Logger`. +func (c *Client) SetLogger(l Logger) *Client { + c.log = l return c } @@ -476,7 +473,7 @@ func (c *Client) SetError(err interface{}) *Client { func (c *Client) SetRedirectPolicy(policies ...interface{}) *Client { for _, p := range policies { if _, ok := p.(RedirectPolicy); !ok { - c.Log.Printf("ERORR: %v does not implement resty.RedirectPolicy (missing Apply method)", + c.log.Errorf("%v does not implement resty.RedirectPolicy (missing Apply method)", functionName(p)) } } @@ -539,7 +536,7 @@ func (c *Client) AddRetryCondition(condition RetryConditionFunc) *Client { func (c *Client) SetTLSClientConfig(config *tls.Config) *Client { transport, err := c.transport() if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } transport.TLSClientConfig = config @@ -555,13 +552,13 @@ func (c *Client) SetTLSClientConfig(config *tls.Config) *Client { func (c *Client) SetProxy(proxyURL string) *Client { transport, err := c.transport() if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } pURL, err := url.Parse(proxyURL) if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } @@ -575,7 +572,7 @@ func (c *Client) SetProxy(proxyURL string) *Client { func (c *Client) RemoveProxy() *Client { transport, err := c.transport() if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } c.proxyURL = nil @@ -587,7 +584,7 @@ func (c *Client) RemoveProxy() *Client { func (c *Client) SetCertificates(certs ...tls.Certificate) *Client { config, err := c.tlsConfig() if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } config.Certificates = append(config.Certificates, certs...) @@ -599,13 +596,13 @@ func (c *Client) SetCertificates(certs ...tls.Certificate) *Client { func (c *Client) SetRootCertificate(pemFilePath string) *Client { rootPemData, err := ioutil.ReadFile(pemFilePath) if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } config, err := c.tlsConfig() if err != nil { - c.Log.Printf("ERROR %v", err) + c.log.Errorf("%v", err) return c } if config.RootCAs == nil { @@ -677,13 +674,6 @@ func (c *Client) SetDoNotParseResponse(parse bool) *Client { return c } -// SetLogPrefix method sets the Resty logger prefix value. -func (c *Client) SetLogPrefix(prefix string) *Client { - c.logPrefix = prefix - c.Log.SetPrefix(prefix) - return c -} - // SetPathParams method sets multiple URL path key-value pairs at one go in the // Resty client instance. // client.SetPathParams(map[string]string{ @@ -859,6 +849,11 @@ func (c *Client) transport() (*http.Transport, error) { return nil, errors.New("current transport is not an *http.Transport instance") } +// just an helper method +func (c *Client) outputLogTo(w io.Writer) { + c.log.(*logger).l.SetOutput(w) +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // File struct and its methods //_______________________________________________________________________ @@ -901,7 +896,6 @@ func createClient(hc *http.Client) *Client { FormData: url.Values{}, Header: http.Header{}, Cookies: make([]*http.Cookie, 0), - Log: getLogger(os.Stderr), RetryWaitTime: defaultWaitTime, RetryMaxWaitTime: defaultMaxWaitTime, JSONMarshal: json.Marshal, @@ -912,8 +906,8 @@ func createClient(hc *http.Client) *Client { pathParams: make(map[string]string), } - // Log Prefix - c.SetLogPrefix("RESTY ") + // Logger + c.SetLogger(createLogger()) // default before request middlewares c.beforeRequest = []func(*Client, *Request) error{ diff --git a/client_test.go b/client_test.go index 71a32b5d..34b174f4 100644 --- a/client_test.go +++ b/client_test.go @@ -315,11 +315,11 @@ func TestClientOptions(t *testing.T) { assertEqual(t, true, transport.TLSClientConfig.InsecureSkipVerify) client.OnBeforeRequest(func(c *Client, r *Request) error { - c.Log.Println("I'm in Request middleware") + c.log.Debugf("I'm in Request middleware") return nil // if it success }) client.OnAfterResponse(func(c *Client, r *Response) error { - c.Log.Println("I'm in Response middleware") + c.log.Debugf("I'm in Response middleware") return nil // if it success }) @@ -344,19 +344,17 @@ func TestClientOptions(t *testing.T) { client.SetCloseConnection(true) assertEqual(t, client.closeConnection, true) - - client.SetLogger(ioutil.Discard) } func TestClientPreRequestHook(t *testing.T) { client := dc() client.SetPreRequestHook(func(c *Client, r *http.Request) error { - c.Log.Println("I'm in Pre-Request Hook") + c.log.Debugf("I'm in Pre-Request Hook") return nil }) client.SetPreRequestHook(func(c *Client, r *http.Request) error { - c.Log.Println("I'm Overwriting existing Pre-Request Hook") + c.log.Debugf("I'm Overwriting existing Pre-Request Hook") return nil }) } @@ -379,7 +377,7 @@ func TestClientAllowsGetMethodPayload(t *testing.T) { func TestClientRoundTripper(t *testing.T) { c := NewWithClient(&http.Client{}) - c.SetLogger(ioutil.Discard) + c.outputLogTo(ioutil.Discard) rt := &CustomRoundTripper{} c.SetTransport(rt) @@ -409,8 +407,8 @@ func TestDebugBodySizeLimit(t *testing.T) { var lgr bytes.Buffer c := dc() c.SetDebug(true) - c.SetLogger(&lgr) c.SetDebugBodyLimit(30) + c.outputLogTo(&lgr) testcases := []struct{ url, want string }{ // Text, does not exceed limit. @@ -479,7 +477,7 @@ func TestLogCallbacks(t *testing.T) { c := New().SetDebug(true) var lgr bytes.Buffer - c.SetLogger(&lgr) + c.outputLogTo(&lgr) c.OnRequestLog(func(r *RequestLog) error { // masking authorzation header diff --git a/middleware.go b/middleware.go index 4cd0689b..0a2f289b 100644 --- a/middleware.go +++ b/middleware.go @@ -216,7 +216,7 @@ func addCredentials(c *Client, r *Request) error { if !c.DisableWarn { if isBasicAuth && !strings.HasPrefix(r.URL, "https") { - c.Log.Println("WARNING - Using Basic Auth in HTTP mode is not secure.") + c.log.Warnf("Using Basic Auth in HTTP mode is not secure, use HTTPS") } } @@ -281,7 +281,7 @@ func responseLogger(c *Client, res *Response) error { } debugLog += "==============================================================================\n" - c.Log.Print(debugLog) + c.log.Debugf(debugLog) } return nil diff --git a/request.go b/request.go index 889a2364..cb5de8ca 100644 --- a/request.go +++ b/request.go @@ -172,7 +172,7 @@ func (r *Request) SetQueryString(query string) *Request { } } } else { - r.client.Log.Printf("ERROR %v", err) + r.client.log.Errorf("%v", err) } return r } @@ -574,7 +574,7 @@ func (r *Request) Execute(method, url string) (*Response, error) { resp, err = r.client.execute(r) if err != nil { - r.client.Log.Printf("ERROR %v, Attempt %v", err, attempt) + r.client.log.Errorf("%v, Attempt %v", err, attempt) if r.ctx != nil && r.ctx.Err() != nil { // stop Backoff from retrying request if request has been // canceled by context diff --git a/request_test.go b/request_test.go index 8d4085cc..3fdc23d9 100644 --- a/request_test.go +++ b/request_test.go @@ -69,8 +69,8 @@ func TestGetClientParamRequestParam(t *testing.T) { c := dc() c.SetQueryParam("client_param", "true"). SetQueryParams(map[string]string{"req_1": "jeeva", "req_3": "jeeva3"}). - SetDebug(true). - SetLogger(ioutil.Discard) + SetDebug(true) + c.outputLogTo(ioutil.Discard) resp, err := c.R(). SetQueryParams(map[string]string{"req_1": "req 1 value", "req_2": "req 2 value"}). @@ -498,8 +498,8 @@ func TestFormData(t *testing.T) { c := dc() c.SetFormData(map[string]string{"zip_code": "00000", "city": "Los Angeles"}). SetContentLength(true). - SetDebug(true). - SetLogger(ioutil.Discard) + SetDebug(true) + c.outputLogTo(ioutil.Discard) resp, err := c.R(). SetFormData(map[string]string{"first_name": "Jeevanandam", "last_name": "M", "zip_code": "00001"}). @@ -520,9 +520,8 @@ func TestMultiValueFormData(t *testing.T) { } c := dc() - c.SetContentLength(true). - SetDebug(true). - SetLogger(ioutil.Discard) + c.SetContentLength(true).SetDebug(true) + c.outputLogTo(ioutil.Discard) resp, err := c.R(). SetQueryParamsFromValues(v). @@ -541,8 +540,8 @@ func TestFormDataDisableWarn(t *testing.T) { c.SetFormData(map[string]string{"zip_code": "00000", "city": "Los Angeles"}). SetContentLength(true). SetDebug(true). - SetLogger(ioutil.Discard). SetDisableWarn(true) + c.outputLogTo(ioutil.Discard) resp, err := c.R(). SetFormData(map[string]string{"first_name": "Jeevanandam", "last_name": "M", "zip_code": "00001"}). @@ -820,7 +819,8 @@ func TestPutJSONString(t *testing.T) { return nil }) - client.SetDebug(true).SetLogger(ioutil.Discard) + client.SetDebug(true) + client.outputLogTo(ioutil.Discard) resp, err := client.R(). SetHeaders(map[string]string{hdrContentTypeKey: jsonContentType, hdrAcceptKey: jsonContentType}). @@ -1159,8 +1159,8 @@ func TestOutputFileWithBaseDirAndRelativePath(t *testing.T) { client := dc(). SetRedirectPolicy(FlexibleRedirectPolicy(10)). SetOutputDirectory(filepath.Join(getTestDataPath(), "dir-sample")). - SetDebug(true). - SetLogger(ioutil.Discard) + SetDebug(true) + client.outputLogTo(ioutil.Discard) resp, err := client.R(). SetOutput("go-resty/test-img-success.png"). diff --git a/resty_test.go b/resty_test.go index 771e9663..ab534ca0 100644 --- a/resty_test.go +++ b/resty_test.go @@ -540,9 +540,9 @@ func createTestServer(fn func(w http.ResponseWriter, r *http.Request)) *httptest } func dc() *Client { - client := New() - client.SetLogger(ioutil.Discard) - return client + c := New() + c.outputLogTo(ioutil.Discard) + return c } func dcr() *Request { @@ -552,8 +552,7 @@ func dcr() *Request { func dclr() *Request { c := dc() c.SetDebug(true) - c.SetLogger(ioutil.Discard) - + c.outputLogTo(ioutil.Discard) return c.R() } diff --git a/util.go b/util.go index baa9ce6c..ed9129ff 100644 --- a/util.go +++ b/util.go @@ -21,6 +21,45 @@ import ( "strings" ) +//‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ +// Logger interface +//_______________________________________________________________________ + +// Logger interface is to abstract the logging from Resty. Gives control to +// the Resty users, choice of the logger. +type Logger interface { + Errorf(format string, v ...interface{}) + Warnf(format string, v ...interface{}) + Debugf(format string, v ...interface{}) +} + +func createLogger() *logger { + l := &logger{l: log.New(os.Stderr, "", log.Ldate|log.Lmicroseconds)} + return l +} + +var _ Logger = (*logger)(nil) + +type logger struct { + l *log.Logger +} + +func (l *logger) Errorf(format string, v ...interface{}) { + l.output("ERROR RESTY "+format, v...) +} + +func (l *logger) Warnf(format string, v ...interface{}) { + l.output("WARN RESTY "+format, v...) +} + +func (l *logger) Debugf(format string, v ...interface{}) { + l.output("DEBUG RESTY "+format, v...) +} + +func (l *logger) output(format string, v ...interface{}) { + l.l.Printf(format, v...) +} + //‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾‾ // Package Helper methods //_______________________________________________________________________ @@ -114,10 +153,6 @@ func firstNonEmpty(v ...string) string { return "" } -func getLogger(w io.Writer) *log.Logger { - return log.New(w, "RESTY ", log.LstdFlags) -} - var quoteEscaper = strings.NewReplacer("\\", "\\\\", `"`, "\\\"") func escapeQuotes(s string) string {