From 8951cce6d066508745e6376c274b236947ce3657 Mon Sep 17 00:00:00 2001 From: jonaustin09 Date: Thu, 14 Sep 2023 16:17:29 -0400 Subject: [PATCH] fix: Fixes #179, Fixes #180, Fixes ListObject marker bug --- backend/walk.go | 18 ++++++----- integration/action-tests.go | 1 + integration/tests.go | 63 +++++++++++++++++++++++++++++-------- integration/utils.go | 20 ++++++++++++ 4 files changed, 82 insertions(+), 20 deletions(-) diff --git a/backend/walk.go b/backend/walk.go index ed0aaff..8ad764e 100644 --- a/backend/walk.go +++ b/backend/walk.go @@ -64,8 +64,12 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int32, getObj } if pastMax { - newMarker = path - truncated = true + if max < 1 { + truncated = true + } else { + newMarker = *objects[len(objects)-1].Key + truncated = true + } return fs.SkipAll } @@ -75,8 +79,8 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int32, getObj // match this prefix. Make sure to append the / at the end of // directories since this is implied as a directory path name. // If path is a prefix of prefix, then path could still be - // building to match. So only skip if path isnt a prefix of prefix - // and prefix isnt a prefix of path. + // building to match. So only skip if path isn't a prefix of prefix + // and prefix isn't a prefix of path. if prefix != "" && !strings.HasPrefix(path+string(os.PathSeparator), prefix) && !strings.HasPrefix(prefix, path+string(os.PathSeparator)) { @@ -104,10 +108,10 @@ func Walk(fileSystem fs.FS, prefix, delimiter, marker string, max int32, getObj } if !pastMarker { - if path != marker { - return nil + if path == marker { + pastMarker = true } - pastMarker = true + return nil } // If object doesn't have prefix, don't include in results. diff --git a/integration/action-tests.go b/integration/action-tests.go index 887734a..bb9593b 100644 --- a/integration/action-tests.go +++ b/integration/action-tests.go @@ -72,6 +72,7 @@ func TestListObjects(s *S3Conf) { ListObject_truncated(s) ListObjects_invalid_max_keys(s) ListObjects_max_keys_0(s) + ListObjects_delimiter(s) } func TestDeleteObject(s *S3Conf) { diff --git a/integration/tests.go b/integration/tests.go index 2a25990..d3e7ee7 100644 --- a/integration/tests.go +++ b/integration/tests.go @@ -1347,7 +1347,7 @@ func ListObject_truncated(s *S3Conf) { } ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) - out, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ + out1, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ Bucket: &bucket, MaxKeys: maxKeys, }) @@ -1356,35 +1356,41 @@ func ListObject_truncated(s *S3Conf) { return err } - if !out.IsTruncated { - return fmt.Errorf("expected output to be truncated") + if !out1.IsTruncated { + return fmt.Errorf("expected out1put to be truncated") } - if out.MaxKeys != maxKeys { - return fmt.Errorf("expected max-keys to be %v, instead got %v", maxKeys, out.MaxKeys) + if out1.MaxKeys != maxKeys { + return fmt.Errorf("expected max-keys to be %v, instead got %v", maxKeys, out1.MaxKeys) } - if !compareObjects([]string{"bar", "baz"}, out.Contents) { + if *out1.NextMarker != "baz" { + return fmt.Errorf("expected nex-marker to be baz, instead got %v", *out1.NextMarker) + } + + if !compareObjects([]string{"bar", "baz"}, out1.Contents) { return fmt.Errorf("unexpected output for list objects with max-keys") } - //TODO: Add next marker checker after bug-fixing - ctx, cancel = context.WithTimeout(context.Background(), shortTimeout) - out, err = s3client.ListObjects(ctx, &s3.ListObjectsInput{ + out2, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ Bucket: &bucket, - Marker: out.NextMarker, + Marker: out1.NextMarker, }) cancel() if err != nil { return err } - if out.IsTruncated { + if out2.IsTruncated { return fmt.Errorf("expected output not to be truncated") } - if !compareObjects([]string{"foo"}, out.Contents) { + if *out2.Marker != *out1.NextMarker { + return fmt.Errorf("expected marker to be %v, instead got %v", *out1.NextMarker, *out2.Marker) + } + + if !compareObjects([]string{"foo"}, out2.Contents) { return fmt.Errorf("unexpected output for list objects with max-keys") } return nil @@ -1434,7 +1440,38 @@ func ListObjects_max_keys_0(s *S3Conf) { }) } -//TODO: Add a test case for delimiter after bug-fixing, as delimiter doesn't work as intended +func ListObjects_delimiter(s *S3Conf) { + testName := "ListObjects_delimiter" + actionHandler(s, testName, func(s3client *s3.Client, bucket string) error { + err := putObjects(s3client, []string{"foo/bar/baz", "foo/bar/xyzzy", "quux/thud", "asdf"}, bucket) + if err != nil { + return err + } + + ctx, cancel := context.WithTimeout(context.Background(), shortTimeout) + out, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{ + Bucket: &bucket, + Delimiter: getPtr("/"), + }) + cancel() + if err != nil { + return err + } + + if *out.Delimiter != "/" { + return fmt.Errorf("expected delimiter to be /, instead got %v", *out.Delimiter) + } + if len(out.Contents) != 1 || *out.Contents[0].Key != "asdf" { + return fmt.Errorf("expected result [\"asdf\"], instead got %v", out.Contents) + } + + if !comparePrefixes([]string{"foo/", "quux/"}, out.CommonPrefixes) { + return fmt.Errorf("expected common prefixes to be %v, instead got %v", []string{"foo/", "quux/"}, out.CommonPrefixes) + } + + return nil + }) +} func DeleteObject_non_existing_object(s *S3Conf) { testName := "DeleteObject_non_existing_object" diff --git a/integration/utils.go b/integration/utils.go index da1e8c5..25a658b 100644 --- a/integration/utils.go +++ b/integration/utils.go @@ -401,6 +401,26 @@ func compareObjects(list1 []string, list2 []types.Object) bool { return true } +func comparePrefixes(list1 []string, list2 []types.CommonPrefix) bool { + if len(list1) != len(list2) { + return false + } + + elementMap := make(map[string]bool) + + for _, elem := range list1 { + elementMap[elem] = true + } + + for _, elem := range list2 { + if _, found := elementMap[*elem.Prefix]; !found { + return false + } + } + + return true +} + func compareDelObjects(list1 []string, list2 []types.DeletedObject) bool { if len(list1) != len(list2) { return false