Skip to content
Draft
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
44 changes: 43 additions & 1 deletion pkg/storages/fs/folder.go
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,9 @@ import (
"io"
"os"
"path"
"path/filepath"
"strings"
"syscall"

"github.com/pkg/errors"
"github.com/wal-g/tracelog"
Expand Down Expand Up @@ -54,17 +56,57 @@ func (folder *Folder) ListFolder() (objects []storage.Object, subFolders []stora

func (folder *Folder) DeleteObjects(objectsWithRelativePaths []storage.Object) error {
for _, object := range objectsWithRelativePaths {
err := os.RemoveAll(folder.GetFilePath(object.GetName()))
filePath := folder.GetFilePath(object.GetName())
err := os.RemoveAll(filePath)
if os.IsNotExist(err) {
continue
}
if err != nil {
return fmt.Errorf("unable to delete file %q: %w", object.GetName(), err)
}
if err = folder.cleanupEmptyFolders(filepath.Dir(filePath)); err != nil {
return err
}
}
return nil
}

// cleanupEmptyFolders removes empty parent directories up to the root folder path.
func (folder *Folder) cleanupEmptyFolders(dirPath string) error {
rootPath := filepath.Clean(folder.rootPath)
for {
dirPath = filepath.Clean(dirPath)
rel, err := filepath.Rel(rootPath, dirPath)
if err != nil || rel == "." || strings.HasPrefix(rel, "..") {
break
}
entries, err := os.ReadDir(dirPath)
if err != nil {
if os.IsNotExist(err) {
break
}
return fmt.Errorf("unable to read directory %q: %w", dirPath, err)
}
if len(entries) > 0 {
break
}
if err = os.Remove(dirPath); err != nil {
// Directory became non-empty after ReadDir (race) or doesn't exist — stop quietly.
if os.IsNotExist(err) || isNotEmpty(err) {
break
}
return fmt.Errorf("unable to remove empty directory %q: %w", dirPath, err)
}
Comment on lines +90 to +99
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s a race between os.ReadDir and os.Remove: a directory can become non-empty after the read, causing os.Remove to fail with ENOTEMPTY/EEXIST. Returning an error here can make DeleteObjects fail even though the object deletion succeeded. Treat “directory not empty” removal failures as a normal stop condition (break cleanup) rather than an error, while still erroring on other failure types.

Copilot uses AI. Check for mistakes.
dirPath = filepath.Dir(dirPath)
}
return nil
}

// isNotEmpty reports whether the error from os.Remove indicates the directory is not empty.
func isNotEmpty(err error) bool {
return errors.Is(err, syscall.ENOTEMPTY)
}

func (folder *Folder) Exists(objectRelativePath string) (bool, error) {
_, err := os.Stat(folder.GetFilePath(objectRelativePath))
if os.IsNotExist(err) {
Expand Down
61 changes: 61 additions & 0 deletions pkg/storages/fs/folder_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package fs
import (
"os"
"path/filepath"
"strings"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/wal-g/wal-g/pkg/storages/storage"
Expand All @@ -20,6 +22,65 @@ func TestFSFolder(t *testing.T) {
storage.RunFolderTest(st.RootFolder(), t)
}

func TestDeleteObjectsRemovesEmptyFolders(t *testing.T) {
tmpDir := setupTmpDir(t)
defer os.RemoveAll(tmpDir)

st, err := ConfigureStorage(tmpDir, nil)
assert.NoError(t, err)

root := st.RootFolder()

// Create a nested structure: root/a/b/file
sub := root.GetSubFolder("a/b")
err = sub.PutObject("file", strings.NewReader("data"))
assert.NoError(t, err)

// Verify the directory exists
_, err = os.Stat(filepath.Join(tmpDir, "a", "b"))
assert.NoError(t, err)

// Delete the file
err = sub.DeleteObjects([]storage.Object{storage.NewLocalObject("file", time.Time{}, 0)})
assert.NoError(t, err)

// Both empty subdirectories should have been cleaned up
_, err = os.Stat(filepath.Join(tmpDir, "a", "b"))
assert.True(t, os.IsNotExist(err), "empty subdirectory 'b' should have been removed")

_, err = os.Stat(filepath.Join(tmpDir, "a"))
assert.True(t, os.IsNotExist(err), "empty subdirectory 'a' should have been removed")

// Root directory must still exist
_, err = os.Stat(tmpDir)
assert.NoError(t, err, "root directory must not be removed")
}

func TestDeleteObjectsKeepsNonEmptyFolders(t *testing.T) {
tmpDir := setupTmpDir(t)
defer os.RemoveAll(tmpDir)

st, err := ConfigureStorage(tmpDir, nil)
assert.NoError(t, err)

root := st.RootFolder()

// Create two files in the same subdirectory
sub := root.GetSubFolder("a/b")
err = sub.PutObject("file1", strings.NewReader("data1"))
assert.NoError(t, err)
err = sub.PutObject("file2", strings.NewReader("data2"))
assert.NoError(t, err)

// Delete only one file
err = sub.DeleteObjects([]storage.Object{storage.NewLocalObject("file1", time.Time{}, 0)})
assert.NoError(t, err)

// The subdirectory should still exist because file2 is still there
_, err = os.Stat(filepath.Join(tmpDir, "a", "b"))
assert.NoError(t, err, "non-empty subdirectory 'b' should not be removed")
}

func setupTmpDir(t *testing.T) string {
cwd, err := filepath.Abs("./")
if err != nil {
Expand Down
Loading