goamz

Merge lp:~niemeyer/goamz/put-all into lp:~gophers/goamz/trunk

  1. put-all
  2. Merge into trunk
Proposed by Gustavo Niemeyer
Status: Merged
Merged at revision: 31
Proposed branch: lp:~niemeyer/goamz/put-all
Merge into: lp:~gophers/goamz/trunk
Diff against target: 632 lines (+366/-54)
5 files modified
s3/multi.go (+109/-17)
s3/multi_test.go (+218/-26)
s3/responses_test.go (+6/-6)
s3/s3_test.go (+11/-4)
s3/s3i_test.go (+22/-1)
To merge this branch: bzr merge lp:~niemeyer/goamz/put-all
Reviewer Review Type Date Requested Status
The Go Language Gophers Pending
Review via email: mp+147829@code.launchpad.net

Description of the change

s3: Multi.PutAll for auto-chunking and resuming

Bucket.Multi method also added for trivial resume-or-init handling.

https://codereview.appspot.com/7313081/

To post a comment you must log in.
Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Reviewers: mp+147829_code.launchpad.net,

Message:
Please take a look.

Description:
s3: Multi.PutAll for auto-chunking and resuming

Bucket.Multi method also added for trivial resume-or-init handling.

https://code.launchpad.net/~niemeyer/goamz/put-all/+merge/147829

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/7313081/

Affected files:
   A [revision details]
   M s3/multi.go
   M s3/multi_test.go
   M s3/responses_test.go
   M s3/s3_test.go
   M s3/s3i_test.go

Revision history for this message
John A Meinel (jameinel) wrote :

LGTM

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode272
s3/multi.go:272: // the new part, or otherwise overwritten with the new
content.
does partSize need to match exactly the pre-uploaded content? I don't
see places where that is checked here. Probably at least a doc comment
about it.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
What will this return if totalSize is 0? the return of ListParts? But
doesn't that violate the 'you should return nil, otherwise the caller
gets an interface object that contains nil but isn't nil'?

So I would probably split this:
if err != nil {
   return nil, err
} else if totalSize == 0 {
   return nil, nil
}

Though since you don't have a result object, wouldn't you want a non-nil
err? Otherwise you'll get a segfault accessing the result later.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode310
s3/multi.go:310: part, err := m.putPart(current, section, partSize,
md5b64)
As an aside, would there be any reason to do parallel uploads?

https://codereview.appspot.com/7313081/

Revision history for this message
Dimiter Naydenov (dimitern) wrote :

LGTM, a few suggestions.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode227
s3/multi.go:227: var listPartsMax = 1000
Is this an absolute limit of all parts per client connection?

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/12 07:32:38, jameinel wrote:
> What will this return if totalSize is 0? the return of ListParts? But
 doesn't
> that violate the 'you should return nil, otherwise the caller gets an
 interface
> object that contains nil but isn't nil'?

> So I would probably split this:
> if err != nil {
> return nil, err
> } else if totalSize == 0 {
> return nil, nil
> }

> Though since you don't have a result object, wouldn't you want a
non-nil err?
> Otherwise you'll get a segfault accessing the result later.

ISTM if totalSize == 0 it will return nil, nil, since there won't be an
error. But if it's not obvious, +1 on the suggestion.

https://codereview.appspot.com/7313081/diff/1/s3/multi_test.go
File s3/multi_test.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi_test.go#newcode53
s3/multi_test.go:53: c.Assert(multi.UploadId, Matches,
"JNbR_[A-Za-z0-9.]+QQ--")
Maybe move this in assertId()? This seems to be used multiple times, so
it'll be better to have it at one place, I think.

https://codereview.appspot.com/7313081/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Thanks for the reviews.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode227
s3/multi.go:227: var listPartsMax = 1000
On 2013/02/12 08:02:52, dimitern wrote:
> Is this an absolute limit of all parts per client connection?

No, this is a per-request limit, and 1000 is the default limit as well.
It's here as a variable just so we can test it. I'll add a comment.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode272
s3/multi.go:272: // the new part, or otherwise overwritten with the new
content.
On 2013/02/12 07:32:38, jameinel wrote:
> does partSize need to match exactly the pre-uploaded content? I don't
 see places
> where that is checked here. Probably at least a doc comment about it.

It's actually explicitly checked, but that's a bit redundant with the
above statement. If the size changes, the checksum won't match. That
said, given that you felt the need to ask, it won't hurt to mention it
explicitly.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/12 07:32:38, jameinel wrote:
> So I would probably split this:
> if err != nil {
> return nil, err
> } else if totalSize == 0 {
> return nil, nil
> }

Hmm.. that does exactly the same thing that the current version does.

> Though since you don't have a result object, wouldn't you want a
non-nil err?
> Otherwise you'll get a segfault accessing the result later.

That's not the case with slices. The most conventional way to represent
an empty slice is with a nil/zero value. len, append, etc, all work
properly with them, and code taking slices should behave properly as
well, or may otherwise be considered broken.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode310
s3/multi.go:310: part, err := m.putPart(current, section, partSize,
md5b64)
On 2013/02/12 07:32:38, jameinel wrote:
> As an aside, would there be any reason to do parallel uploads?

That's an interesting question. I guess we could probably get some speed
up by sending parts in parallel. It's a good topic to do some
experimentation on.

https://codereview.appspot.com/7313081/

Revision history for this message
John A Meinel (jameinel) wrote :

LGTM

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
That's fine. I wasn't thinking about it being a slice, it does seem
correct there.

The concern was stuff like: http://play.golang.org/p/ZcjGU2BgbC

But that doesn't apply here because err has type "error" rather than a
custom type that could be a nil pointer.

https://codereview.appspot.com/7313081/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

really nice API.
a few suggestions below.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode88
s3/multi.go:88: // inside b. If a multipart upload exists for key, it is
returned,
if we assume that a bucket implies its key, then i think the explanation
can become simpler. how about this, perhaps?

// Multi returns a multipart upload handler for the bucket.
// If one already exists, it is returned, otherwise
// a new multipart upload is initiated with contType and perm.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode229
s3/multi.go:229: // ListParts returns the list of previously uploaded
parts in m.
// ordered by part number.
?

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode269
s3/multi.go:269: // PutAll sends all of r via a multipart upload with
parts no large
s/large/larger/

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode273
s3/multi.go:273: // PutAll returns all the parts of m (reused or not).
nice interface.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode279
s3/multi.go:279: reuse := 1 // Index of next old part to consider
reusing.
I was a little confused here by the one-based indexing used for reuse
here - it makes it look like it's in the same units as current, but
reuse really is an index (into old) and current is a part number.

Also, current isn't really the index of the last good part handled,
AFAICS - it's the part number of the part we're about to consider
putting.

I'd be inclined to do:

reuse := 0 // Index of next old part to consider reusing.
current := 1 // Part number of current part.

and adjust the use of reuse below.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/12 08:02:52, dimitern wrote:
> On 2013/02/12 07:32:38, jameinel wrote:
> > What will this return if totalSize is 0? the return of ListParts?
 But doesn't
> > that violate the 'you should return nil, otherwise the caller gets
 an
> interface
> > object that contains nil but isn't nil'?
> >
> > So I would probably split this:
> > if err != nil {
> > return nil, err
> > } else if totalSize == 0 {
> > return nil, nil
> > }
> >
> > Though since you don't have a result object, wouldn't you want a
 non-nil err?
> > Otherwise you'll get a segfault accessing the result later.

> ISTM if totalSize == 0 it will return nil, nil, since there won't be
an error.
> But if it's not obvious, +1 on the suggestion.

i don't mind the way it's phrased here, but i would like to see a test
case that tests putting a zero-length file live (i *presume* you can
read a file ok if it's had no PUT requests, but it's only a
presumption). perhaps i missed a relevant test though.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode316
s3/multi.go:316: }
the logic above is moderately intricate - i'd like to see some more test
cases to go with TestPutAllResume,
at least a case with a reused part in the middle of other non-reused
parts, and vice versa.

https://codereview.appspot.com/7313081/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :
Download full text (3.6 KiB)

https://codereview.appspot.com/7313081/diff/1/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode88
s3/multi.go:88: // inside b. If a multipart upload exists for key, it is
returned,
On 2013/02/13 10:22:03, rog wrote:
> if we assume that a bucket implies its key, then i think the
 explanation can
> become simpler. how about this, perhaps?

> // Multi returns a multipart upload handler for the bucket.
> // If one already exists, it is returned, otherwise
> // a new multipart upload is initiated with contType and perm.

A bucket doesn't imply its key. A bucket contains keys. What's described
above is not the same as what was documented in the method, or what the
method does.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode229
s3/multi.go:229: // ListParts returns the list of previously uploaded
parts in m.
On 2013/02/13 10:22:03, rog wrote:
> // ordered by part number.
> ?

Sounds good.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode269
s3/multi.go:269: // PutAll sends all of r via a multipart upload with
parts no large
On 2013/02/13 10:22:03, rog wrote:
> s/large/larger/

Thanks.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode279
s3/multi.go:279: reuse := 1 // Index of next old part to consider
reusing.
On 2013/02/13 10:22:03, rog wrote:
> I was a little confused here by the one-based indexing used for reuse
 here - it
> makes it look like it's in the same units as current, but reuse really
 is an
> index (into old) and current is a part number.

> Also, current isn't really the index of the last good part handled,
AFAICS -
> it's the part number of the part we're about to consider putting.

> I'd be inclined to do:

> reuse := 0 // Index of next old part to consider reusing.
> current := 1 // Part number of current part.

> and adjust the use of reuse below.

Sounds good.

https://codereview.appspot.com/7313081/diff/1/s3/multi.go#newcode282
s3/multi.go:282: if totalSize == 0 || err != nil {
On 2013/02/13 10:22:03, rog wrote:
> On 2013/02/12 08:02:52, dimitern wrote:
> > On 2013/02/12 07:32:38, jameinel wrote:
> > > What will this return if totalSize is 0? the return of ListParts?
 But
> doesn't
> > > that violate the 'you should return nil, otherwise the caller gets
 an
> > interface
> > > object that contains nil but isn't nil'?
> > >
> > > So I would probably split this:
> > > if err != nil {
> > > return nil, err
> > > } else if totalSize == 0 {
> > > return nil, nil
> > > }
> > >
> > > Though since you don't have a result object, wouldn't you want a
 non-nil
> err?
> > > Otherwise you'll get a segfault accessing the result later.
> >
> > ISTM if totalSize == 0 it will return nil, nil, since there won't be
 an error.
> > But if it's not obvious, +1 on the suggestion.

> i don't mind the way it's phrased here, but i would like to see a test
case that
> tests putting a zero-length file live (i *presume* you can read a file
ok if
> it's had no PUT requests, but it's only a presumption). perhaps i
missed a
> relevant test though.

I don't think it matters whether that works or not in the server. That's
an edge case, and the ser...

Read more...

lp:~niemeyer/goamz/put-all updated
32. By Gustavo Niemeyer

Changes after reviews.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Please take a look.

https://codereview.appspot.com/7313081/

Revision history for this message
Roger Peppe (rogpeppe) wrote :

LGTM modulo the below.

https://codereview.appspot.com/7313081/diff/8001/s3/multi.go
File s3/multi.go (right):

https://codereview.appspot.com/7313081/diff/8001/s3/multi.go#newcode276
s3/multi.go:276: // new content.
As discussed, perhaps a comment like:
// Multipart uploads of zero length files may not work.

https://codereview.appspot.com/7313081/diff/8001/s3/multi.go#newcode284
s3/multi.go:284: current := 1 // Index of latest good part handled.
This comment is still misleading. Current isn't an index in the same way
that reuse is, and at this point we haven't any good parts, so there's
no "latest" good part - current is the part number of the next part to
handle.

https://codereview.appspot.com/7313081/

lp:~niemeyer/goamz/put-all updated
33. By Gustavo Niemeyer

Add edge case for zero-length files.

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

Please take a look.

https://codereview.appspot.com/7313081/

Revision history for this message
Gustavo Niemeyer (niemeyer) wrote :

*** Submitted:

s3: Multi.PutAll for auto-chunking and resuming

Bucket.Multi method also added for trivial resume-or-init handling.

R=jameinel, dimitern, rog
CC=
https://codereview.appspot.com/7313081

https://codereview.appspot.com/7313081/

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 's3/multi.go'
--- s3/multi.go 2013-02-05 22:56:23 +0000
+++ s3/multi.go 2013-02-13 12:22:21 +0000
@@ -4,6 +4,7 @@
4 "bytes"4 "bytes"
5 "crypto/md5"5 "crypto/md5"
6 "encoding/base64"6 "encoding/base64"
7 "encoding/hex"
7 "encoding/xml"8 "encoding/xml"
8 "errors"9 "errors"
9 "io"10 "io"
@@ -24,6 +25,7 @@
24 UploadId string25 UploadId string
25}26}
2627
28// That's the default. Here just for testing.
27var listMultiMax = 100029var listMultiMax = 1000
2830
29type listMultiResp struct {31type listMultiResp struct {
@@ -83,6 +85,22 @@
83 panic("unreachable")85 panic("unreachable")
84}86}
8587
88// Multi returns a multipart upload handler for the provided key
89// inside b. If a multipart upload exists for key, it is returned,
90// otherwise a new multipart upload is initiated with contType and perm.
91func (b *Bucket) Multi(key, contType string, perm ACL) (*Multi, error) {
92 multis, _, err := b.ListMulti(key, "")
93 if err != nil && !hasCode(err, "NoSuchUpload") {
94 return nil, err
95 }
96 for _, m := range multis {
97 if m.Key == key {
98 return m, nil
99 }
100 }
101 return b.InitMulti(key, contType, perm)
102}
103
86// InitMulti initializes a new multipart upload at the provided104// InitMulti initializes a new multipart upload at the provided
87// key inside b and returns a value for manipulating it.105// key inside b and returns a value for manipulating it.
88//106//
@@ -124,13 +142,17 @@
124//142//
125// See http://goo.gl/pqZer for details.143// See http://goo.gl/pqZer for details.
126func (m *Multi) PutPart(n int, r io.ReadSeeker) (Part, error) {144func (m *Multi) PutPart(n int, r io.ReadSeeker) (Part, error) {
127 length, b64md5, err := seekerInfo(r)145 partSize, _, md5b64, err := seekerInfo(r)
128 if err != nil {146 if err != nil {
129 return Part{}, err147 return Part{}, err
130 }148 }
149 return m.putPart(n, r, partSize, md5b64)
150}
151
152func (m *Multi) putPart(n int, r io.ReadSeeker, partSize int64, md5b64 string) (Part, error) {
131 headers := map[string][]string{153 headers := map[string][]string{
132 "Content-Length": {strconv.FormatInt(length, 10)},154 "Content-Length": {strconv.FormatInt(partSize, 10)},
133 "Content-MD5": {b64md5},155 "Content-MD5": {md5b64},
134 }156 }
135 params := map[string][]string{157 params := map[string][]string{
136 "uploadId": {m.UploadId},158 "uploadId": {m.UploadId},
@@ -164,23 +186,25 @@
164 if etag == "" {186 if etag == "" {
165 return Part{}, errors.New("part upload succeeded with no ETag")187 return Part{}, errors.New("part upload succeeded with no ETag")
166 }188 }
167 return Part{n, etag, length}, nil189 return Part{n, etag, partSize}, nil
168 }190 }
169 panic("unreachable")191 panic("unreachable")
170}192}
171193
172func seekerInfo(r io.ReadSeeker) (length int64, b64md5 string, err error) {194func seekerInfo(r io.ReadSeeker) (size int64, md5hex string, md5b64 string, err error) {
173 _, err = r.Seek(0, 0)195 _, err = r.Seek(0, 0)
174 if err != nil {196 if err != nil {
175 return 0, "", err197 return 0, "", "", err
176 }198 }
177 digest := md5.New()199 digest := md5.New()
178 length, err = io.Copy(digest, r)200 size, err = io.Copy(digest, r)
179 if err != nil {201 if err != nil {
180 return 0, "", err202 return 0, "", "", err
181 }203 }
182 b64md5 = base64.StdEncoding.EncodeToString(digest.Sum(nil))204 sum := digest.Sum(nil)
183 return length, b64md5, nil205 md5hex = hex.EncodeToString(sum)
206 md5b64 = base64.StdEncoding.EncodeToString(sum)
207 return size, md5hex, md5b64, nil
184}208}
185209
186type Part struct {210type Part struct {
@@ -189,15 +213,23 @@
189 Size int64213 Size int64
190}214}
191215
216type partSlice []Part
217
218func (s partSlice) Len() int { return len(s) }
219func (s partSlice) Less(i, j int) bool { return s[i].N < s[j].N }
220func (s partSlice) Swap(i, j int) { s[i], s[j] = s[j], s[i] }
221
192type listPartsResp struct {222type listPartsResp struct {
193 NextPartNumberMarker string223 NextPartNumberMarker string
194 IsTruncated bool224 IsTruncated bool
195 Part []Part225 Part []Part
196}226}
197227
228// That's the default. Here just for testing.
198var listPartsMax = 1000229var listPartsMax = 1000
199230
200// ListParts returns the list of previously uploaded parts in m.231// ListParts returns the list of previously uploaded parts in m,
232// ordered by part number.
201//233//
202// See http://goo.gl/ePioY for details.234// See http://goo.gl/ePioY for details.
203func (m *Multi) ListParts() ([]Part, error) {235func (m *Multi) ListParts() ([]Part, error) {
@@ -205,7 +237,7 @@
205 "uploadId": {m.UploadId},237 "uploadId": {m.UploadId},
206 "max-parts": {strconv.FormatInt(int64(listPartsMax), 10)},238 "max-parts": {strconv.FormatInt(int64(listPartsMax), 10)},
207 }239 }
208 var parts []Part240 var parts partSlice
209 for attempt := attempts.Start(); attempt.Next(); {241 for attempt := attempts.Start(); attempt.Next(); {
210 req := &request{242 req := &request{
211 method: "GET",243 method: "GET",
@@ -223,6 +255,7 @@
223 }255 }
224 parts = append(parts, resp.Part...)256 parts = append(parts, resp.Part...)
225 if !resp.IsTruncated {257 if !resp.IsTruncated {
258 sort.Sort(parts)
226 return parts, nil259 return parts, nil
227 }260 }
228 params["part-number-marker"] = []string{resp.NextPartNumberMarker}261 params["part-number-marker"] = []string{resp.NextPartNumberMarker}
@@ -231,6 +264,65 @@
231 panic("unreachable")264 panic("unreachable")
232}265}
233266
267type ReaderAtSeeker interface {
268 io.ReaderAt
269 io.ReadSeeker
270}
271
272// PutAll sends all of r via a multipart upload with parts no larger
273// than partSize bytes, which must be set to at least 5MB.
274// Parts previously uploaded are either reused if their checksum
275// and size match the new part, or otherwise overwritten with the
276// new content.
277// PutAll returns all the parts of m (reused or not).
278func (m *Multi) PutAll(r ReaderAtSeeker, partSize int64) ([]Part, error) {
279 old, err := m.ListParts()
280 if err != nil && !hasCode(err, "NoSuchUpload") {
281 return nil, err
282 }
283 reuse := 0 // Index of next old part to consider reusing.
284 current := 1 // Part number of latest good part handled.
285 totalSize, err := r.Seek(0, 2)
286 if err != nil {
287 return nil, err
288 }
289 first := true // Must send at least one empty part if the file is empty.
290 var result []Part
291NextSection:
292 for offset := int64(0); offset < totalSize || first; offset += partSize {
293 first = false
294 if offset+partSize > totalSize {
295 partSize = totalSize - offset
296 }
297 section := io.NewSectionReader(r, offset, partSize)
298 _, md5hex, md5b64, err := seekerInfo(section)
299 if err != nil {
300 return nil, err
301 }
302 for reuse < len(old) && old[reuse].N <= current {
303 // Looks like this part was already sent.
304 part := &old[reuse]
305 etag := `"` + md5hex + `"`
306 if part.N == current && part.Size == partSize && part.ETag == etag {
307 // Checksum matches. Reuse the old part.
308 result = append(result, *part)
309 current++
310 continue NextSection
311 }
312 reuse++
313 }
314
315 // Part wasn't found or doesn't match. Send it.
316 part, err := m.putPart(current, section, partSize, md5b64)
317 if err != nil {
318 return nil, err
319 }
320 result = append(result, part)
321 current++
322 }
323 return result, nil
324}
325
234type completeUpload struct {326type completeUpload struct {
235 XMLName xml.Name `xml:"CompleteMultipartUpload"`327 XMLName xml.Name `xml:"CompleteMultipartUpload"`
236 Parts completeParts `xml:"Part"`328 Parts completeParts `xml:"Part"`
@@ -298,14 +390,14 @@
298// See http://goo.gl/dnyJw for details.390// See http://goo.gl/dnyJw for details.
299func (m *Multi) Abort() error {391func (m *Multi) Abort() error {
300 params := map[string][]string{392 params := map[string][]string{
301 "uploadId": {m.UploadId},393 "uploadId": {m.UploadId},
302 }394 }
303 for attempt := attempts.Start(); attempt.Next(); {395 for attempt := attempts.Start(); attempt.Next(); {
304 req := &request{396 req := &request{
305 method: "DELETE",397 method: "DELETE",
306 bucket: m.Bucket.Name,398 bucket: m.Bucket.Name,
307 path: m.Key,399 path: m.Key,
308 params: params,400 params: params,
309 }401 }
310 err := m.Bucket.S3.query(req, nil)402 err := m.Bucket.S3.query(req, nil)
311 if shouldRetry(err) && attempt.HasNext() {403 if shouldRetry(err) && attempt.HasNext() {
312404
=== modified file 's3/multi_test.go'
--- s3/multi_test.go 2013-02-01 05:58:42 +0000
+++ s3/multi_test.go 2013-02-13 12:22:21 +0000
@@ -2,12 +2,13 @@
22
3import (3import (
4 "encoding/xml"4 "encoding/xml"
5 "io"
6 "io/ioutil"
5 "launchpad.net/goamz/s3"7 "launchpad.net/goamz/s3"
6 . "launchpad.net/gocheck"8 . "launchpad.net/gocheck"
7 "strings"9 "strings"
8)10)
911
10
11func (s *S) TestInitMulti(c *C) {12func (s *S) TestInitMulti(c *C) {
12 testServer.Response(200, nil, InitMultiResultDump)13 testServer.Response(200, nil, InitMultiResultDump)
1314
@@ -26,6 +27,88 @@
26 c.Assert(multi.UploadId, Matches, "JNbR_[A-Za-z0-9.]+QQ--")27 c.Assert(multi.UploadId, Matches, "JNbR_[A-Za-z0-9.]+QQ--")
27}28}
2829
30func (s *S) TestMultiNoPreviousUpload(c *C) {
31 // Don't retry the NoSuchUpload error.
32 s.DisableRetries()
33
34 testServer.Response(404, nil, NoSuchUploadErrorDump)
35 testServer.Response(200, nil, InitMultiResultDump)
36
37 b := s.s3.Bucket("sample")
38
39 multi, err := b.Multi("multi", "text/plain", s3.Private)
40 c.Assert(err, IsNil)
41
42 req := testServer.WaitRequest()
43 c.Assert(req.Method, Equals, "GET")
44 c.Assert(req.URL.Path, Equals, "/sample/")
45 c.Assert(req.Form["uploads"], DeepEquals, []string{""})
46 c.Assert(req.Form["prefix"], DeepEquals, []string{"multi"})
47
48 req = testServer.WaitRequest()
49 c.Assert(req.Method, Equals, "POST")
50 c.Assert(req.URL.Path, Equals, "/sample/multi")
51 c.Assert(req.Form["uploads"], DeepEquals, []string{""})
52
53 c.Assert(multi.UploadId, Matches, "JNbR_[A-Za-z0-9.]+QQ--")
54}
55
56func (s *S) TestMultiReturnOld(c *C) {
57 testServer.Response(200, nil, ListMultiResultDump)
58
59 b := s.s3.Bucket("sample")
60
61 multi, err := b.Multi("multi1", "text/plain", s3.Private)
62 c.Assert(err, IsNil)
63 c.Assert(multi.Key, Equals, "multi1")
64 c.Assert(multi.UploadId, Equals, "iUVug89pPvSswrikD")
65
66 req := testServer.WaitRequest()
67 c.Assert(req.Method, Equals, "GET")
68 c.Assert(req.URL.Path, Equals, "/sample/")
69 c.Assert(req.Form["uploads"], DeepEquals, []string{""})
70 c.Assert(req.Form["prefix"], DeepEquals, []string{"multi1"})
71}
72
73func (s *S) TestListParts(c *C) {
74 testServer.Response(200, nil, InitMultiResultDump)
75 testServer.Response(200, nil, ListPartsResultDump1)
76 testServer.Response(404, nil, NoSuchUploadErrorDump) // :-(
77 testServer.Response(200, nil, ListPartsResultDump2)
78
79 b := s.s3.Bucket("sample")
80
81 multi, err := b.InitMulti("multi", "text/plain", s3.Private)
82 c.Assert(err, IsNil)
83
84 parts, err := multi.ListParts()
85 c.Assert(err, IsNil)
86 c.Assert(parts, HasLen, 3)
87 c.Assert(parts[0].N, Equals, 1)
88 c.Assert(parts[0].Size, Equals, int64(5))
89 c.Assert(parts[0].ETag, Equals, `"ffc88b4ca90a355f8ddba6b2c3b2af5c"`)
90 c.Assert(parts[1].N, Equals, 2)
91 c.Assert(parts[1].Size, Equals, int64(5))
92 c.Assert(parts[1].ETag, Equals, `"d067a0fa9dc61a6e7195ca99696b5a89"`)
93 c.Assert(parts[2].N, Equals, 3)
94 c.Assert(parts[2].Size, Equals, int64(5))
95 c.Assert(parts[2].ETag, Equals, `"49dcd91231f801159e893fb5c6674985"`)
96 testServer.WaitRequest()
97 req := testServer.WaitRequest()
98 c.Assert(req.Method, Equals, "GET")
99 c.Assert(req.URL.Path, Equals, "/sample/multi")
100 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")
101 c.Assert(req.Form["max-parts"], DeepEquals, []string{"1000"})
102
103 testServer.WaitRequest() // The internal error.
104 req = testServer.WaitRequest()
105 c.Assert(req.Method, Equals, "GET")
106 c.Assert(req.URL.Path, Equals, "/sample/multi")
107 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")
108 c.Assert(req.Form["max-parts"], DeepEquals, []string{"1000"})
109 c.Assert(req.Form["part-number-marker"], DeepEquals, []string{"2"})
110}
111
29func (s *S) TestPutPart(c *C) {112func (s *S) TestPutPart(c *C) {
30 headers := map[string]string{113 headers := map[string]string{
31 "ETag": `"26f90efd10d614f100252ff56d88dad8"`,114 "ETag": `"26f90efd10d614f100252ff56d88dad8"`,
@@ -54,44 +137,153 @@
54 c.Assert(req.Header["Content-Md5"], DeepEquals, []string{"JvkO/RDWFPEAJS/1bYja2A=="})137 c.Assert(req.Header["Content-Md5"], DeepEquals, []string{"JvkO/RDWFPEAJS/1bYja2A=="})
55}138}
56139
57func (s *S) TestListParts(c *C) {140func readAll(r io.Reader) string {
141 data, err := ioutil.ReadAll(r)
142 if err != nil {
143 panic(err)
144 }
145 return string(data)
146}
147
148func (s *S) TestPutAllNoPreviousUpload(c *C) {
149 // Don't retry the NoSuchUpload error.
150 s.DisableRetries()
151
152 etag1 := map[string]string{"ETag": `"etag1"`}
153 etag2 := map[string]string{"ETag": `"etag2"`}
154 etag3 := map[string]string{"ETag": `"etag3"`}
155 testServer.Response(200, nil, InitMultiResultDump)
156 testServer.Response(404, nil, NoSuchUploadErrorDump)
157 testServer.Response(200, etag1, "")
158 testServer.Response(200, etag2, "")
159 testServer.Response(200, etag3, "")
160
161 b := s.s3.Bucket("sample")
162
163 multi, err := b.InitMulti("multi", "text/plain", s3.Private)
164 c.Assert(err, IsNil)
165
166 parts, err := multi.PutAll(strings.NewReader("part1part2last"), 5)
167 c.Assert(parts, HasLen, 3)
168 c.Assert(parts[0].ETag, Equals, `"etag1"`)
169 c.Assert(parts[1].ETag, Equals, `"etag2"`)
170 c.Assert(parts[2].ETag, Equals, `"etag3"`)
171 c.Assert(err, IsNil)
172
173 // Init
174 testServer.WaitRequest()
175
176 // List old parts. Won't find anything.
177 req := testServer.WaitRequest()
178 c.Assert(req.Method, Equals, "GET")
179 c.Assert(req.URL.Path, Equals, "/sample/multi")
180
181 // Send part 1.
182 req = testServer.WaitRequest()
183 c.Assert(req.Method, Equals, "PUT")
184 c.Assert(req.URL.Path, Equals, "/sample/multi")
185 c.Assert(req.Form["partNumber"], DeepEquals, []string{"1"})
186 c.Assert(req.Header["Content-Length"], DeepEquals, []string{"5"})
187 c.Assert(readAll(req.Body), Equals, "part1")
188
189 // Send part 2.
190 req = testServer.WaitRequest()
191 c.Assert(req.Method, Equals, "PUT")
192 c.Assert(req.URL.Path, Equals, "/sample/multi")
193 c.Assert(req.Form["partNumber"], DeepEquals, []string{"2"})
194 c.Assert(req.Header["Content-Length"], DeepEquals, []string{"5"})
195 c.Assert(readAll(req.Body), Equals, "part2")
196
197 // Send part 3 with shorter body.
198 req = testServer.WaitRequest()
199 c.Assert(req.Method, Equals, "PUT")
200 c.Assert(req.URL.Path, Equals, "/sample/multi")
201 c.Assert(req.Form["partNumber"], DeepEquals, []string{"3"})
202 c.Assert(req.Header["Content-Length"], DeepEquals, []string{"4"})
203 c.Assert(readAll(req.Body), Equals, "last")
204}
205
206func (s *S) TestPutAllZeroSizeFile(c *C) {
207 // Don't retry the NoSuchUpload error.
208 s.DisableRetries()
209
210 etag1 := map[string]string{"ETag": `"etag1"`}
211 testServer.Response(200, nil, InitMultiResultDump)
212 testServer.Response(404, nil, NoSuchUploadErrorDump)
213 testServer.Response(200, etag1, "")
214
215 b := s.s3.Bucket("sample")
216
217 multi, err := b.InitMulti("multi", "text/plain", s3.Private)
218 c.Assert(err, IsNil)
219
220 // Must send at least one part, so that completing it will work.
221 parts, err := multi.PutAll(strings.NewReader(""), 5)
222 c.Assert(parts, HasLen, 1)
223 c.Assert(parts[0].ETag, Equals, `"etag1"`)
224 c.Assert(err, IsNil)
225
226 // Init
227 testServer.WaitRequest()
228
229 // List old parts. Won't find anything.
230 req := testServer.WaitRequest()
231 c.Assert(req.Method, Equals, "GET")
232 c.Assert(req.URL.Path, Equals, "/sample/multi")
233
234 // Send empty part.
235 req = testServer.WaitRequest()
236 c.Assert(req.Method, Equals, "PUT")
237 c.Assert(req.URL.Path, Equals, "/sample/multi")
238 c.Assert(req.Form["partNumber"], DeepEquals, []string{"1"})
239 c.Assert(req.Header["Content-Length"], DeepEquals, []string{"0"})
240 c.Assert(readAll(req.Body), Equals, "")
241}
242
243func (s *S) TestPutAllResume(c *C) {
244 etag2 := map[string]string{"ETag": `"etag2"`}
58 testServer.Response(200, nil, InitMultiResultDump)245 testServer.Response(200, nil, InitMultiResultDump)
59 testServer.Response(200, nil, ListPartsResultDump1)246 testServer.Response(200, nil, ListPartsResultDump1)
60 testServer.Response(404, nil, NoSuchUploadErrorDump) // :-(
61 testServer.Response(200, nil, ListPartsResultDump2)247 testServer.Response(200, nil, ListPartsResultDump2)
248 testServer.Response(200, etag2, "")
62249
63 b := s.s3.Bucket("sample")250 b := s.s3.Bucket("sample")
64251
65 multi, err := b.InitMulti("multi", "text/plain", s3.Private)252 multi, err := b.InitMulti("multi", "text/plain", s3.Private)
66 c.Assert(err, IsNil)253 c.Assert(err, IsNil)
67254
68 parts, err := multi.ListParts()255 // "part1" and "part3" match the checksums in ResultDump1.
69 c.Assert(err, IsNil)256 // The middle one is a mismatch (it refers to "part2").
257 parts, err := multi.PutAll(strings.NewReader("part1partXpart3"), 5)
70 c.Assert(parts, HasLen, 3)258 c.Assert(parts, HasLen, 3)
71 c.Assert(parts[0].N, Equals, 1)259 c.Assert(parts[0].N, Equals, 1)
72 c.Assert(parts[0].Size, Equals, int64(8))260 c.Assert(parts[0].Size, Equals, int64(5))
73 c.Assert(parts[0].ETag, Equals, `"26f90efd10d614f100252ff56d88dad8"`)261 c.Assert(parts[0].ETag, Equals, `"ffc88b4ca90a355f8ddba6b2c3b2af5c"`)
74 c.Assert(parts[1].N, Equals, 2)262 c.Assert(parts[1].N, Equals, 2)
75 c.Assert(parts[1].Size, Equals, int64(8))263 c.Assert(parts[1].Size, Equals, int64(5))
76 c.Assert(parts[1].ETag, Equals, `"b572ef59bfa4a719fdf2b3c13c583af8"`)264 c.Assert(parts[1].ETag, Equals, `"etag2"`)
77 c.Assert(parts[2].N, Equals, 3)265 c.Assert(parts[2].N, Equals, 3)
78 c.Assert(parts[2].Size, Equals, int64(4))266 c.Assert(parts[2].Size, Equals, int64(5))
79 c.Assert(parts[2].ETag, Equals, `"50dbe110509c7952ba70a96587bd7c40"`)267 c.Assert(parts[2].ETag, Equals, `"49dcd91231f801159e893fb5c6674985"`)
268 c.Assert(err, IsNil)
80269
270 // Init
81 testServer.WaitRequest()271 testServer.WaitRequest()
272
273 // List old parts, broken in two requests.
274 for i := 0; i < 2; i++ {
275 req := testServer.WaitRequest()
276 c.Assert(req.Method, Equals, "GET")
277 c.Assert(req.URL.Path, Equals, "/sample/multi")
278 }
279
280 // Send part 2, as it didn't match the checksum.
82 req := testServer.WaitRequest()281 req := testServer.WaitRequest()
83 c.Assert(req.Method, Equals, "GET")282 c.Assert(req.Method, Equals, "PUT")
84 c.Assert(req.URL.Path, Equals, "/sample/multi")283 c.Assert(req.URL.Path, Equals, "/sample/multi")
85 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")284 c.Assert(req.Form["partNumber"], DeepEquals, []string{"2"})
86 c.Assert(req.Form["max-parts"], DeepEquals, []string{"1000"})285 c.Assert(req.Header["Content-Length"], DeepEquals, []string{"5"})
87286 c.Assert(readAll(req.Body), Equals, "partX")
88 testServer.WaitRequest() // The internal error.
89 req = testServer.WaitRequest()
90 c.Assert(req.Method, Equals, "GET")
91 c.Assert(req.URL.Path, Equals, "/sample/multi")
92 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")
93 c.Assert(req.Form["max-parts"], DeepEquals, []string{"1000"})
94 c.Assert(req.Form["part-number-marker"], DeepEquals, []string{"2"})
95}287}
96288
97func (s *S) TestMultiComplete(c *C) {289func (s *S) TestMultiComplete(c *C) {
@@ -115,11 +307,11 @@
115 c.Assert(req.URL.Path, Equals, "/sample/multi")307 c.Assert(req.URL.Path, Equals, "/sample/multi")
116 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")308 c.Assert(req.Form.Get("uploadId"), Matches, "JNbR_[A-Za-z0-9.]+QQ--")
117309
118 var payload struct{310 var payload struct {
119 XMLName xml.Name311 XMLName xml.Name
120 Part []struct{312 Part []struct {
121 PartNumber int313 PartNumber int
122 ETag string314 ETag string
123 }315 }
124 }316 }
125317
126318
=== modified file 's3/responses_test.go'
--- s3/responses_test.go 2013-02-01 05:58:42 +0000
+++ s3/responses_test.go 2013-02-13 12:22:21 +0000
@@ -87,14 +87,14 @@
87 <Part>87 <Part>
88 <PartNumber>1</PartNumber>88 <PartNumber>1</PartNumber>
89 <LastModified>2013-01-30T13:45:51.000Z</LastModified>89 <LastModified>2013-01-30T13:45:51.000Z</LastModified>
90 <ETag>&quot;26f90efd10d614f100252ff56d88dad8&quot;</ETag>90 <ETag>&quot;ffc88b4ca90a355f8ddba6b2c3b2af5c&quot;</ETag>
91 <Size>8</Size>91 <Size>5</Size>
92 </Part>92 </Part>
93 <Part>93 <Part>
94 <PartNumber>2</PartNumber>94 <PartNumber>2</PartNumber>
95 <LastModified>2013-01-30T13:45:52.000Z</LastModified>95 <LastModified>2013-01-30T13:45:52.000Z</LastModified>
96 <ETag>&quot;b572ef59bfa4a719fdf2b3c13c583af8&quot;</ETag>96 <ETag>&quot;d067a0fa9dc61a6e7195ca99696b5a89&quot;</ETag>
97 <Size>8</Size>97 <Size>5</Size>
98 </Part>98 </Part>
99</ListPartsResult>99</ListPartsResult>
100`100`
@@ -121,8 +121,8 @@
121 <Part>121 <Part>
122 <PartNumber>3</PartNumber>122 <PartNumber>3</PartNumber>
123 <LastModified>2013-01-30T13:46:50.000Z</LastModified>123 <LastModified>2013-01-30T13:46:50.000Z</LastModified>
124 <ETag>&quot;50dbe110509c7952ba70a96587bd7c40&quot;</ETag>124 <ETag>&quot;49dcd91231f801159e893fb5c6674985&quot;</ETag>
125 <Size>4</Size>125 <Size>5</Size>
126 </Part>126 </Part>
127</ListPartsResult>127</ListPartsResult>
128`128`
129129
=== modified file 's3/s3_test.go'
--- s3/s3_test.go 2013-02-01 20:20:47 +0000
+++ s3/s3_test.go 2013-02-13 12:22:21 +0000
@@ -29,6 +29,13 @@
29 testServer.Start()29 testServer.Start()
30 auth := aws.Auth{"abc", "123"}30 auth := aws.Auth{"abc", "123"}
31 s.s3 = s3.New(auth, aws.Region{Name: "faux-region-1", S3Endpoint: testServer.URL})31 s.s3 = s3.New(auth, aws.Region{Name: "faux-region-1", S3Endpoint: testServer.URL})
32}
33
34func (s *S) TearDownSuite(c *C) {
35 s3.SetAttemptStrategy(nil)
36}
37
38func (s *S) SetUpTest(c *C) {
32 attempts := aws.AttemptStrategy{39 attempts := aws.AttemptStrategy{
33 Total: 300 * time.Millisecond,40 Total: 300 * time.Millisecond,
34 Delay: 100 * time.Millisecond,41 Delay: 100 * time.Millisecond,
@@ -36,14 +43,14 @@
36 s3.SetAttemptStrategy(&attempts)43 s3.SetAttemptStrategy(&attempts)
37}44}
3845
39func (s *S) TearDownSuite(c *C) {
40 s3.SetAttemptStrategy(nil)
41}
42
43func (s *S) TearDownTest(c *C) {46func (s *S) TearDownTest(c *C) {
44 testServer.Flush()47 testServer.Flush()
45}48}
4649
50func (s *S) DisableRetries() {
51 s3.SetAttemptStrategy(&aws.AttemptStrategy{})
52}
53
47// PutBucket docs: http://goo.gl/kBTCu54// PutBucket docs: http://goo.gl/kBTCu
4855
49func (s *S) TestPutBucket(c *C) {56func (s *S) TestPutBucket(c *C) {
5057
=== modified file 's3/s3i_test.go'
--- s3/s3i_test.go 2013-02-11 21:32:40 +0000
+++ s3/s3i_test.go 2013-02-13 12:22:21 +0000
@@ -102,7 +102,7 @@
102}102}
103103
104var attempts = aws.AttemptStrategy{104var attempts = aws.AttemptStrategy{
105 Min: 5,105 Min: 5,
106 Total: 20 * time.Second,106 Total: 20 * time.Second,
107 Delay: 100 * time.Millisecond,107 Delay: 100 * time.Millisecond,
108}108}
@@ -567,3 +567,24 @@
567 c.Assert(multis[1].Key, Equals, "a/multi3")567 c.Assert(multis[1].Key, Equals, "a/multi3")
568 c.Assert(multis[1].UploadId, Matches, ".+")568 c.Assert(multis[1].UploadId, Matches, ".+")
569}569}
570
571func (s *ClientTests) TestMultiPutAllZeroLength(c *C) {
572 b := testBucket(s.s3)
573 err := b.PutBucket(s3.Private)
574 c.Assert(err, IsNil)
575
576 multi, err := b.InitMulti("multi", "text/plain", s3.Private)
577 c.Assert(err, IsNil)
578 defer multi.Abort()
579
580 // This tests an edge case. Amazon requires at least one
581 // part for multiprat uploads to work, even the part is empty.
582 parts, err := multi.PutAll(strings.NewReader(""), 5*1024*1024)
583 c.Assert(err, IsNil)
584 c.Assert(parts, HasLen, 1)
585 c.Assert(parts[0].Size, Equals, int64(0))
586 c.Assert(parts[0].ETag, Equals, `"d41d8cd98f00b204e9800998ecf8427e"`)
587
588 err = multi.Complete(parts)
589 c.Assert(err, IsNil)
590}

Subscribers

People subscribed via source and target branches