diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d0d390f..bd3441c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,8 @@ - Remove some `go get` lines in `.travis.yml` file [4469](https://github.com/beego/beego/pull/4469) - Fix 4451: support QueryExecutor interface. [4461](https://github.com/beego/beego/pull/4461) - Add some testing scripts [4461](https://github.com/beego/beego/pull/4461) +- Refactor httplib: Move debug code to a filter [4440](https://github.com/beego/beego/issues/4440) ## Fix Sonar -- [4473](https://github.com/beego/beego/pull/4473) \ No newline at end of file +- [4473](https://github.com/beego/beego/pull/4473) diff --git a/adapter/httplib/httplib.go b/adapter/httplib/httplib.go index 691bf28b..005eee0f 100644 --- a/adapter/httplib/httplib.go +++ b/adapter/httplib/httplib.go @@ -115,12 +115,6 @@ func (b *BeegoHTTPRequest) SetUserAgent(useragent string) *BeegoHTTPRequest { return b } -// Debug sets show debug or not when executing request. -func (b *BeegoHTTPRequest) Debug(isdebug bool) *BeegoHTTPRequest { - b.delegate.Debug(isdebug) - return b -} - // Retries sets Retries times. // default is 0 means no retried. // -1 means retried forever. @@ -135,17 +129,6 @@ func (b *BeegoHTTPRequest) RetryDelay(delay time.Duration) *BeegoHTTPRequest { return b } -// DumpBody setting whether need to Dump the Body. -func (b *BeegoHTTPRequest) DumpBody(isdump bool) *BeegoHTTPRequest { - b.delegate.DumpBody(isdump) - return b -} - -// DumpRequest return the DumpRequest -func (b *BeegoHTTPRequest) DumpRequest() []byte { - return b.delegate.DumpRequest() -} - // SetTimeout sets connect time out and read-write time out for BeegoRequest. func (b *BeegoHTTPRequest) SetTimeout(connectTimeout, readWriteTimeout time.Duration) *BeegoHTTPRequest { b.delegate.SetTimeout(connectTimeout, readWriteTimeout) diff --git a/client/httplib/filter/log/filter.go b/client/httplib/filter/log/filter.go new file mode 100644 index 00000000..9d2e09d3 --- /dev/null +++ b/client/httplib/filter/log/filter.go @@ -0,0 +1,130 @@ +// Copyright 2020 beego +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + "context" + "io" + "net/http" + "net/http/httputil" + + "github.com/beego/beego/v2/client/httplib" + "github.com/beego/beego/v2/core/logs" +) + +// FilterChainBuilder can build a log filter +type FilterChainBuilder struct { + printableContentTypes []string // only print the body of included mime types of request and response + log func(f interface{}, v ...interface{}) // custom log function +} + +// BuilderOption option constructor +type BuilderOption func(*FilterChainBuilder) + +type logInfo struct { + req []byte + resp []byte + err error +} + +var defaultprintableContentTypes = []string{ + "text/plain", "text/xml", "text/html", "text/csv", + "text/calendar", "text/javascript", "text/javascript", + "text/css", +} + +// NewFilterChainBuilder initialize a filterChainBuilder, pass options to customize +func NewFilterChainBuilder(opts ...BuilderOption) *FilterChainBuilder { + res := &FilterChainBuilder{ + printableContentTypes: defaultprintableContentTypes, + log: logs.Debug, + } + for _, o := range opts { + o(res) + } + + return res +} + +// WithLog return option constructor modify log function +func WithLog(f func(f interface{}, v ...interface{})) BuilderOption { + return func(h *FilterChainBuilder) { + h.log = f + } +} + +// WithprintableContentTypes return option constructor modify printableContentTypes +func WithprintableContentTypes(types []string) BuilderOption { + return func(h *FilterChainBuilder) { + h.printableContentTypes = types + } +} + +// FilterChain can print the request after FilterChain processing and response before processsing +func (builder *FilterChainBuilder) FilterChain(next httplib.Filter) httplib.Filter { + return func(ctx context.Context, req *httplib.BeegoHTTPRequest) (*http.Response, error) { + info := &logInfo{} + defer info.print(builder.log) + resp, err := next(ctx, req) + info.err = err + contentType := req.GetRequest().Header.Get("Content-Type") + shouldPrintBody := builder.shouldPrintBody(contentType, req.GetRequest().Body) + dump, err := httputil.DumpRequest(req.GetRequest(), shouldPrintBody) + info.req = dump + if err != nil { + logs.Error(err) + } + if resp != nil { + contentType = resp.Header.Get("Content-Type") + shouldPrintBody = builder.shouldPrintBody(contentType, resp.Body) + dump, err = httputil.DumpResponse(resp, shouldPrintBody) + info.resp = dump + if err != nil { + logs.Error(err) + } + } + return resp, err + } +} + +func (builder *FilterChainBuilder) shouldPrintBody(contentType string, body io.ReadCloser) bool { + if contains(builder.printableContentTypes, contentType) { + return true + } + if body != nil { + logs.Warn("printableContentTypes do not contain %s, if you want to print request and response body please add it.", contentType) + } + return false +} + +func contains(s []string, e string) bool { + for _, a := range s { + if a == e { + return true + } + } + return false +} + +func (info *logInfo) print(log func(f interface{}, v ...interface{})) { + log("Request: ====================") + log("%q", info.req) + log("Response: ===================") + log("%q", info.resp) + if info.err != nil { + log("Error: ======================") + log("%q", info.err) + } +} diff --git a/client/httplib/filter/log/filter_test.go b/client/httplib/filter/log/filter_test.go new file mode 100644 index 00000000..4ee94a0d --- /dev/null +++ b/client/httplib/filter/log/filter_test.go @@ -0,0 +1,62 @@ +// Copyright 2020 beego +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package log + +import ( + "context" + "net/http" + "testing" + "time" + + "github.com/beego/beego/v2/client/httplib" + "github.com/stretchr/testify/assert" +) + +func TestFilterChain(t *testing.T) { + next := func(ctx context.Context, req *httplib.BeegoHTTPRequest) (*http.Response, error) { + time.Sleep(100 * time.Millisecond) + return &http.Response{ + StatusCode: 404, + }, nil + } + builder := NewFilterChainBuilder() + filter := builder.FilterChain(next) + req := httplib.Get("https://github.com/notifications?query=repo%3Aastaxie%2Fbeego") + resp, err := filter(context.Background(), req) + assert.NotNil(t, resp) + assert.Nil(t, err) +} + +func TestContains(t *testing.T) { + jsonType := "application/json" + cases := []struct { + Name string + Types []string + ContentType string + Expected bool + }{ + {"case1", []string{jsonType}, jsonType, true}, + {"case2", []string{"text/plain"}, jsonType, false}, + } + + for _, c := range cases { + t.Run(c.Name, func(t *testing.T) { + if ans := contains(c.Types, c.ContentType); ans != c.Expected { + t.Fatalf("Types: %v, ContentType: %v, expected %v, but %v got", + c.Types, c.ContentType, c.Expected, ans) + } + }) + } +} diff --git a/client/httplib/httplib.go b/client/httplib/httplib.go index c3c7f6f5..b50b2aca 100644 --- a/client/httplib/httplib.go +++ b/client/httplib/httplib.go @@ -43,7 +43,6 @@ import ( "mime/multipart" "net" "net/http" - "net/http/httputil" "net/url" "os" "path" @@ -155,12 +154,6 @@ func (b *BeegoHTTPRequest) SetUserAgent(useragent string) *BeegoHTTPRequest { return b } -// Debug sets show debug or not when executing request. -func (b *BeegoHTTPRequest) Debug(isdebug bool) *BeegoHTTPRequest { - b.setting.ShowDebug = isdebug - return b -} - // Retries sets Retries times. // default is 0 (never retry) // -1 retry indefinitely (forever) @@ -176,17 +169,6 @@ func (b *BeegoHTTPRequest) RetryDelay(delay time.Duration) *BeegoHTTPRequest { return b } -// DumpBody sets the DumbBody field -func (b *BeegoHTTPRequest) DumpBody(isdump bool) *BeegoHTTPRequest { - b.setting.DumpBody = isdump - return b -} - -// DumpRequest returns the DumpRequest -func (b *BeegoHTTPRequest) DumpRequest() []byte { - return b.dump -} - // SetTimeout sets connect time out and read-write time out for BeegoRequest. func (b *BeegoHTTPRequest) SetTimeout(connectTimeout, readWriteTimeout time.Duration) *BeegoHTTPRequest { b.setting.ConnectTimeout = connectTimeout @@ -446,7 +428,6 @@ func (b *BeegoHTTPRequest) DoRequest() (resp *http.Response, err error) { } func (b *BeegoHTTPRequest) DoRequestWithCtx(ctx context.Context) (resp *http.Response, err error) { - root := doRequestFilter if len(b.setting.FilterChains) > 0 { for i := len(b.setting.FilterChains) - 1; i >= 0; i-- { @@ -484,13 +465,6 @@ func (b *BeegoHTTPRequest) doRequest(ctx context.Context) (*http.Response, error client.CheckRedirect = b.setting.CheckRedirect } - if b.setting.ShowDebug { - dump, e := httputil.DumpRequest(b.req, b.setting.DumpBody) - if e != nil { - logs.Error("%+v", e) - } - b.dump = dump - } return b.sendRequest(client) } diff --git a/client/httplib/setting.go b/client/httplib/setting.go index d25fc4a9..df8eff4b 100644 --- a/client/httplib/setting.go +++ b/client/httplib/setting.go @@ -25,7 +25,6 @@ import ( // BeegoHTTPSettings is the http.Client setting type BeegoHTTPSettings struct { - ShowDebug bool UserAgent string ConnectTimeout time.Duration ReadWriteTimeout time.Duration @@ -35,7 +34,6 @@ type BeegoHTTPSettings struct { CheckRedirect func(req *http.Request, via []*http.Request) error EnableCookie bool Gzip bool - DumpBody bool Retries int // if set to -1 means will retry forever RetryDelay time.Duration FilterChains []FilterChain @@ -62,7 +60,6 @@ var defaultSetting = BeegoHTTPSettings{ ConnectTimeout: 60 * time.Second, ReadWriteTimeout: 60 * time.Second, Gzip: true, - DumpBody: true, FilterChains: make([]FilterChain, 0, 4), }