From f01ed0dc1c102d2f4ed956490326fd56ce7d521c Mon Sep 17 00:00:00 2001 From: andig Date: Thu, 11 Feb 2021 16:52:17 +0100 Subject: [PATCH] Cleanup request api and simplify creating a logging transport (#657) --- charger/evsewifi.go | 14 ++-- charger/mcc.go | 7 +- meter/tesla.go | 3 +- provider/http.go | 3 +- provider/socket.go | 3 +- util/request/helper.go | 81 ++------------------ {vehicle/tesla => util/request}/roundtrip.go | 12 ++- util/request/transport.go | 27 +++---- vehicle/tesla/client.go | 9 ++- 9 files changed, 45 insertions(+), 114 deletions(-) rename {vehicle/tesla => util/request}/roundtrip.go (74%) diff --git a/charger/evsewifi.go b/charger/evsewifi.go index 6a8779904..734312220 100644 --- a/charger/evsewifi.go +++ b/charger/evsewifi.go @@ -126,22 +126,18 @@ func (evse *EVSEWifi) apiURL(service string) string { // query evse parameters func (evse *EVSEWifi) getParameters() (EVSEListEntry, error) { - var pr EVSEParameterResponse + var res EVSEParameterResponse url := evse.apiURL(evseGetParameters) - err := evse.GetJSON(url, &pr) + err := evse.GetJSON(url, &res) if err != nil { return EVSEListEntry{}, err } - if len(pr.List) != 1 { - var body []byte - if resp := evse.LastResponse(); resp != nil { - body, _ = request.ReadBody(resp) - } - return EVSEListEntry{}, fmt.Errorf("unexpected response: %s", string(body)) + if len(res.List) != 1 { + return EVSEListEntry{}, fmt.Errorf("unexpected response: %s", res.Type) } - params := pr.List[0] + params := res.List[0] if !params.AlwaysActive { evse.log.WARN.Println("evse should be configured to remote mode") } diff --git a/charger/mcc.go b/charger/mcc.go index b69be8e30..dab863fb6 100644 --- a/charger/mcc.go +++ b/charger/mcc.go @@ -1,7 +1,6 @@ package charger import ( - "crypto/tls" "encoding/json" "fmt" "net/http" @@ -80,14 +79,16 @@ func NewMobileConnectFromConfig(other map[string]interface{}) (api.Charger, erro // NewMobileConnect creates MCC charger func NewMobileConnect(uri string, password string) (*MobileConnect, error) { + log := util.NewLogger("mcc") + mcc := &MobileConnect{ - Helper: request.NewHelper(util.NewLogger("mcc")), + Helper: request.NewHelper(log), uri: strings.TrimRight(uri, "/"), password: password, } // ignore the self signed certificate - mcc.Helper.Transport(request.NewTransport().WithTLSConfig(&tls.Config{InsecureSkipVerify: true})) + mcc.Client.Transport = request.NewTripper(log, request.InsecureTransport()) return mcc, nil } diff --git a/meter/tesla.go b/meter/tesla.go index 5b8d3476d..18156875a 100644 --- a/meter/tesla.go +++ b/meter/tesla.go @@ -1,7 +1,6 @@ package meter import ( - "crypto/tls" "errors" "fmt" "strings" @@ -85,7 +84,7 @@ func NewTesla(uri, usage string) (api.Meter, error) { } // ignore the self signed certificate - m.Helper.Transport(request.NewTransport().WithTLSConfig(&tls.Config{InsecureSkipVerify: true})) + m.Client.Transport = request.NewTripper(log, request.InsecureTransport()) // decorate api.MeterEnergy var totalEnergy func() (float64, error) diff --git a/provider/http.go b/provider/http.go index fb2e5727e..f9fae6ab3 100644 --- a/provider/http.go +++ b/provider/http.go @@ -1,7 +1,6 @@ package provider import ( - "crypto/tls" "encoding/base64" "fmt" "io" @@ -101,7 +100,7 @@ func NewHTTP(log *util.Logger, method, uri string, headers map[string]string, bo // ignore the self signed certificate if insecure { - p.Helper.Transport(request.NewTransport().WithTLSConfig(&tls.Config{InsecureSkipVerify: true})) + p.Client.Transport = request.NewTripper(log, request.InsecureTransport()) } if jq != "" { diff --git a/provider/socket.go b/provider/socket.go index 175a0d833..279619905 100644 --- a/provider/socket.go +++ b/provider/socket.go @@ -1,7 +1,6 @@ package provider import ( - "crypto/tls" "fmt" "math" "net/http" @@ -77,7 +76,7 @@ func NewSocketProviderFromConfig(other map[string]interface{}) (IntProvider, err // ignore the self signed certificate if cc.Insecure { - p.Helper.Transport(request.NewTransport().WithTLSConfig(&tls.Config{InsecureSkipVerify: true})) + p.Client.Transport = request.NewTripper(log, request.InsecureTransport()) } if cc.Jq != "" { diff --git a/util/request/helper.go b/util/request/helper.go index ad64206c6..23ad42fbd 100644 --- a/util/request/helper.go +++ b/util/request/helper.go @@ -1,95 +1,30 @@ package request import ( - "fmt" - "log" "net/http" - "net/http/httputil" - "strings" "time" "github.com/andig/evcc/util" ) +// Timeout is the default request timeout used by the Helper +var Timeout = 10 * time.Second + // Helper provides utility primitives type Helper struct { *http.Client - log *log.Logger - last *http.Response // last response } // NewHelper creates http helper for simplified PUT GET logic func NewHelper(log *util.Logger) *Helper { r := &Helper{ - Client: &http.Client{Timeout: 10 * time.Second}, - log: log.TRACE, - } - - // intercept for logging - r.Transport(http.DefaultTransport) - - return r -} - -// LastResponse returns last http.Response that was read without error -func (r *Helper) LastResponse() *http.Response { - return r.last -} - -type helperTransport struct { - log *log.Logger - detailed bool - lastResponse func(*http.Response) - roundTripper http.RoundTripper -} - -func (r *helperTransport) RoundTrip(req *http.Request) (*http.Response, error) { - msg := fmt.Sprintf("%s %s", req.Method, req.URL) - if r.detailed { - if body, err := httputil.DumpRequest(req, true); err == nil { - msg += "\n" + strings.TrimSpace(string(body)) + "\n" - } - } - - resp, err := r.roundTripper.RoundTrip(req) - r.lastResponse(resp) - - if r.log != nil { - if resp != nil { - if r.detailed { - if body, err := httputil.DumpResponse(resp, true); err == nil { - msg += "\n" + strings.TrimSpace(string(body)) - } - } else { - msg += "\n" + resp.Status - - if body, _ := ReadBody(resp); len(body) > 0 { - const max = 2048 - - str := string(body) - if len(str) >= max { - str = str[:max] - } - - msg += "\n" + strings.TrimSpace(str) - } - } - } - r.log.Println(msg) - } - - return resp, err -} - -// Transport wraps the provided transport with logging and sets it as client transport -func (r *Helper) Transport(roundTripper http.RoundTripper) { - r.Client.Transport = &helperTransport{ - log: r.log, - roundTripper: roundTripper, - lastResponse: func(resp *http.Response) { - r.last = resp + Client: &http.Client{ + Timeout: Timeout, + Transport: NewTripper(log, http.DefaultTransport), }, } + + return r } // DoBody executes HTTP request and returns the response body diff --git a/vehicle/tesla/roundtrip.go b/util/request/roundtrip.go similarity index 74% rename from vehicle/tesla/roundtrip.go rename to util/request/roundtrip.go index 19eec5829..6fb16b817 100644 --- a/vehicle/tesla/roundtrip.go +++ b/util/request/roundtrip.go @@ -1,4 +1,4 @@ -package tesla +package request import ( "net/http" @@ -15,6 +15,16 @@ type roundTripper struct { const max = 2048 +// NewTripper creates a logging roundtrip handler +func NewTripper(log *util.Logger, transport http.RoundTripper) http.RoundTripper { + tripper := &roundTripper{ + log: log, + transport: transport, + } + + return tripper +} + func (r *roundTripper) RoundTrip(req *http.Request) (*http.Response, error) { if body, err := httputil.DumpRequest(req, true); err == nil { s := strings.TrimSpace(string(body)) diff --git a/util/request/transport.go b/util/request/transport.go index 4f0dbc575..17cc5ba8f 100644 --- a/util/request/transport.go +++ b/util/request/transport.go @@ -5,27 +5,18 @@ import ( "net/http" ) -// Transport decorates http.Transport with fluent style -type Transport struct { - *http.Transport -} - -// NewDefaultTransport creates a clone of the http.DefaultTransport -func NewDefaultTransport() *http.Transport { - return http.DefaultTransport.(*http.Transport).Clone() -} - -// NewTransport creates an HTTP transport -func NewTransport() *Transport { - t := &Transport{ - Transport: NewDefaultTransport(), +// DefaultTransport returns http.DefaultTransport as http.Transport instead of http.RoundTripper +func DefaultTransport() *http.Transport { + t, ok := http.DefaultTransport.(*http.Transport) + if !ok { + panic("http.DefaultTransport is not an http.Transport") } - return t } -// WithTLSConfig sets the transports TLS configuration -func (t *Transport) WithTLSConfig(tls *tls.Config) *Transport { - t.Transport.TLSClientConfig = tls +// InsecureTransport is an http.Transport with TLSClientConfig.InsecureSkipVerify enabled +func InsecureTransport() *http.Transport { + t := DefaultTransport() + t.TLSClientConfig = &tls.Config{InsecureSkipVerify: true} return t } diff --git a/vehicle/tesla/client.go b/vehicle/tesla/client.go index eb1810732..b8e862f8f 100644 --- a/vehicle/tesla/client.go +++ b/vehicle/tesla/client.go @@ -11,6 +11,7 @@ import ( "net/http" "github.com/andig/evcc/util" + "github.com/andig/evcc/util/request" "github.com/uhthomas/tesla" "golang.org/x/oauth2" ) @@ -45,10 +46,10 @@ func pkce() (verifier, challenge string, err error) { // NewClient creates a tesla authentication client func NewClient(log *util.Logger) (*Client, error) { - httpClient := &http.Client{Transport: &roundTripper{ - log: log, - transport: http.DefaultTransport, - }} + httpClient := &http.Client{ + Timeout: request.Timeout, + Transport: request.NewTripper(log, http.DefaultTransport), + } config := &oauth2.Config{ ClientID: "ownerapi",