From e2e3b9ffe4aeef70ed44bc63b6afc5e4c353c159 Mon Sep 17 00:00:00 2001 From: Markus Kienast Date: Wed, 23 Aug 2017 13:28:59 +0200 Subject: [PATCH] added error, when size > MaxObjectSize; additional case in algorithm + tests; go fmt --- s3store/s3store.go | 29 ++++++++++++++++------------- s3store/s3store_test.go | 25 +++++++++++++++---------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/s3store/s3store.go b/s3store/s3store.go index 28e6a08..86ac863 100644 --- a/s3store/s3store.go +++ b/s3store/s3store.go @@ -86,7 +86,6 @@ import ( "fmt" "io" "io/ioutil" - "math" "os" "regexp" "strings" @@ -234,8 +233,8 @@ func (store S3Store) WriteChunk(id string, offset int64, src io.Reader) (int64, size := info.Size bytesUploaded := int64(0) - optimalPartSize := store.CalcOptimalPartSize(&size) - if optimalPartSize == 0 { + optimalPartSize, err := store.CalcOptimalPartSize(size) + if err != nil { return bytesUploaded, nil } @@ -562,27 +561,31 @@ func isAwsError(err error, code string) bool { return false } -func (store S3Store) CalcOptimalPartSize(size *int64) int64 { +func (store S3Store) CalcOptimalPartSize(size int64) (int64, error) { switch { // We can only manage files up to MaxObjectSize, else we need to fail. - case *size > store.MaxObjectSize: - return 0 + case size > store.MaxObjectSize: + return 0, fmt.Errorf("CalcOptimalPartSize: size of %v bytes exceeds MaxObjectSize of %v bytes", size, store.MaxObjectSize) // When upload is smaller or equal MinPartSize, we upload in just one part. - case *size <= store.MinPartSize: - return store.MinPartSize + case size <= store.MinPartSize: + return store.MinPartSize, nil // When we need 9999 parts or less with MinPartSize. - case *size/store.MinPartSize < store.MaxMultipartParts: - return store.MinPartSize + case size/store.MinPartSize < store.MaxMultipartParts: + return store.MinPartSize, nil // When we can divide our upload into exactly MaxMultipartParts parts with // no bytes leftover, we will not need an spare last part. // Also, when MaxObjectSize is equal to MaxPartSize * MaxMultipartParts // (which is not the case with the current AWS S3 API specification, but // might be in the future or with other S3-aware stores), we need this in // order for our Multipart-Upload to reach full MaxObjectSize. - case int64(math.Mod(float64(*size), float64(store.MaxMultipartParts))) == 0: - return *size / store.MaxMultipartParts + case size%store.MaxMultipartParts == 0: + return size / store.MaxMultipartParts, nil + // If the last part would be larger than MaxPartSize, which is only the case + // when we are close to MaxObjectSize, we have to go with MaxPartSize. + case size%(store.MaxMultipartParts-1) > store.MaxPartSize: + return store.MaxPartSize, nil // In all other cases, we need a spare last piece for the remaining bytes. default: - return *size / (store.MaxMultipartParts - 1) + return size / (store.MaxMultipartParts - 1), nil } } diff --git a/s3store/s3store_test.go b/s3store/s3store_test.go index 1ae264b..c640984 100644 --- a/s3store/s3store_test.go +++ b/s3store/s3store_test.go @@ -4,7 +4,6 @@ import ( "bytes" "fmt" "io/ioutil" - "math" "testing" "github.com/golang/mock/gomock" @@ -49,9 +48,9 @@ func TestCalcOptimalPartSize(t *testing.T) { var LimitedMaxPartSize = MaxObjectSize / (MaxMultipartParts - 1) // the size of the last part, when upload has MaxObjectSize and we use // LimitedMaxPartSize for uploading - var LastPartSize int64 = int64(math.Mod(float64(MaxObjectSize), float64(MaxMultipartParts-1))) + var LastPartSize int64 = MaxObjectSize % (MaxMultipartParts - 1) - var optimalPartSize, equalparts, lastpartsize int64 + var equalparts, lastpartsize int64 var err string // some of these tests are actually duplicates, as they specify the same size @@ -85,9 +84,9 @@ func TestCalcOptimalPartSize(t *testing.T) { } for index, size := range testcases { - optimalPartSize = store.CalcOptimalPartSize(&size) - if size > MaxObjectSize && optimalPartSize != 0 { - err += fmt.Sprintf("Testcase #%v: size=%v exceeds MaxObjectSize=%v but optimalPartSize is not 0\n", index, size, MaxObjectSize) + optimalPartSize, calcError := store.CalcOptimalPartSize(size) + if size > MaxObjectSize && calcError == nil { + err += fmt.Sprintf("Testcase #%v: size=%v exceeds MaxObjectSize=%v but CalcOptimalPartSize did not throw an error\n", index, size, MaxObjectSize) } if optimalPartSize*(MaxMultipartParts-1) > MaxObjectSize { err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v, exceeds MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) @@ -95,17 +94,23 @@ func TestCalcOptimalPartSize(t *testing.T) { if optimalPartSize > MaxPartSize { err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v exceeds MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) } - if optimalPartSize > 0 { + if calcError == nil { equalparts = size / optimalPartSize - lastpartsize = int64(math.Mod(float64(size), float64(optimalPartSize))) + lastpartsize = size % optimalPartSize if optimalPartSize < MinPartSize { err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v is below MinPartSize=%v\n", index, optimalPartSize, MinPartSize) } + if optimalPartSize > MaxPartSize { + err += fmt.Sprintf("Testcase #%v: optimalPartSize=%v is larger than MaxPartSize=%v\n", index, optimalPartSize, MaxPartSize) + } + if lastpartsize > MaxPartSize { + err += fmt.Sprintf("Testcase #%v: size of last part %v is exceeding MaxPartSize limit of %v\n", index, lastpartsize, MaxPartSize) + } if equalparts > 10000 { - err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts\n", index, equalparts) + err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts. %v equal parts of size %v, size of last part %v\n", index, equalparts, equalparts, optimalPartSize, lastpartsize) } if equalparts == 10000 && lastpartsize > 0 { - err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts\n", index, equalparts+1) + err += fmt.Sprintf("Testcase #%v: max-parts=%v exceeds limit of 10.000 parts. %v equal parts of size %v, size of last part %v\n", index, equalparts+1, equalparts, optimalPartSize, lastpartsize) } } if len(err) > 0 {