Merge lp:~axwalk/gwacl/retry-rewind-request-body into lp:gwacl

Proposed by Andrew Wilkins on 2014-03-10
Status: Merged
Approved by: Andrew Wilkins on 2014-03-11
Approved revision: 233
Merged at revision: 232
Proposed branch: lp:~axwalk/gwacl/retry-rewind-request-body
Merge into: lp:gwacl
Diff against target: 80 lines (+39/-2)
2 files modified
x509dispatcher.go (+14/-2)
x509dispatcher_test.go (+25/-0)
To merge this branch: bzr merge lp:~axwalk/gwacl/retry-rewind-request-body
Reviewer Review Type Date Requested Status
Ian Booth 2014-03-10 Approve on 2014-03-11
Review via email: mp+210118@code.launchpad.net

Commit message

Rewind body when retrying POST/PUT/etc.

Azure may tell us to retry POST/PUT, which
may have a body. We must rewind the reader
if this happens.

Description of the change

Rewind body when retrying POST/PUT/etc.

Azure may tell us to retry POST/PUT, which
may have a body. We must rewind the reader
if this happens.

To post a comment you must log in.
Ian Booth (wallyworld) wrote :

Looks good.
Small typo:
+// to the beginning of the reader upong Close being called.

review: Approve
Go Bot (go-bot) wrote :

The attempt to merge lp:~axwalk/gwacl/retry-rewind-request-body into lp:gwacl failed. Below is the output from the failed tests.

./utilities/format -s
? launchpad.net/gwacl/example/management [no test files]
? launchpad.net/gwacl/example/storage [no test files]
? launchpad.net/gwacl/fork/http [no test files]
? launchpad.net/gwacl/fork/tls [no test files]

# launchpad.net/gwacl
deletedisk_test.go:8:5: cannot find package "launchpad.net/gocheck" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gocheck (from $GOROOT)
 /home/tarmac/gwacl-trees/src/launchpad.net/gocheck (from $GOPATH)
# launchpad.net/gwacl/dedent
deletedisk_test.go:8:5: cannot find package "launchpad.net/gocheck" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gocheck (from $GOROOT)
 /home/tarmac/gwacl-trees/src/launchpad.net/gocheck (from $GOPATH)
# launchpad.net/gwacl/logging
deletedisk_test.go:8:5: cannot find package "launchpad.net/gocheck" in any of:
 /usr/lib/go/src/pkg/launchpad.net/gocheck (from $GOROOT)
 /home/tarmac/gwacl-trees/src/launchpad.net/gocheck (from $GOPATH)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'x509dispatcher.go'
2--- x509dispatcher.go 2014-02-05 04:46:09 +0000
3+++ x509dispatcher.go 2014-03-11 03:39:32 +0000
4@@ -6,7 +6,7 @@
5 import (
6 "bytes"
7 "fmt"
8- "io/ioutil"
9+ "io"
10 "launchpad.net/gwacl/fork/http"
11 . "launchpad.net/gwacl/logging"
12 "net/url"
13@@ -70,6 +70,17 @@
14 Header http.Header
15 }
16
17+// rewindCloser returns an io.ReadCloser that seeks back
18+// to the beginning of the reader upon Close being called.
19+type rewindCloser struct {
20+ io.ReadSeeker
21+}
22+
23+func (c rewindCloser) Close() error {
24+ _, err := c.Seek(0, 0)
25+ return err
26+}
27+
28 func performX509Request(session *x509Session, request *X509Request) (*x509Response, error) {
29 response := &x509Response{}
30
31@@ -78,11 +89,12 @@
32 Debugf("Request body:\n%s", request.Payload)
33 }
34
35- bodyReader := ioutil.NopCloser(bytes.NewReader(request.Payload))
36+ bodyReader := rewindCloser{bytes.NewReader(request.Payload)}
37 httpRequest, err := http.NewRequest(request.Method, request.URL, bodyReader)
38 if err != nil {
39 return nil, err
40 }
41+ httpRequest.ContentLength = int64(len(request.Payload))
42 httpRequest.Header.Set("Content-Type", request.ContentType)
43 httpRequest.Header.Set("x-ms-version", request.APIVersion)
44 retrier := session.retryPolicy.getForkedHttpRetrier(session.client)
45
46=== modified file 'x509dispatcher_test.go'
47--- x509dispatcher_test.go 2014-02-05 04:46:09 +0000
48+++ x509dispatcher_test.go 2014-03-11 03:39:32 +0000
49@@ -219,6 +219,31 @@
50 }
51 }
52
53+func (*x509DispatcherSuite) TestRedirectRewindsBody(c *C) {
54+ httpRequests := make(chan *Request, 2)
55+ serverConflict := makeRecordingHTTPServer(httpRequests, http.StatusConflict, nil, nil)
56+ defer serverConflict.Close()
57+ redirPath := "/else/where"
58+ responseHeaders := make(http.Header)
59+ responseHeaders.Set("Location", serverConflict.URL+redirPath)
60+ serverRedir := makeRecordingHTTPServer(httpRequests, http.StatusTemporaryRedirect, nil, responseHeaders)
61+ defer serverRedir.Close()
62+ session, err := newX509Session("subscriptionid", "", "West US", NoRetryPolicy)
63+ c.Assert(err, IsNil)
64+ path := "/foo/bar"
65+ version := "test-version"
66+ content := []byte("ponies")
67+ contentType := "text/plain"
68+
69+ request := newX509RequestPOST(serverRedir.URL+path, version, content, contentType)
70+ response, err := performX509Request(session, request)
71+ c.Assert(err, IsNil)
72+ c.Assert(response.StatusCode, Equals, http.StatusConflict)
73+ c.Assert(httpRequests, HasLen, 2)
74+ c.Assert((<-httpRequests).BodyContent, DeepEquals, content)
75+ c.Assert((<-httpRequests).BodyContent, DeepEquals, content)
76+}
77+
78 func (*x509DispatcherSuite) TestRequestsLimitRedirects(c *C) {
79 httpRequests := make(chan *Request, 10)
80 serverRedir := makeRecordingHTTPServer(httpRequests, http.StatusTemporaryRedirect, nil, nil)

Subscribers

People subscribed via source and target branches

to all changes: