Skip to content

Commit a6aafac

Browse files
nopcodernopcoder
authored
Local block adapter remove empty folders when possible (treeverse#1348)
1 parent 2b52225 commit a6aafac

File tree

2 files changed

+137
-17
lines changed

2 files changed

+137
-17
lines changed

‎block/local/adapter.go

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ type Adapter struct {
2828
path string
2929
ctx context.Context
3030
uploadIDTranslator block.UploadIDTranslator
31+
removeEmptyDir bool
3132
}
3233

3334
var (
@@ -41,6 +42,7 @@ func (l *Adapter) WithContext(ctx context.Context) block.Adapter {
4142
path: l.path,
4243
ctx: ctx,
4344
uploadIDTranslator: l.uploadIDTranslator,
45+
removeEmptyDir: l.removeEmptyDir,
4446
}
4547
}
4648

@@ -50,6 +52,12 @@ func WithTranslator(t block.UploadIDTranslator) func(a *Adapter) {
5052
}
5153
}
5254

55+
func WithRemoveEmptyDir(b bool) func(a *Adapter) {
56+
return func(a *Adapter) {
57+
a.removeEmptyDir = b
58+
}
59+
}
60+
5361
func NewAdapter(path string, opts ...func(a *Adapter)) (*Adapter, error) {
5462
path = filepath.Clean(path)
5563
err := os.MkdirAll(path, 0700)
@@ -63,6 +71,7 @@ func NewAdapter(path string, opts ...func(a *Adapter)) (*Adapter, error) {
6371
path: path,
6472
ctx: context.Background(),
6573
uploadIDTranslator: &block.NoOpTranslator{},
74+
removeEmptyDir: true,
6675
}
6776
for _, opt := range opts {
6877
opt(adapter)
@@ -104,6 +113,10 @@ func maybeMkdir(path string, f func(p string) (*os.File, error)) (*os.File, erro
104113
return f(path)
105114
}
106115

116+
func (l *Adapter) Path() string {
117+
return l.path
118+
}
119+
107120
func (l *Adapter) Put(obj block.ObjectPointer, _ int64, reader io.Reader, _ block.PutOpts) error {
108121
p, err := l.getPath(obj)
109122
if err != nil {
@@ -127,7 +140,34 @@ func (l *Adapter) Remove(obj block.ObjectPointer) error {
127140
return err
128141
}
129142
p = filepath.Clean(p)
130-
return os.Remove(p)
143+
err = os.Remove(p)
144+
if err != nil {
145+
return err
146+
}
147+
if l.removeEmptyDir {
148+
dir := filepath.Dir(p)
149+
removeEmptyDirUntil(dir, l.path)
150+
}
151+
return nil
152+
}
153+
154+
func removeEmptyDirUntil(dir string, stopAt string) {
155+
if stopAt == "" {
156+
return
157+
}
158+
if !strings.HasSuffix(stopAt, "/") {
159+
stopAt += "/"
160+
}
161+
for strings.HasPrefix(dir, stopAt) && dir != stopAt {
162+
err := os.Remove(dir)
163+
if err != nil {
164+
break
165+
}
166+
dir = filepath.Dir(dir)
167+
if dir == "/" {
168+
break
169+
}
170+
}
131171
}
132172

133173
func (l *Adapter) Copy(sourceObj, destinationObj block.ObjectPointer) error {

‎block/local/adapter_test.go

Lines changed: 96 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,40 +1,46 @@
11
package local_test
22

33
import (
4-
"github.com/aws/aws-sdk-go/aws"
5-
"github.com/aws/aws-sdk-go/service/s3"
64
"io/ioutil"
75
"os"
6+
"path/filepath"
7+
"sort"
88
"strings"
99
"testing"
1010

11+
"github.com/aws/aws-sdk-go/aws"
12+
"github.com/aws/aws-sdk-go/service/s3"
13+
"github.com/go-test/deep"
1114
"github.com/treeverse/lakefs/block"
1215
"github.com/treeverse/lakefs/block/local"
1316
"github.com/treeverse/lakefs/testutil"
1417
)
1518

16-
func makeAdapter(t *testing.T) (*local.Adapter, func()) {
19+
const testStorageNamespace = "local://test"
20+
21+
func makeAdapter(t *testing.T) *local.Adapter {
1722
t.Helper()
1823
dir, err := ioutil.TempDir("", "testing-local-adapter-*")
1924
testutil.MustDo(t, "TempDir", err)
2025
testutil.MustDo(t, "NewAdapter", os.MkdirAll(dir, 0700))
2126
a, err := local.NewAdapter(dir)
2227
testutil.MustDo(t, "NewAdapter", err)
2328

24-
return a, func() {
25-
if _, ok := os.LookupEnv("GOTEST_KEEP_LOCAL"); !ok {
26-
testutil.MustDo(t, "RemoveAll (cleanup)", os.RemoveAll(dir))
29+
t.Cleanup(func() {
30+
if _, ok := os.LookupEnv("GOTEST_KEEP_LOCAL"); ok {
31+
return
2732
}
28-
}
33+
_ = os.RemoveAll(dir)
34+
})
35+
return a
2936
}
3037

3138
func makePointer(path string) block.ObjectPointer {
32-
return block.ObjectPointer{Identifier: path, StorageNamespace: "local://test/"}
39+
return block.ObjectPointer{Identifier: path, StorageNamespace: testStorageNamespace}
3340
}
3441

3542
func TestLocalPutExistsGet(t *testing.T) {
36-
a, cleanup := makeAdapter(t)
37-
defer cleanup()
43+
a := makeAdapter(t)
3844

3945
cases := []struct {
4046
name string
@@ -66,8 +72,7 @@ func TestLocalPutExistsGet(t *testing.T) {
6672
}
6773

6874
func TestLocalNotExists(t *testing.T) {
69-
a, cleanup := makeAdapter(t)
70-
defer cleanup()
75+
a := makeAdapter(t)
7176

7277
cases := []string{"missing", "nested/down", "nested/quite/deeply/and/missing"}
7378
for _, c := range cases {
@@ -82,8 +87,7 @@ func TestLocalNotExists(t *testing.T) {
8287
}
8388

8489
func TestLocalMultipartUpload(t *testing.T) {
85-
a, cleanup := makeAdapter(t)
86-
defer cleanup()
90+
a := makeAdapter(t)
8791

8892
cases := []struct {
8993
name string
@@ -125,8 +129,7 @@ func TestLocalMultipartUpload(t *testing.T) {
125129
}
126130

127131
func TestLocalCopy(t *testing.T) {
128-
a, cleanup := makeAdapter(t)
129-
defer cleanup()
132+
a := makeAdapter(t)
130133

131134
contents := "foo bar baz quux"
132135

@@ -141,3 +144,80 @@ func TestLocalCopy(t *testing.T) {
141144
t.Errorf("expected to read \"%s\" as written, got \"%s\"", contents, string(got))
142145
}
143146
}
147+
148+
func dumpPathTree(t testing.TB, root string) []string {
149+
t.Helper()
150+
tree := make([]string, 0)
151+
err := filepath.Walk(root, func(path string, _ os.FileInfo, err error) error {
152+
if err != nil {
153+
return err
154+
}
155+
p := strings.TrimPrefix(path, root)
156+
tree = append(tree, p)
157+
return nil
158+
})
159+
if err != nil {
160+
t.Fatalf("walking on '%s': %s", root, err)
161+
}
162+
sort.Strings(tree)
163+
return tree
164+
}
165+
166+
func TestAdapter_Remove(t *testing.T) {
167+
const content = "Content used for testing"
168+
tests := []struct {
169+
name string
170+
additionalObjects []string
171+
path string
172+
wantErr bool
173+
wantTree []string
174+
}{
175+
{
176+
name: "single",
177+
path: "README",
178+
wantErr: false,
179+
wantTree: []string{""},
180+
},
181+
182+
{
183+
name: "under folder",
184+
path: "src/tools.go",
185+
wantErr: false,
186+
wantTree: []string{""},
187+
},
188+
{
189+
name: "under multiple folders",
190+
path: "a/b/c/d.txt",
191+
wantErr: false,
192+
wantTree: []string{""},
193+
},
194+
{
195+
name: "file in the way",
196+
path: "a/b/c/d.txt",
197+
additionalObjects: []string{"a/b/blocker.txt"},
198+
wantErr: false,
199+
wantTree: []string{"", "/test", "/test/a", "/test/a/b", "/test/a/b/blocker.txt"},
200+
},
201+
}
202+
203+
for _, tt := range tests {
204+
t.Run(tt.name, func(t *testing.T) {
205+
// setup env
206+
adp := makeAdapter(t)
207+
envObjects := append(tt.additionalObjects, tt.path)
208+
for _, o := range envObjects {
209+
obj := makePointer(o)
210+
testutil.MustDo(t, "Put", adp.Put(obj, 0, strings.NewReader(content), block.PutOpts{}))
211+
}
212+
// test Remove with remove empty folders
213+
obj := makePointer(tt.path)
214+
if err := adp.Remove(obj); (err != nil) != tt.wantErr {
215+
t.Errorf("Remove() error = %v, wantErr %v", err, tt.wantErr)
216+
}
217+
tree := dumpPathTree(t, adp.Path())
218+
if diff := deep.Equal(tt.wantTree, tree); diff != nil {
219+
t.Errorf("Remove() tree diff = %s", diff)
220+
}
221+
})
222+
}
223+
}

0 commit comments

Comments
 (0)