Merge pull request #93 from lytics/remove_empty_folders

Delete folders when empty for localfs

localfs/store.go

@@ -2,6 +2,7 @@ package localfs
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"io/ioutil"
@@ -278,11 +279,32 @@ func (l *LocalStore) Get(ctx context.Context, o string) (cloudstorage.Object, er
 // Delete the object from underlying store.
 func (l *LocalStore) Delete(ctx context.Context, obj string) error {
 	fo := path.Join(l.storepath, obj)
-	os.Remove(fo)
+	if err := os.Remove(fo); err != nil {
+		return fmt.Errorf("removing dir=%s: %w", fo, err)
+	}
 	mf := fo + ".metadata"
 	if cloudstorage.Exists(mf) {
-		os.Remove(mf)
+		if err := os.Remove(mf); err != nil {
+			return fmt.Errorf("removing dir=%s: %w", mf, err)
+		}
+	}
+
+	// When the last item in a folder is deleted, the folder
+	// should also be deleted. This matches the behavior in GCS.
+	dir, err := os.Open(l.storepath)
+	if err != nil {
+		return fmt.Errorf("failed to open store dir=%s err=%w", l.storepath, err)
 	}
+	if _, err = dir.Readdirnames(1); errors.Is(err, io.EOF) {
+		dir.Close()
+		// it's empty, so remove it.
+		if err := os.Remove(l.storepath); err != nil {
+			return fmt.Errorf("failed to remove store dir=%s err=%w", l.storepath, err)
+		}
+	} else {
+		dir.Close()
+	}
+
 	return nil
 }
 

testutils/testutils.go

@@ -90,6 +90,9 @@ func Clearstore(t TestingT, store cloudstorage.Store) {
 }
 
 func RunTests(t TestingT, s cloudstorage.Store, conf *cloudstorage.Config) {
+	// Ensure testing dirs are clean.
+	Clearstore(t, s)
+	defer Clearstore(t, s)
 
 	t.Logf("running store setup: type:%v", s.Type())
 	StoreSetup(t, s)
@@ -136,6 +139,7 @@ func deleteIfExists(store cloudstorage.Store, filePath string) {
 		obj.Delete()
 	}
 }
+
 func StoreSetup(t TestingT, store cloudstorage.Store) {
 
 	// Ensure the store has a String identifying store type
@@ -150,6 +154,12 @@ func BasicRW(t TestingT, store cloudstorage.Store) {
 	// Read the object from store, delete if it exists
 	deleteIfExists(store, "prefix/test.csv")
 
+	// Store should be empty
+	all, err := store.List(context.Background(), cloudstorage.NewQueryAll())
+	assert.NoError(t, err)
+	assert.NotNil(t, all)
+	assert.Empty(t, all.Objects)
+
 	// Create a new object and write to it.
 	obj, err := store.NewObject("prefix/test.csv")
 	assert.Equal(t, nil, err)
@@ -190,6 +200,12 @@ func BasicRW(t TestingT, store cloudstorage.Store) {
 
 	assert.Equal(t, testcsv, string(bytes))
 
+	// Store should be not empty
+	all, err = store.List(context.Background(), cloudstorage.NewQueryAll())
+	assert.NoError(t, err)
+	assert.NotNil(t, all)
+	assert.NotEmpty(t, all.Objects)
+
 	// Now delete again
 	err = obj2.Delete()
 	assert.Equal(t, nil, err)