added error, when size > MaxObjectSize; additional case in algorithm + tests; go fmt

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)
+	optimalPartSize, err := store.CalcOptimalPartSize(size)
-	if optimalPartSize == 0 {
+	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:
+	case size > store.MaxObjectSize:
-		return 0
+		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:
+	case size <= store.MinPartSize:
-		return store.MinPartSize
+		return store.MinPartSize, nil
 	// When we need 9999 parts or less with MinPartSize.
-	case *size/store.MinPartSize < store.MaxMultipartParts:
+	case size/store.MinPartSize < store.MaxMultipartParts:
-		return store.MinPartSize
+		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:
+	case size%store.MaxMultipartParts == 0:
-		return *size / store.MaxMultipartParts
+		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
 	}
 }

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)
+		optimalPartSize, calcError := store.CalcOptimalPartSize(size)
-		if size > MaxObjectSize && optimalPartSize != 0 {
+		if size > MaxObjectSize && calcError == nil {
-			err += fmt.Sprintf("Testcase #%v: size=%v exceeds MaxObjectSize=%v but optimalPartSize is not 0\n", index, size, MaxObjectSize)
+			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 {