From 4d11e5e9427222480dbd47f53c01d47c4d5196dc Mon Sep 17 00:00:00 2001 From: iurii Date: Sun, 29 Sep 2019 00:25:13 +0300 Subject: [PATCH] (#796) User should be responsible for request encoding with client endpoint --- transport/http/client.go | 177 ++++++++++++------------- transport/http/client_test.go | 46 +++---- transport/http/deprecated_client.go | 192 ++++++++++++++++++++++++++++ transport/http/encode_decode.go | 7 + 4 files changed, 312 insertions(+), 110 deletions(-) create mode 100644 transport/http/deprecated_client.go diff --git a/transport/http/client.go b/transport/http/client.go index 4cd8f27a8..32e2640ca 100644 --- a/transport/http/client.go +++ b/transport/http/client.go @@ -13,85 +13,23 @@ import ( "github.com/go-kit/kit/endpoint" ) -// HTTPClient is an interface that models *http.Client. -type HTTPClient interface { - Do(req *http.Request) (*http.Response, error) -} - -// Client wraps a URL and provides a method that implements endpoint.Endpoint. -type Client struct { - client HTTPClient - method string - tgt *url.URL - enc EncodeRequestFunc - dec DecodeResponseFunc - before []RequestFunc - after []ClientResponseFunc - finalizer []ClientFinalizerFunc - bufferedStream bool -} - -// NewClient constructs a usable Client for a single remote method. -func NewClient( +// NewClientEndpoint returns a usable endpoint that invokes the remote endpoint. +func NewClientEndpoint( method string, tgt *url.URL, - enc EncodeRequestFunc, + enc EncodeClientRequestFunc, dec DecodeResponseFunc, - options ...ClientOption, -) *Client { - c := &Client{ + options ...ClientEndpointOption, +) endpoint.Endpoint { + opts := &clientEndpointOpts{ client: http.DefaultClient, - method: method, - tgt: tgt, - enc: enc, - dec: dec, before: []RequestFunc{}, after: []ClientResponseFunc{}, bufferedStream: false, } for _, option := range options { - option(c) + option(opts) } - return c -} - -// ClientOption sets an optional parameter for clients. -type ClientOption func(*Client) - -// SetClient sets the underlying HTTP client used for requests. -// By default, http.DefaultClient is used. -func SetClient(client HTTPClient) ClientOption { - return func(c *Client) { c.client = client } -} - -// ClientBefore sets the RequestFuncs that are applied to the outgoing HTTP -// request before it's invoked. -func ClientBefore(before ...RequestFunc) ClientOption { - return func(c *Client) { c.before = append(c.before, before...) } -} - -// ClientAfter sets the ClientResponseFuncs applied to the incoming HTTP -// request prior to it being decoded. This is useful for obtaining anything off -// of the response and adding onto the context prior to decoding. -func ClientAfter(after ...ClientResponseFunc) ClientOption { - return func(c *Client) { c.after = append(c.after, after...) } -} - -// ClientFinalizer is executed at the end of every HTTP request. -// By default, no finalizer is registered. -func ClientFinalizer(f ...ClientFinalizerFunc) ClientOption { - return func(s *Client) { s.finalizer = append(s.finalizer, f...) } -} - -// BufferedStream sets whether the Response.Body is left open, allowing it -// to be read from later. Useful for transporting a file as a buffered stream. -// That body has to be Closed to propery end the request. -func BufferedStream(buffered bool) ClientOption { - return func(c *Client) { c.bufferedStream = buffered } -} - -// Endpoint returns a usable endpoint that invokes the remote endpoint. -func (c Client) Endpoint() endpoint.Endpoint { return func(ctx context.Context, request interface{}) (interface{}, error) { ctx, cancel := context.WithCancel(ctx) @@ -99,34 +37,37 @@ func (c Client) Endpoint() endpoint.Endpoint { resp *http.Response err error ) - if c.finalizer != nil { + if opts.finalizer != nil { defer func() { if resp != nil { ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header) ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength) } - for _, f := range c.finalizer { + for _, f := range opts.finalizer { f(ctx, err) } }() } - req, err := http.NewRequest(c.method, c.tgt.String(), nil) + req, err := enc(ctx, method, tgt.String(), request) if err != nil { cancel() return nil, err } - - if err = c.enc(ctx, req, request); err != nil { - cancel() - return nil, err + // If user didn't bother to create a req object we have to set a default value for it ourselves. + if req == nil { + req, err = http.NewRequest(method, tgt.String(), nil) + if err != nil { + cancel() + return nil, err + } } - for _, f := range c.before { + for _, f := range opts.before { ctx = f(ctx, req) } - resp, err = c.client.Do(req.WithContext(ctx)) + resp, err = opts.client.Do(req.WithContext(ctx)) if err != nil { cancel() @@ -135,18 +76,18 @@ func (c Client) Endpoint() endpoint.Endpoint { // If we expect a buffered stream, we don't cancel the context when the endpoint returns. // Instead, we should call the cancel func when closing the response body. - if c.bufferedStream { + if opts.bufferedStream { resp.Body = bodyWithCancel{ReadCloser: resp.Body, cancel: cancel} } else { defer resp.Body.Close() defer cancel() } - for _, f := range c.after { + for _, f := range opts.after { ctx = f(ctx, resp) } - response, err := c.dec(ctx, resp) + response, err := dec(ctx, resp) if err != nil { return nil, err } @@ -155,6 +96,54 @@ func (c Client) Endpoint() endpoint.Endpoint { } } +// ClientEndpointOption sets an optional parameter for client endpoint. +type ClientEndpointOption func(*clientEndpointOpts) + +type clientEndpointOpts struct { + client HTTPClient + before []RequestFunc + after []ClientResponseFunc + finalizer []ClientFinalizerFunc + bufferedStream bool +} + +// ClientEndpointSetClient sets the underlying HTTP client used for requests. +// By default, http.DefaultClient is used. +func ClientEndpointSetClient(client HTTPClient) ClientEndpointOption { + return func(e *clientEndpointOpts) { e.client = client } +} + +// ClientEndpointBefore sets the RequestFuncs that are applied to the outgoing HTTP +// request before it's invoked. +func ClientEndpointBefore(before ...RequestFunc) ClientEndpointOption { + return func(e *clientEndpointOpts) { e.before = append(e.before, before...) } +} + +// ClientEndpointAfter sets the ClientResponseFuncs applied to the incoming HTTP +// request prior to it being decoded. This is useful for obtaining anything off +// of the response and adding onto the context prior to decoding. +func ClientEndpointAfter(after ...ClientResponseFunc) ClientEndpointOption { + return func(e *clientEndpointOpts) { e.after = append(e.after, after...) } +} + +// ClientEndpointFinalizer is executed at the end of every HTTP request. +// By default, no finalizer is registered. +func ClientEndpointFinalizer(f ...ClientFinalizerFunc) ClientEndpointOption { + return func(e *clientEndpointOpts) { e.finalizer = append(e.finalizer, f...) } +} + +// ClientEndpointBufferedStream sets whether the Response.Body is left open, allowing it +// to be read from later. Useful for transporting a file as a buffered stream. +// That body has to be Closed to propery end the request. +func ClientEndpointBufferedStream(buffered bool) ClientEndpointOption { + return func(e *clientEndpointOpts) { e.bufferedStream = buffered } +} + +// HTTPClient is an interface that models *http.Client. +type HTTPClient interface { + Do(req *http.Request) (*http.Response, error) +} + // bodyWithCancel is a wrapper for an io.ReadCloser with also a // cancel function which is called when the Close is used type bodyWithCancel struct { @@ -177,11 +166,15 @@ func (bwc bodyWithCancel) Close() error { // depending on when an error occurs. type ClientFinalizerFunc func(ctx context.Context, err error) -// EncodeJSONRequest is an EncodeRequestFunc that serializes the request as a +// EncodeJSONClientRequest is an EncodeRequestFunc that serializes the request as a // JSON object to the Request body. Many JSON-over-HTTP services can use it as // a sensible default. If the request implements Headerer, the provided headers // will be applied to the request. -func EncodeJSONRequest(c context.Context, r *http.Request, request interface{}) error { +func EncodeJSONClientRequest(c context.Context, method, url string, request interface{}) (*http.Request, error) { + r, err := http.NewRequest(method, url, nil) + if err != nil { + return nil, err + } r.Header.Set("Content-Type", "application/json; charset=utf-8") if headerer, ok := request.(Headerer); ok { for k := range headerer.Headers() { @@ -190,13 +183,20 @@ func EncodeJSONRequest(c context.Context, r *http.Request, request interface{}) } var b bytes.Buffer r.Body = ioutil.NopCloser(&b) - return json.NewEncoder(&b).Encode(request) + if err := json.NewEncoder(&b).Encode(request); err != nil { + return nil, err + } + return r, nil } -// EncodeXMLRequest is an EncodeRequestFunc that serializes the request as a +// EncodeXMLClientRequest is an EncodeRequestFunc that serializes the request as a // XML object to the Request body. If the request implements Headerer, // the provided headers will be applied to the request. -func EncodeXMLRequest(c context.Context, r *http.Request, request interface{}) error { +func EncodeXMLClientRequest(c context.Context, method, url string, request interface{}) (*http.Request, error) { + r, err := http.NewRequest(method, url, nil) + if err != nil { + return nil, err + } r.Header.Set("Content-Type", "text/xml; charset=utf-8") if headerer, ok := request.(Headerer); ok { for k := range headerer.Headers() { @@ -205,5 +205,8 @@ func EncodeXMLRequest(c context.Context, r *http.Request, request interface{}) e } var b bytes.Buffer r.Body = ioutil.NopCloser(&b) - return xml.NewEncoder(&b).Encode(request) + if err := xml.NewEncoder(&b).Encode(request); err != nil { + return nil, err + } + return r, nil } diff --git a/transport/http/client_test.go b/transport/http/client_test.go index 9ec1a6caf..32b3ee579 100644 --- a/transport/http/client_test.go +++ b/transport/http/client_test.go @@ -22,7 +22,7 @@ type TestResponse struct { func TestHTTPClient(t *testing.T) { var ( testbody = "testbody" - encode = func(context.Context, *http.Request, interface{}) error { return nil } + encode = func(context.Context, string, string, interface{}) (*http.Request, error) { return nil, nil } decode = func(_ context.Context, r *http.Response) (interface{}, error) { buffer := make([]byte, len(testbody)) r.Body.Read(buffer) @@ -47,16 +47,16 @@ func TestHTTPClient(t *testing.T) { w.Write([]byte(testbody)) })) - client := httptransport.NewClient( + endpoint := httptransport.NewClientEndpoint( "GET", mustParse(server.URL), encode, decode, - httptransport.ClientBefore(httptransport.SetRequestHeader(headerKey, headerVal)), - httptransport.ClientAfter(afterFunc), + httptransport.ClientEndpointBefore(httptransport.SetRequestHeader(headerKey, headerVal)), + httptransport.ClientEndpointAfter(afterFunc), ) - res, err := client.Endpoint()(context.Background(), struct{}{}) + res, err := endpoint(context.Background(), struct{}{}) if err != nil { t.Fatal(err) } @@ -104,7 +104,7 @@ func TestHTTPClientBufferedStream(t *testing.T) { const bodysize = 6000 var ( testbody = string(make([]byte, bodysize)) - encode = func(context.Context, *http.Request, interface{}) error { return nil } + encode = func(context.Context, string, string, interface{}) (*http.Request, error) { return nil, nil } decode = func(_ context.Context, r *http.Response) (interface{}, error) { return TestResponse{r.Body, ""}, nil } @@ -115,15 +115,15 @@ func TestHTTPClientBufferedStream(t *testing.T) { w.Write([]byte(testbody)) })) - client := httptransport.NewClient( + endpoint := httptransport.NewClientEndpoint( "GET", mustParse(server.URL), encode, decode, - httptransport.BufferedStream(true), + httptransport.ClientEndpointBufferedStream(true), ) - res, err := client.Endpoint()(context.Background(), struct{}{}) + res, err := endpoint(context.Background(), struct{}{}) if err != nil { t.Fatal(err) } @@ -154,7 +154,7 @@ func TestClientFinalizer(t *testing.T) { headerVal = "Helllo you stinky lizard" responseBody = "go eat a fly ugly\n" done = make(chan struct{}) - encode = func(context.Context, *http.Request, interface{}) error { return nil } + encode = func(context.Context, string, string, interface{}) (*http.Request, error) { return nil, nil } decode = func(_ context.Context, r *http.Response) (interface{}, error) { return TestResponse{r.Body, ""}, nil } @@ -166,12 +166,12 @@ func TestClientFinalizer(t *testing.T) { })) defer server.Close() - client := httptransport.NewClient( + endpoint := httptransport.NewClientEndpoint( "GET", mustParse(server.URL), encode, decode, - httptransport.ClientFinalizer(func(ctx context.Context, err error) { + httptransport.ClientEndpointFinalizer(func(ctx context.Context, err error) { responseHeader := ctx.Value(httptransport.ContextKeyResponseHeaders).(http.Header) if want, have := headerVal, responseHeader.Get(headerKey); want != have { t.Errorf("%s: want %q, have %q", headerKey, want, have) @@ -186,7 +186,7 @@ func TestClientFinalizer(t *testing.T) { }), ) - _, err := client.Endpoint()(context.Background(), struct{}{}) + _, err := endpoint(context.Background(), struct{}{}) if err != nil { t.Fatal(err) } @@ -219,12 +219,12 @@ func TestEncodeJSONRequest(t *testing.T) { t.Fatal(err) } - client := httptransport.NewClient( + endpoint := httptransport.NewClientEndpoint( "POST", serverURL, - httptransport.EncodeJSONRequest, + httptransport.EncodeJSONClientRequest, func(context.Context, *http.Response) (interface{}, error) { return nil, nil }, - ).Endpoint() + ) for _, test := range []struct { value interface{} @@ -237,7 +237,7 @@ func TestEncodeJSONRequest(t *testing.T) { {"test", "\"test\"\n"}, {enhancedRequest{Foo: "foo"}, "{\"foo\":\"foo\"}\n"}, } { - if _, err := client(context.Background(), test.value); err != nil { + if _, err := endpoint(context.Background(), test.value); err != nil { t.Error(err) continue } @@ -247,7 +247,7 @@ func TestEncodeJSONRequest(t *testing.T) { } } - if _, err := client(context.Background(), enhancedRequest{Foo: "foo"}); err != nil { + if _, err := endpoint(context.Background(), enhancedRequest{Foo: "foo"}); err != nil { t.Fatal(err) } @@ -262,7 +262,7 @@ func TestEncodeJSONRequest(t *testing.T) { func TestSetClient(t *testing.T) { var ( - encode = func(context.Context, *http.Request, interface{}) error { return nil } + encode = func(context.Context, string, string, interface{}) (*http.Request, error) { return nil, nil } decode = func(_ context.Context, r *http.Response) (interface{}, error) { t, err := ioutil.ReadAll(r.Body) if err != nil { @@ -280,15 +280,15 @@ func TestSetClient(t *testing.T) { }, nil }) - client := httptransport.NewClient( + endpoint := httptransport.NewClientEndpoint( "GET", &url.URL{}, encode, decode, - httptransport.SetClient(testHttpClient), - ).Endpoint() + httptransport.ClientEndpointSetClient(testHttpClient), + ) - resp, err := client(context.Background(), nil) + resp, err := endpoint(context.Background(), nil) if err != nil { t.Fatal(err) } diff --git a/transport/http/deprecated_client.go b/transport/http/deprecated_client.go new file mode 100644 index 000000000..9d5910898 --- /dev/null +++ b/transport/http/deprecated_client.go @@ -0,0 +1,192 @@ +package http + +import ( + "bytes" + "context" + "encoding/json" + "encoding/xml" + "io/ioutil" + "net/http" + "net/url" + + "github.com/go-kit/kit/endpoint" +) + +// Client wraps a URL and provides a method that implements endpoint.Endpoint. +// Deprecated: Use NewClientEndpoint instead (for details see https://github.com/go-kit/kit/issues/796). +type Client struct { + client HTTPClient + method string + tgt *url.URL + enc EncodeRequestFunc + dec DecodeResponseFunc + before []RequestFunc + after []ClientResponseFunc + finalizer []ClientFinalizerFunc + bufferedStream bool +} + +// NewClient constructs a usable Client for a single remote method. +// Deprecated: Use NewClientEndpoint instead (for details see https://github.com/go-kit/kit/issues/796). +func NewClient( + method string, + tgt *url.URL, + enc EncodeRequestFunc, + dec DecodeResponseFunc, + options ...ClientOption, +) *Client { + c := &Client{ + client: http.DefaultClient, + method: method, + tgt: tgt, + enc: enc, + dec: dec, + before: []RequestFunc{}, + after: []ClientResponseFunc{}, + bufferedStream: false, + } + for _, option := range options { + option(c) + } + return c +} + +// Endpoint returns a usable endpoint that invokes the remote endpoint. +// Deprecated: Use NewClientEndpoint instead (for details see https://github.com/go-kit/kit/issues/796). +func (c Client) Endpoint() endpoint.Endpoint { + return func(ctx context.Context, request interface{}) (interface{}, error) { + ctx, cancel := context.WithCancel(ctx) + + var ( + resp *http.Response + err error + ) + if c.finalizer != nil { + defer func() { + if resp != nil { + ctx = context.WithValue(ctx, ContextKeyResponseHeaders, resp.Header) + ctx = context.WithValue(ctx, ContextKeyResponseSize, resp.ContentLength) + } + for _, f := range c.finalizer { + f(ctx, err) + } + }() + } + + req, err := http.NewRequest(c.method, c.tgt.String(), nil) + if err != nil { + cancel() + return nil, err + } + + if err = c.enc(ctx, req, request); err != nil { + cancel() + return nil, err + } + + for _, f := range c.before { + ctx = f(ctx, req) + } + + resp, err = c.client.Do(req.WithContext(ctx)) + + if err != nil { + cancel() + return nil, err + } + + // If we expect a buffered stream, we don't cancel the context when the endpoint returns. + // Instead, we should call the cancel func when closing the response body. + if c.bufferedStream { + resp.Body = bodyWithCancel{ReadCloser: resp.Body, cancel: cancel} + } else { + defer resp.Body.Close() + defer cancel() + } + + for _, f := range c.after { + ctx = f(ctx, resp) + } + + response, err := c.dec(ctx, resp) + if err != nil { + return nil, err + } + + return response, nil + } +} + +// ClientOption sets an optional parameter for clients. +// Deprecated: Use ClientEndpointOption instead (for details see https://github.com/go-kit/kit/issues/796). +type ClientOption func(*Client) + +// SetClient sets the underlying HTTP client used for requests. +// By default, http.DefaultClient is used. +// Deprecated: Use ClientEndpointSetClient instead (for details see https://github.com/go-kit/kit/issues/796). +func SetClient(client HTTPClient) ClientOption { + return func(c *Client) { c.client = client } +} + +// ClientBefore sets the RequestFuncs that are applied to the outgoing HTTP +// request before it's invoked. +// Deprecated: Use ClientEndpointBefore instead (for details see https://github.com/go-kit/kit/issues/796). +func ClientBefore(before ...RequestFunc) ClientOption { + return func(c *Client) { c.before = append(c.before, before...) } +} + +// ClientAfter sets the ClientResponseFuncs applied to the incoming HTTP +// request prior to it being decoded. This is useful for obtaining anything off +// of the response and adding onto the context prior to decoding. +// Deprecated: Use ClientEndpointAfter instead (for details see https://github.com/go-kit/kit/issues/796). +func ClientAfter(after ...ClientResponseFunc) ClientOption { + return func(c *Client) { c.after = append(c.after, after...) } +} + +// ClientFinalizer is executed at the end of every HTTP request. +// By default, no finalizer is registered. +// Deprecated: Use ClientEndpointFinalizer instead (for details see https://github.com/go-kit/kit/issues/796). +func ClientFinalizer(f ...ClientFinalizerFunc) ClientOption { + return func(s *Client) { s.finalizer = append(s.finalizer, f...) } +} + +// BufferedStream sets whether the Response.Body is left open, allowing it +// to be read from later. Useful for transporting a file as a buffered stream. +// That body has to be Closed to propery end the request. +// Deprecated: Use ClientEndpointBufferedStream instead (for details see https://github.com/go-kit/kit/issues/796). +func BufferedStream(buffered bool) ClientOption { + return func(c *Client) { c.bufferedStream = buffered } +} + +// EncodeJSONRequest is an EncodeRequestFunc that serializes the request as a +// JSON object to the Request body. Many JSON-over-HTTP services can use it as +// a sensible default. If the request implements Headerer, the provided headers +// will be applied to the request. +// Deprecated: Use EncodeJSONClientRequest instead (for details see https://github.com/go-kit/kit/issues/796). +func EncodeJSONRequest(c context.Context, r *http.Request, request interface{}) error { + r.Header.Set("Content-Type", "application/json; charset=utf-8") + if headerer, ok := request.(Headerer); ok { + for k := range headerer.Headers() { + r.Header.Set(k, headerer.Headers().Get(k)) + } + } + var b bytes.Buffer + r.Body = ioutil.NopCloser(&b) + return json.NewEncoder(&b).Encode(request) +} + +// EncodeXMLRequest is an EncodeRequestFunc that serializes the request as a +// XML object to the Request body. If the request implements Headerer, +// the provided headers will be applied to the request. +// Deprecated: Use EncodeXMLClientRequest instead (for details see https://github.com/go-kit/kit/issues/796). +func EncodeXMLRequest(c context.Context, r *http.Request, request interface{}) error { + r.Header.Set("Content-Type", "text/xml; charset=utf-8") + if headerer, ok := request.(Headerer); ok { + for k := range headerer.Headers() { + r.Header.Set(k, headerer.Headers().Get(k)) + } + } + var b bytes.Buffer + r.Body = ioutil.NopCloser(&b) + return xml.NewEncoder(&b).Encode(request) +} diff --git a/transport/http/encode_decode.go b/transport/http/encode_decode.go index 2b0334b06..63ea73f1d 100644 --- a/transport/http/encode_decode.go +++ b/transport/http/encode_decode.go @@ -11,10 +11,17 @@ import ( // JSON decodes from the request body to the concrete request type. type DecodeRequestFunc func(context.Context, *http.Request) (request interface{}, err error) +// EncodeClientRequestFunc encodes the passed request object and returns it as HTTP request +// object. It's designed to be used in HTTP clients, for client-side +// endpoints. One straightforward EncodeClientRequestFunc could be something that JSON +// encodes the object directly to the request body. +type EncodeClientRequestFunc func(ctx context.Context, method, url string, request interface{}) (*http.Request, error) + // EncodeRequestFunc encodes the passed request object into the HTTP request // object. It's designed to be used in HTTP clients, for client-side // endpoints. One straightforward EncodeRequestFunc could be something that JSON // encodes the object directly to the request body. +// Deprecated: Use EncodeClientRequestFunc instead (for details see https://github.com/go-kit/kit/issues/796). type EncodeRequestFunc func(context.Context, *http.Request, interface{}) error // EncodeResponseFunc encodes the passed response object to the HTTP response