Merge pull request #1803 from versity/sis/list-mp-delimiter

feat: adds delimiter support in ListMultipartUploads

tests/integration/ListMultipartUploads.go

@@ -17,9 +17,13 @@ package integration
 import (
 	"context"
 	"fmt"
+	"sort"
+	"strings"
+	"time"
 
 	"github.com/aws/aws-sdk-go-v2/service/s3"
 	"github.com/aws/aws-sdk-go-v2/service/s3/types"
+	"github.com/google/uuid"
 	"github.com/versity/versitygw/s3err"
 )
 
@@ -174,34 +178,6 @@ func ListMultipartUploads_exceeding_max_uploads(s *S3Conf) error {
 	})
 }
 
-func ListMultipartUploads_incorrect_next_key_marker(s *S3Conf) error {
-	testName := "ListMultipartUploads_incorrect_next_key_marker"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		for i := 1; i < 6; i++ {
-			_, err := createMp(s3client, bucket, fmt.Sprintf("obj%v", i))
-			if err != nil {
-				return err
-			}
-		}
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
-			Bucket:    &bucket,
-			KeyMarker: getPtr("wrong_object_key"),
-		})
-		cancel()
-		if err != nil {
-			return err
-		}
-
-		if len(out.Uploads) != 0 {
-			return fmt.Errorf("expected empty list of multipart uploads, instead got %v",
-				out.Uploads)
-		}
-
-		return nil
-	})
-}
-
 func ListMultipartUploads_ignore_upload_id_marker(s *S3Conf) error {
 	testName := "ListMultipartUploads_ignore_upload_id_marker"
 	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
@@ -226,11 +202,378 @@ func ListMultipartUploads_ignore_upload_id_marker(s *S3Conf) error {
 		if err != nil {
 			return err
 		}
-		if ok := compareMultipartUploads(out.Uploads, uploads); !ok {
+		if !compareMultipartUploads(out.Uploads, uploads) {
 			return fmt.Errorf("expected multipart uploads to be %v, instead got %v",
 				uploads, out.Uploads)
 		}
 
+		// should ignore invalid uploaId marker
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out, err = s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			UploadIdMarker: getPtr("invalid_uploadId_marker"),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+		if !compareMultipartUploads(out.Uploads, uploads) {
+			return fmt.Errorf("expected multipart uploads to be %v, instead got %v",
+				uploads, out.Uploads)
+		}
+
+		return nil
+	})
+}
+
+func ListMultipartUploads_invalid_uploadId_marker(s *S3Conf) error {
+	testName := "ListMultipartUploads_invalid_uploadId_marker"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		uploads := make([]types.MultipartUpload, 0, 5)
+		for i := range 5 {
+			out, err := createMp(s3client, bucket, fmt.Sprintf("obj-%v", i))
+			if err != nil {
+				return err
+			}
+
+			uploads = append(uploads, types.MultipartUpload{
+				UploadId:     out.UploadId,
+				Key:          out.Key,
+				StorageClass: types.StorageClassStandard,
+			})
+		}
+
+		// invalid UUID
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		_, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			KeyMarker:      getPtr("obj-2"),
+			UploadIdMarker: getPtr("invalid_uploadId_marker"),
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidUploadIdMarker)); err != nil {
+			return err
+		}
+
+		// valid UUID, but not from the list
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			KeyMarker:      getPtr("obj-2"),
+			UploadIdMarker: getPtr(uuid.New().String()),
+		})
+		cancel()
+		if err := checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidUploadIdMarker)); err != nil {
+			return err
+		}
+
+		// uploadId marker and key marker mismatch
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		_, err = s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			KeyMarker:      getPtr("obj-2"),
+			UploadIdMarker: uploads[4].UploadId,
+		})
+		cancel()
+		return checkApiErr(err, s3err.GetAPIError(s3err.ErrInvalidUploadIdMarker))
+	})
+}
+
+func ListMultipartUploads_keyMarker_not_from_list(s *S3Conf) error {
+	testName := "ListMultipartUploads_keyMarker_not_from_list"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		uploads := make([]types.MultipartUpload, 0, 9)
+		for _, mp := range []struct {
+			key   string
+			count int
+		}{
+			{"bar", 3},
+			{"baz", 4},
+			{"foo", 2},
+		} {
+			for range mp.count {
+				out, err := createMp(s3client, bucket, mp.key)
+				if err != nil {
+					return err
+				}
+				uploads = append(uploads, types.MultipartUpload{
+					Key:          out.Key,
+					UploadId:     out.UploadId,
+					StorageClass: types.StorageClassStandard,
+				})
+				if s.azureTests {
+					// add an artificial delay for azure tests
+					// as azure uploads all these mps with the same
+					// identical creation time
+					time.Sleep(time.Second)
+				}
+			}
+		}
+
+		// without uploadId marker
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:    &bucket,
+			KeyMarker: getPtr("bat"),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if !compareMultipartUploads(uploads[3:], out.Uploads) {
+			return fmt.Errorf("expected the mp list to be %v, instead got %v", uploads[:3], out.Uploads)
+		}
+
+		// should start the listing after the specified uploadId marker
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out, err = s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			KeyMarker:      getPtr("bat"),
+			UploadIdMarker: uploads[4].UploadId,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if !compareMultipartUploads(uploads[5:], out.Uploads) {
+			return fmt.Errorf("expected the mp list to be %v, instead got %v", uploads[5:], out.Uploads)
+		}
+
+		return nil
+	})
+}
+
+func ListMultipartUploads_delimiter_truncated(s *S3Conf) error {
+	testName := "ListMultipartUploads_delimiter_truncated"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		uploads := make([]types.MultipartUpload, 0, 6)
+		for _, key := range []string{
+			"abc/something",
+			"foo/bar/baz",
+			"foo/quxx",
+			"xyz/hello",
+			"zzz/bca",
+			"some/very/nested/mp/object",
+		} {
+			out, err := createMp(s3client, bucket, key)
+			if err != nil {
+				return err
+			}
+			uploads = append(uploads, types.MultipartUpload{
+				Key:          out.Key,
+				UploadId:     out.UploadId,
+				StorageClass: types.StorageClassStandard,
+			})
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:     &bucket,
+			Delimiter:  getPtr("/"),
+			MaxUploads: getPtr(int32(2)),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if len(out.Uploads) != 0 {
+			return fmt.Errorf("expected empty uplodas list, instead got %v", out.Uploads)
+		}
+		expectedCps := []string{"abc/", "foo/"}
+		if !comparePrefixes(expectedCps, out.CommonPrefixes) {
+			return fmt.Errorf("expected the common prefixes to be %v, instead got %v", expectedCps, out.CommonPrefixes)
+		}
+		if getString(out.NextKeyMarker) != "foo/" {
+			return fmt.Errorf("expected the next key marker to be 'foo/', instead got %s", getString(out.NextKeyMarker))
+		}
+		if getString(out.NextUploadIdMarker) != getString(uploads[1].UploadId) {
+			return fmt.Errorf("expected the next upload id marker to be %s, instead got %s", getString(uploads[1].UploadId), getString(out.NextUploadIdMarker))
+		}
+		if !*out.IsTruncated {
+			return fmt.Errorf("expected a truncated response")
+		}
+
+		ctx, cancel = context.WithTimeout(context.Background(), shortTimeout)
+		out2, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:         &bucket,
+			Delimiter:      getPtr("/"),
+			UploadIdMarker: out.NextUploadIdMarker,
+			KeyMarker:      out.NextKeyMarker,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if len(out2.Uploads) != 0 {
+			return fmt.Errorf("expected empty uplodas list, instead got %v", out2.Uploads)
+		}
+		expectedCps = []string{"foo/", "some/", "xyz/", "zzz/"}
+		if !comparePrefixes(expectedCps, out2.CommonPrefixes) {
+			return fmt.Errorf("expected the common prefixes to be %v, instead got %v", expectedCps, out2.CommonPrefixes)
+		}
+		if getString(out2.KeyMarker) != "foo/" {
+			return fmt.Errorf("expected key marker to be 'foo/', instead got %s", getString(out2.KeyMarker))
+		}
+		if getString(out2.UploadIdMarker) != getString(uploads[1].UploadId) {
+			return fmt.Errorf("expected the upload id marker to be %s, instead got %s", getString(uploads[1].UploadId), getString(out2.UploadIdMarker))
+		}
+		if getString(out2.NextKeyMarker) != "" {
+			return fmt.Errorf("expected empty next key marker, instead got %s", getString(out2.NextKeyMarker))
+		}
+		if getString(out2.NextUploadIdMarker) != "" {
+			return fmt.Errorf("expected empty next upload id marker, instead got %s", getString(out2.NextUploadIdMarker))
+		}
+		if *out2.IsTruncated {
+			return fmt.Errorf("expected a non-truncated response")
+		}
+
+		return nil
+	})
+}
+
+func ListMultipartUploads_prefix(s *S3Conf) error {
+	testName := "ListMultipartUploads_prefix"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		prefix := "foo"
+		uploads := make([]types.MultipartUpload, 0, 8)
+		for _, key := range []string{
+			"abc/something",
+			"foo/bar/baz",
+			"foo/quxx",
+			"hello/world",
+			"xyz/hello",
+			"zzz/bca",
+			"some/very/nested/mp/object",
+			"foo/xyz",
+		} {
+			out, err := createMp(s3client, bucket, key)
+			if err != nil {
+				return err
+			}
+
+			if strings.HasPrefix(key, prefix) {
+				uploads = append(uploads, types.MultipartUpload{
+					Key:          out.Key,
+					UploadId:     out.UploadId,
+					StorageClass: types.StorageClassStandard,
+				})
+			}
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket: &bucket,
+			Prefix: &prefix,
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		if getString(out.Prefix) != prefix {
+			return fmt.Errorf("expected the prefix to be %s, instead got %s", prefix, getString(out.Prefix))
+		}
+		if !compareMultipartUploads(out.Uploads, uploads) {
+			return fmt.Errorf("expected the uploads list to be %v, instead got %v", uploads, out.Uploads)
+		}
+
+		return nil
+	})
+}
+
+func ListMultipartUploads_both_delimiter_and_prefix(s *S3Conf) error {
+	testName := "ListMultipartUploads_both_delimiter_and_prefix"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		for _, key := range []string{
+			"foo/abc/bbb/aaa/c",
+			"abc/something",
+			"foo/bar/baz",
+			"foo/quxx",
+			"hello/world",
+			"foo/random/object",
+			"foo/random/another/object",
+			"xyz/hello",
+			"zzz/bca",
+			"some/very/nested/mp/object",
+			"foo/xyz",
+		} {
+			_, err := createMp(s3client, bucket, key)
+			if err != nil {
+				return err
+			}
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:    &bucket,
+			Delimiter: getPtr("/"),
+			Prefix:    getPtr("foo/"),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		expectedCps := []string{"foo/abc/", "foo/bar/", "foo/random/"}
+		if !comparePrefixes(expectedCps, out.CommonPrefixes) {
+			return fmt.Errorf("expected the common prefixes to be %v, instead got %v", expectedCps, out.CommonPrefixes)
+		}
+
+		return nil
+	})
+}
+
+func ListMultipartUploads_delimiter_no_matches(s *S3Conf) error {
+	testName := "ListMultipartUploads_delimiter_no_matches"
+	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
+		uploads := make([]types.MultipartUpload, 0, 8)
+		for _, key := range []string{
+			"abc/something",
+			"foo/bar/baz",
+			"foo/quxx",
+			"hello/world",
+			"xyz/hello",
+			"zzz/bca",
+			"some/very/nested/mp/object",
+			"foo/xyz",
+		} {
+			out, err := createMp(s3client, bucket, key)
+			if err != nil {
+				return err
+			}
+
+			uploads = append(uploads, types.MultipartUpload{
+				Key:          out.Key,
+				UploadId:     out.UploadId,
+				StorageClass: types.StorageClassStandard,
+			})
+		}
+
+		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
+		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
+			Bucket:    &bucket,
+			Delimiter: getPtr("delim"),
+		})
+		cancel()
+		if err != nil {
+			return err
+		}
+
+		sort.SliceStable(uploads, func(i, j int) bool {
+			return *uploads[i].Key < *uploads[j].Key
+		})
+
+		if !compareMultipartUploads(uploads, out.Uploads) {
+			return fmt.Errorf("expected the uploads to be %v, instead got %v", uploads, out.Uploads)
+		}
+		if len(out.CommonPrefixes) != 0 {
+			return fmt.Errorf("expected empty common prefixes, instead got %v", out.CommonPrefixes)
+		}
+
 		return nil
 	})
 }
@@ -302,52 +645,3 @@ func ListMultipartUploads_with_checksums(s *S3Conf) error {
 		return nil
 	})
 }
-
-func ListMultipartUploads_success(s *S3Conf) error {
-	testName := "ListMultipartUploads_success"
-	return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
-		obj1, obj2 := "my-obj-1", "my-obj-2"
-		out1, err := createMp(s3client, bucket, obj1)
-		if err != nil {
-			return err
-		}
-
-		out2, err := createMp(s3client, bucket, obj2)
-		if err != nil {
-			return err
-		}
-
-		ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
-		out, err := s3client.ListMultipartUploads(ctx, &s3.ListMultipartUploadsInput{
-			Bucket: &bucket,
-		})
-		cancel()
-		if err != nil {
-			return err
-		}
-
-		expected := []types.MultipartUpload{
-			{
-				Key:          &obj1,
-				UploadId:     out1.UploadId,
-				StorageClass: types.StorageClassStandard,
-			},
-			{
-				Key:          &obj2,
-				UploadId:     out2.UploadId,
-				StorageClass: types.StorageClassStandard,
-			},
-		}
-
-		if len(out.Uploads) != 2 {
-			return fmt.Errorf("expected 2 upload, instead got %v",
-				len(out.Uploads))
-		}
-		if ok := compareMultipartUploads(out.Uploads, expected); !ok {
-			return fmt.Errorf("expected uploads %v, instead got %v",
-				expected, out.Uploads)
-		}
-
-		return nil
-	})
-}

tests/integration/group-tests.go

@@ -459,13 +459,17 @@ func TestListMultipartUploads(ts *TestState) {
 	ts.Run(ListMultipartUploads_invalid_max_uploads)
 	ts.Run(ListMultipartUploads_max_uploads)
 	ts.Run(ListMultipartUploads_exceeding_max_uploads)
-	ts.Run(ListMultipartUploads_incorrect_next_key_marker)
 	ts.Run(ListMultipartUploads_ignore_upload_id_marker)
+	ts.Run(ListMultipartUploads_invalid_uploadId_marker)
+	ts.Run(ListMultipartUploads_keyMarker_not_from_list)
+	ts.Run(ListMultipartUploads_delimiter_truncated)
+	ts.Run(ListMultipartUploads_prefix)
+	ts.Run(ListMultipartUploads_both_delimiter_and_prefix)
+	ts.Run(ListMultipartUploads_delimiter_no_matches)
 	//TODO: remove the condition after implementing checksums in azure
 	if !ts.conf.azureTests {
 		ts.Run(ListMultipartUploads_with_checksums)
 	}
-	ts.Run(ListMultipartUploads_success)
 }
 
 func TestAbortMultipartUpload(ts *TestState) {
@@ -1473,10 +1477,13 @@ func GetIntTests() IntTests {
 		"ListMultipartUploads_invalid_max_uploads":                                 ListMultipartUploads_invalid_max_uploads,
 		"ListMultipartUploads_max_uploads":                                         ListMultipartUploads_max_uploads,
 		"ListMultipartUploads_exceeding_max_uploads":                               ListMultipartUploads_exceeding_max_uploads,
-		"ListMultipartUploads_incorrect_next_key_marker":                           ListMultipartUploads_incorrect_next_key_marker,
 		"ListMultipartUploads_ignore_upload_id_marker":                             ListMultipartUploads_ignore_upload_id_marker,
+		"ListMultipartUploads_invalid_uploadId_marker":                             ListMultipartUploads_invalid_uploadId_marker,
+		"ListMultipartUploads_keyMarker_not_from_list":                             ListMultipartUploads_keyMarker_not_from_list,
+		"ListMultipartUploads_delimiter_truncated":                                 ListMultipartUploads_delimiter_truncated,
+		"ListMultipartUploads_prefix":                                              ListMultipartUploads_prefix,
+		"ListMultipartUploads_both_delimiter_and_prefix":                           ListMultipartUploads_both_delimiter_and_prefix,
 		"ListMultipartUploads_with_checksums":                                      ListMultipartUploads_with_checksums,
-		"ListMultipartUploads_success":                                             ListMultipartUploads_success,
 		"AbortMultipartUpload_non_existing_bucket":                                 AbortMultipartUpload_non_existing_bucket,
 		"AbortMultipartUpload_incorrect_uploadId":                                  AbortMultipartUpload_incorrect_uploadId,
 		"AbortMultipartUpload_incorrect_object_key":                                AbortMultipartUpload_incorrect_object_key,

tests/integration/utils.go

@@ -838,14 +838,13 @@ func comparePrefixes(list1 []string, list2 []types.CommonPrefix) bool {
 		return false
 	}
 
-	elementMap := make(map[string]bool)
-
-	for _, elem := range list1 {
-		elementMap[elem] = true
-	}
-
-	for _, elem := range list2 {
-		if _, found := elementMap[*elem.Prefix]; !found {
+	for i, prefix := range list1 {
+		if list2[i].Prefix == nil {
+			fmt.Printf("unexpected nil prefix on index %v", i)
+			return false
+		}
+		if *list2[i].Prefix != prefix {
+			fmt.Printf("prefix mismatch on index %v: expected %s, got %v", i, prefix, *list2[i].Prefix)
 			return false
 		}
 	}