Repository: go-openapi/runtime
Pull request #131 (2019-03-01 02:10:09)

Remove bytecopy from GetBody
req.Body is already populated, and this, even on the first call, will write the payload again.  It seems the solution is to just remove the byte copy all together.

Also adds a test guarding against the bug.  Any feedback on the approach or style is appreciated :)

Closes https://github.com/go-openapi/runtime/issues/130

codecov[bot]: # [Codecov](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=h1) Report
> Merging [#131](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=desc) into [master](https://codecov.io/gh/go-openapi/runtime/commit/41e24cc66d7af6af39eb9b5a6418e901bcdd333c?src=pr&el=desc) will **increase** coverage by `0.25%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/go-openapi/runtime/pull/131/graphs/tree.svg?width=650&token=6wl4NsM8aQ&height=150&src=pr)](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master     #131      +/-   ##
==========================================
+ Coverage   80.59%   80.84%   +0.25%     
==========================================
  Files          39       39              
  Lines        2231     2229       -2     
==========================================
+ Hits         1798     1802       +4     
+ Misses        341      335       -6     
  Partials       92       92
```


| [Impacted Files](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [client/request.go](https://codecov.io/gh/go-openapi/runtime/pull/131/diff?src=pr&el=tree#diff-Y2xpZW50L3JlcXVlc3QuZ28=) | `68.9% <ø> (+3.23%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=footer). Last update [41e24cc...a78512c](https://codecov.io/gh/go-openapi/runtime/pull/131?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/client/request.go b/client/request.go
index a1c1915..e643cf1 100644
--- a/client/request.go
+++ b/client/request.go
@@ -213,9 +213,6 @@ func (r *request) buildHTTP(mediaType, basePath string, producers map[string]run
 				return nil, err
 			}
 
-			if _, err := r.buf.Write(b.Bytes()); err != nil {
-				return nil, err
-			}
 			return ioutil.NopCloser(&b), nil
 		}
 


Test patch:
diff --git a/client/request_test.go b/client/request_test.go
index e6ee6ac..c1db584 100644
--- a/client/request_test.go
+++ b/client/request_test.go
@@ -18,9 +18,14 @@ import (
 	"bytes"
 	"encoding/json"
 	"encoding/xml"
+	"errors"
+	"io"
 	"io/ioutil"
 	"mime"
 	"mime/multipart"
+	"net/http"
+	"net/http/httptest"
+	"net/url"
 	"os"
 	"path/filepath"
 	"strings"
@@ -29,6 +34,7 @@ import (
 	"github.com/go-openapi/runtime"
 	"github.com/go-openapi/strfmt"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 var testProducers = map[string]runtime.Producer{
@@ -509,3 +515,81 @@ func TestBuildRequest_BuildHTTP_EscapedPath(t *testing.T) {
 		assert.Equal(t, runtime.JSONMime, req.Header.Get(runtime.HeaderContentType))
 	}
 }
+
+type testReqFn func(*testing.T, *http.Request)
+
+type testRoundTripper struct {
+	tr          http.RoundTripper
+	testFn      testReqFn
+	testHarness *testing.T
+}
+
+func (t *testRoundTripper) RoundTrip(req *http.Request) (resp *http.Response, err error) {
+	t.testFn(t.testHarness, req)
+	return t.tr.RoundTrip(req)
+}
+
+func TestGetBodyCallsBeforeRoundTrip(t *testing.T) {
+
+	server := httptest.NewServer(http.HandlerFunc(func(rw http.ResponseWriter, req *http.Request) {
+		rw.WriteHeader(http.StatusCreated)
+		_, err := rw.Write([]byte("test result"))
+		require.NoError(t, err)
+	}))
+	defer server.Close()
+	hu, _ := url.Parse(server.URL)
+
+	client := http.DefaultClient
+	transport := http.DefaultTransport
+
+	client.Transport = &testRoundTripper{
+		tr:          transport,
+		testHarness: t,
+		testFn: func(t *testing.T, req *http.Request) {
+			// Read the body once before sending the request
+			body, err := req.GetBody()
+			require.NoError(t, err)
+			bodyContent, err := ioutil.ReadAll(io.Reader(body))
+			require.EqualValues(t, req.ContentLength, len(bodyContent))
+			require.NoError(t, err)
+			require.EqualValues(t, "\"test body\"\n", string(bodyContent))
+
+			// Read the body a second time before sending the request
+			body, err = req.GetBody()
+			require.NoError(t, err)
+			bodyContent, err = ioutil.ReadAll(io.Reader(body))
+			require.NoError(t, err)
+			require.EqualValues(t, req.ContentLength, len(bodyContent))
+			require.EqualValues(t, "\"test body\"\n", string(bodyContent))
+		},
+	}
+
+	rwrtr := runtime.ClientRequestWriterFunc(func(req runtime.ClientRequest, _ strfmt.Registry) error {
+		return req.SetBodyParam("test body")
+	})
+
+	operation := &runtime.ClientOperation{
+		ID:          "getSites",
+		Method:      "POST",
+		PathPattern: "/",
+		Params:      rwrtr,
+		Client:      client,
+		Reader: runtime.ClientResponseReaderFunc(func(response runtime.ClientResponse, consumer runtime.Consumer) (interface{}, error) {
+			if response.Code() == http.StatusCreated {
+				var result string
+				if err := consumer.Consume(response.Body(), &result); err != nil {
+					return nil, err
+				}
+				return result, nil
+			}
+			return nil, errors.New("Unexpected error code")
+		}),
+	}
+
+	openAPIClient := New(hu.Host, "/", []string{"http"})
+	res, err := openAPIClient.Submit(operation)
+
+	require.NoError(t, err)
+	actual := res.(string)
+	require.EqualValues(t, "test result", actual)
+}