Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Fixes ListObjects direcotry objects listing issue. Fixes DeleteO… #1113

Merged
merged 1 commit into from
Mar 4, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion backend/posix/posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -3212,7 +3212,10 @@ func (p *Posix) removeParents(bucket, object string) {
// this with a special attribute to indicate these. stop
// at either the bucket or the first parent we encounter
// with the attribute, whichever comes first.
objPath := object

// Remove the last path separator for the directory objects
// to correctly detect the parent in the loop
objPath := strings.TrimSuffix(object, "/")
for {
parent := filepath.Dir(objPath)
if parent == string(filepath.Separator) || parent == "." {
Expand Down
16 changes: 9 additions & 7 deletions backend/walk.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker strin
// so we can skip a directory without an early return
var skipflag error
if d.IsDir() {
fmt.Println("path: ", path)
// If prefix is defined and the directory does not match prefix,
// do not descend into the directory because nothing will
// match this prefix. Make sure to append the / at the end of
Expand All @@ -108,13 +109,7 @@ func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker strin
strings.Contains(strings.TrimPrefix(path+"/", prefix), delimiter) {
skipflag = fs.SkipDir
} else {
// TODO: can we do better here rather than a second readdir
// per directory?
ents, err := fs.ReadDir(fileSystem, path)
if err != nil {
return fmt.Errorf("readdir %q: %w", path, err)
}
if len(ents) == 0 && delimiter == "" {
if delimiter == "" {
dirobj, err := getObj(path+"/", d)
if err == ErrSkipObj {
return skipflag
Expand All @@ -127,6 +122,13 @@ func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker strin
return skipflag
}

// TODO: can we do better here rather than a second readdir
// per directory?
ents, err := fs.ReadDir(fileSystem, path)
if err != nil {
return fmt.Errorf("readdir %q: %w", path, err)
}

if len(ents) != 0 {
return skipflag
}
Expand Down
4 changes: 4 additions & 0 deletions tests/integration/group-tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,7 @@ func TestListObjects(s *S3Conf) {
ListObjects_max_keys_none(s)
ListObjects_marker_not_from_obj_list(s)
ListObjects_list_all_objs(s)
ListObjects_nested_dir_file_objs(s)
//TODO: remove the condition after implementing checksums in azure
if !s.azureTests {
ListObjects_with_checksum(s)
Expand Down Expand Up @@ -246,6 +247,7 @@ func TestDeleteObject(s *S3Conf) {
DeleteObject_non_existing_object(s)
DeleteObject_directory_object_noslash(s)
DeleteObject_non_existing_dir_object(s)
DeleteObject_directory_object(s)
DeleteObject_success(s)
DeleteObject_success_status_code(s)
}
Expand Down Expand Up @@ -876,6 +878,7 @@ func GetIntTests() IntTests {
"ListObjects_max_keys_none": ListObjects_max_keys_none,
"ListObjects_marker_not_from_obj_list": ListObjects_marker_not_from_obj_list,
"ListObjects_list_all_objs": ListObjects_list_all_objs,
"ListObjects_nested_dir_file_objs": ListObjects_nested_dir_file_objs,
"ListObjects_with_checksum": ListObjects_with_checksum,
"ListObjectsV2_start_after": ListObjectsV2_start_after,
"ListObjectsV2_both_start_after_and_continuation_token": ListObjectsV2_both_start_after_and_continuation_token,
Expand All @@ -892,6 +895,7 @@ func GetIntTests() IntTests {
"DeleteObject_directory_object_noslash": DeleteObject_directory_object_noslash,
"DeleteObject_name_too_long": DeleteObject_name_too_long,
"DeleteObject_non_existing_dir_object": DeleteObject_non_existing_dir_object,
"DeleteObject_directory_object": DeleteObject_directory_object,
"DeleteObject_success": DeleteObject_success,
"DeleteObject_success_status_code": DeleteObject_success_status_code,
"DeleteObjects_empty_input": DeleteObjects_empty_input,
Expand Down
57 changes: 57 additions & 0 deletions tests/integration/tests.go
Original file line number Diff line number Diff line change
Expand Up @@ -4771,6 +4771,44 @@ func ListObjects_list_all_objs(s *S3Conf) error {
})
}

func ListObjects_nested_dir_file_objs(s *S3Conf) error {
testName := "ListObjects_nested_dir_file_objs"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
contents, err := putObjects(s3client, []string{"foo/bar/", "foo/bar/baz", "foo/bar/quxx"}, bucket)
if err != nil {
return err
}

ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
res, err := s3client.ListObjects(ctx, &s3.ListObjectsInput{
Bucket: &bucket,
})
cancel()
if err != nil {
return err
}

if !compareObjects(contents, res.Contents) {
return fmt.Errorf("expected the objects list to be %+v, instead got %+v", contents, res.Contents)
}

// Clean up the nested objects to avoid `ErrDirectoryNotEmpty` error on teardown
for _, obj := range []string{"foo/bar/baz", "foo/bar/quxx"} {
ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err := s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
if err != nil {
return err
}
}

return nil
})
}

func ListObjectsV2_start_after(s *S3Conf) error {
testName := "ListObjectsV2_start_after"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
Expand Down Expand Up @@ -5391,6 +5429,25 @@ func DeleteObject_non_existing_dir_object(s *S3Conf) error {
})
}

func DeleteObject_directory_object(s *S3Conf) error {
testName := "DeleteObject_directory_object"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
obj := "foo/bar/"
_, err := putObjects(s3client, []string{obj}, bucket)
if err != nil {
return err
}

ctx, cancel := context.WithTimeout(context.Background(), shortTimeout)
_, err = s3client.DeleteObject(ctx, &s3.DeleteObjectInput{
Bucket: &bucket,
Key: &obj,
})
cancel()
return err
})
}

func DeleteObject_success(s *S3Conf) error {
testName := "DeleteObject_success"
return actionHandler(s, testName, func(s3client *s3.Client, bucket string) error {
Expand Down
Loading