separated partsize calc and error handling

s3store/s3store.go

@@ -235,7 +235,7 @@ func (store S3Store) WriteChunk(id string, offset int64, src io.Reader) (int64,
 	bytesUploaded := int64(0)
 	optimalPartSize, err := store.CalcOptimalPartSize(size)
 	if err != nil {
-		return bytesUploaded, nil
+		return bytesUploaded, err
 	}
 
 	// Get number of parts to generate next number
@@ -561,20 +561,16 @@ func isAwsError(err error, code string) bool {
 	return false
 }
 
-func (store S3Store) CalcOptimalPartSize(size int64) (int64, error) {
+func (store S3Store) CalcOptimalPartSize(size int64) (optimalPartSize int64, err error) {
+	var errorstrings []string
+
 	switch {
-	// We can only manage files up to MaxObjectSize, else we need to fail.
-	case size > store.MaxObjectSize:
-		fmt.Print("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:
-		fmt.Print("A ")
+		optimalPartSize = store.MinPartSize
-		return store.MinPartSize, nil
 	// When we need 9999 parts or less with MinPartSize.
 	case size/store.MinPartSize < store.MaxMultipartParts:
-		fmt.Print("B ")
+		optimalPartSize = store.MinPartSize
-		return store.MinPartSize, nil
 	// If our upload divides up exactly into MaxMultipartParts parts with
 	// no bytes leftover, we will not need an spare last part. So we can go with
 	// a straight division. Otherwise we would exceed MaxPartSize in a scenario,
@@ -582,16 +578,22 @@ func (store S3Store) CalcOptimalPartSize(size int64) (int64, error) {
 	// (which is not the case with the current AWS S3 API specification, but
 	// might be in the future or with other S3-aware stores).
 	case size%store.MaxMultipartParts == 0:
-		fmt.Print("C ")
+		optimalPartSize = size / store.MaxMultipartParts
-		return size / store.MaxMultipartParts, nil
+	// In all other cases, we need to round up to the next integer.
-	// In all other cases, we need to round up to the next integer, as long as we
-	// stay below MaxPartSize.
-	case size/store.MaxMultipartParts < store.MaxPartSize:
-		fmt.Print("D ")
-		return size/store.MaxMultipartParts + 1, nil
-	// If non of the above matches, we have exceeded the MaxPartSize limit above.
 	default:
-		fmt.Print("X ")
+		optimalPartSize = size/store.MaxMultipartParts + 1
-		return size/store.MaxMultipartParts + 1, fmt.Errorf("CalcOptimalPartSize: to upload %v bytes optimalPartSize %v must exceed MaxPartSize %v", size, (size/store.MaxMultipartParts + 1), store.MaxPartSize)
 	}
+
+	// an upload larger than MaxObjectSize must throw an error
+	if size > store.MaxObjectSize {
+		errorstrings = append(errorstrings, fmt.Sprintf("CalcOptimalPartSize: upload size of %v bytes exceeds MaxObjectSize of %v bytes", size, store.MaxObjectSize))
+	}
+	// optimalPartSize must never exceed MaxPartSize
+	if optimalPartSize > store.MaxPartSize {
+		errorstrings = append(errorstrings, fmt.Sprintf("CalcOptimalPartSize: to upload %v bytes optimalPartSize %v must exceed MaxPartSize %v", size, optimalPartSize, store.MaxPartSize))
+	}
+	if len(errorstrings) > 0 {
+		err = fmt.Errorf(strings.Join(errorstrings, "\n"))
+	}
+	return optimalPartSize, err
 }

s3store/s3store_test.go

@@ -36,10 +36,12 @@ func TestCalcOptimalPartSize(t *testing.T) {
 	assert.Equal(s3obj, store.Service)
 
 	// If you quickly want to override the default values in this test
+	/*
 		store.MinPartSize = 2
 		store.MaxPartSize = 10
 		store.MaxMultipartParts = 20
-	store.MaxObjectSize = 201
+		store.MaxObjectSize = 200
+	*/
 
 	var MinPartSize = store.MinPartSize
 	var MaxPartSize = store.MaxPartSize
@@ -58,9 +60,6 @@ func TestCalcOptimalPartSize(t *testing.T) {
 	}
 	var RemainderWithHighestApplicablePartSize int64 = MaxObjectSize % HighestApplicablePartSize
 
-	fmt.Println("HighestApplicablePartSize", HighestApplicablePartSize)
-	fmt.Println("RemainderWithHighestApplicablePartSize", RemainderWithHighestApplicablePartSize)
-
 	// some of these tests are actually duplicates, as they specify the same size
 	// in bytes - two ways to describe the same thing. That is wanted, in order
 	// to provide a full picture from any angle.
@@ -112,19 +111,17 @@ func TestCalcOptimalPartSize(t *testing.T) {
 	}
 
 	for index, size := range testcases {
-		optimalPartSize, calcError := store.CalcOptimalPartSize(size)
-		equalparts, lastpartsize = 0, 0
 		err = ""
+		optimalPartSize, calcError := store.CalcOptimalPartSize(size)
+		equalparts = size / optimalPartSize
+		lastpartsize = size % optimalPartSize
 		if size > MaxObjectSize && calcError == nil {
 			err += fmt.Sprintf("Testcase #%v size %v: size exceeds MaxObjectSize=%v but no error returned\n", index, size, MaxObjectSize)
 		}
-		if calcError == nil {
-			equalparts = size / optimalPartSize
-			lastpartsize = size % optimalPartSize
 		if optimalPartSize < MinPartSize {
 			err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: optimalPartSize < MinPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MinPartSize)
 		}
-			if optimalPartSize > MaxPartSize {
+		if optimalPartSize > MaxPartSize && calcError == nil {
 			err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: optimalPartSize > MaxPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, MaxPartSize)
 		}
 		if size%optimalPartSize == 0 && equalparts > MaxMultipartParts {
@@ -139,12 +136,13 @@ func TestCalcOptimalPartSize(t *testing.T) {
 		if lastpartsize > optimalPartSize {
 			err += fmt.Sprintf("Testcase #%v size %v, %v parts of size %v, lastpart %v: lastpart > optimalPartSize %v\n", index, size, equalparts, optimalPartSize, lastpartsize, optimalPartSize)
 		}
-		}
+		// fmt.Printf("Testcase #%v size %v, %v parts of size %v, lastpart %v\n", index, size, equalparts, optimalPartSize, lastpartsize)
-		fmt.Printf("Testcase #%v size %v, %v parts of size %v, lastpart %v\n", index, size, equalparts, optimalPartSize, lastpartsize)
 		if len(err) > 0 {
 			t.Errorf(err)
 		}
 	}
+	// fmt.Println("HighestApplicablePartSize", HighestApplicablePartSize)
+	// fmt.Println("RemainderWithHighestApplicablePartSize", RemainderWithHighestApplicablePartSize)
 }
 
 func TestNewUpload(t *testing.T) {