From cbcbcb02c5fef21c0915e42902287976a8452f63 Mon Sep 17 00:00:00 2001 From: Sargun Dhillon Date: Thu, 18 Jan 2018 12:10:20 -0800 Subject: [PATCH] Remove old walk function This removes the old global walk function, and changes all the code to use the per-driver walk functions. Signed-off-by: Sargun Dhillon --- registry/storage/catalog.go | 2 +- registry/storage/driver/base/base.go | 2 +- registry/storage/error.go | 9 ++ registry/storage/purgeuploads.go | 2 +- registry/storage/purgeuploads_test.go | 2 +- registry/storage/walk.go | 51 --------- registry/storage/walk_test.go | 152 -------------------------- 7 files changed, 13 insertions(+), 207 deletions(-) create mode 100644 registry/storage/error.go delete mode 100644 registry/storage/walk.go delete mode 100644 registry/storage/walk_test.go diff --git a/registry/storage/catalog.go b/registry/storage/catalog.go index 902f40c2f..3c1a78ded 100644 --- a/registry/storage/catalog.go +++ b/registry/storage/catalog.go @@ -63,7 +63,7 @@ func (reg *registry) Enumerate(ctx context.Context, ingester func(string) error) return err } - err = Walk(ctx, reg.blobStore.driver, root, func(fileInfo driver.FileInfo) error { + err = reg.blobStore.driver.Walk(ctx, root, func(fileInfo driver.FileInfo) error { return handleRepository(fileInfo, root, "", ingester) }) diff --git a/registry/storage/driver/base/base.go b/registry/storage/driver/base/base.go index 149cfcb2b..87063e431 100644 --- a/registry/storage/driver/base/base.go +++ b/registry/storage/driver/base/base.go @@ -203,7 +203,7 @@ func (base *Base) Walk(ctx context.Context, path string, f storagedriver.WalkFn) ctx, done := dcontext.WithTrace(ctx) defer done("%s.Walk(%q)", base.Name(), path) - if !storagedriver.PathRegexp.MatchString(path) { + if !storagedriver.PathRegexp.MatchString(path) && path != "/" { return storagedriver.InvalidPathError{Path: path, DriverName: base.StorageDriver.Name()} } diff --git a/registry/storage/error.go b/registry/storage/error.go new file mode 100644 index 000000000..109136586 --- /dev/null +++ b/registry/storage/error.go @@ -0,0 +1,9 @@ +package storage + +import "fmt" + +// pushError formats an error type given a path and an error +// and pushes it to a slice of errors +func pushError(errors []error, path string, err error) []error { + return append(errors, fmt.Errorf("%s: %s", path, err)) +} diff --git a/registry/storage/purgeuploads.go b/registry/storage/purgeuploads.go index 4292bf7a0..a82107a74 100644 --- a/registry/storage/purgeuploads.go +++ b/registry/storage/purgeuploads.go @@ -67,7 +67,7 @@ func getOutstandingUploads(ctx context.Context, driver storageDriver.StorageDriv return uploads, append(errors, err) } - err = Walk(ctx, driver, root, func(fileInfo storageDriver.FileInfo) error { + err = driver.Walk(ctx, root, func(fileInfo storageDriver.FileInfo) error { filePath := fileInfo.Path() _, file := path.Split(filePath) if file[0] == '_' { diff --git a/registry/storage/purgeuploads_test.go b/registry/storage/purgeuploads_test.go index 069f212d5..23c553ba3 100644 --- a/registry/storage/purgeuploads_test.go +++ b/registry/storage/purgeuploads_test.go @@ -142,7 +142,7 @@ func TestPurgeMissingStartedAt(t *testing.T) { oneHourAgo := time.Now().Add(-1 * time.Hour) fs, ctx := testUploadFS(t, 1, "test-repo", oneHourAgo) - err := Walk(ctx, fs, "/", func(fileInfo driver.FileInfo) error { + err := fs.Walk(ctx, "/", func(fileInfo driver.FileInfo) error { filePath := fileInfo.Path() _, file := path.Split(filePath) diff --git a/registry/storage/walk.go b/registry/storage/walk.go deleted file mode 100644 index add3ec463..000000000 --- a/registry/storage/walk.go +++ /dev/null @@ -1,51 +0,0 @@ -package storage - -import ( - "context" - "fmt" - "sort" - - storageDriver "github.com/docker/distribution/registry/storage/driver" -) - -// Walk traverses a filesystem defined within driver, starting -// from the given path, calling f on each file -// If the returned error from the WalkFn is ErrSkipDir and fileInfo refers -// to a directory, the directory will not be entered and Walk -// will continue the traversal. Otherwise Walk will return -// the error -func Walk(ctx context.Context, driver storageDriver.StorageDriver, from string, f storageDriver.WalkFn) error { - children, err := driver.List(ctx, from) - if err != nil { - return err - } - sort.Stable(sort.StringSlice(children)) - for _, child := range children { - // TODO(stevvooe): Calling driver.Stat for every entry is quite - // expensive when running against backends with a slow Stat - // implementation, such as s3. This is very likely a serious - // performance bottleneck. - fileInfo, err := driver.Stat(ctx, child) - if err != nil { - return err - } - err = f(fileInfo) - skipDir := (err == storageDriver.ErrSkipDir) - if err != nil && !skipDir { - return err - } - - if fileInfo.IsDir() && !skipDir { - if err := Walk(ctx, driver, child, f); err != nil { - return err - } - } - } - return nil -} - -// pushError formats an error type given a path and an error -// and pushes it to a slice of errors -func pushError(errors []error, path string, err error) []error { - return append(errors, fmt.Errorf("%s: %s", path, err)) -} diff --git a/registry/storage/walk_test.go b/registry/storage/walk_test.go deleted file mode 100644 index b0e7f4bc4..000000000 --- a/registry/storage/walk_test.go +++ /dev/null @@ -1,152 +0,0 @@ -package storage - -import ( - "context" - "fmt" - "sort" - "testing" - - "github.com/docker/distribution/registry/storage/driver" - "github.com/docker/distribution/registry/storage/driver/inmemory" -) - -func testFS(t *testing.T) (driver.StorageDriver, map[string]string, context.Context) { - d := inmemory.New() - ctx := context.Background() - - expected := map[string]string{ - "/a": "dir", - "/a/b": "dir", - "/a/b/c": "dir", - "/a/b/c/d": "file", - "/a/b/c/e": "file", - "/a/b/f": "dir", - "/a/b/f/g": "file", - "/a/b/f/h": "file", - "/a/b/f/i": "file", - "/z": "dir", - "/z/y": "file", - } - - for p, typ := range expected { - if typ != "file" { - continue - } - - if err := d.PutContent(ctx, p, []byte(p)); err != nil { - t.Fatalf("unable to put content into fixture: %v", err) - } - } - - return d, expected, ctx -} - -func TestWalkErrors(t *testing.T) { - d, expected, ctx := testFS(t) - fileCount := len(expected) - err := Walk(ctx, d, "", func(fileInfo driver.FileInfo) error { - return nil - }) - if err == nil { - t.Error("Expected invalid root err") - } - - errEarlyExpected := fmt.Errorf("Early termination") - - err = Walk(ctx, d, "/", func(fileInfo driver.FileInfo) error { - // error on the 2nd file - if fileInfo.Path() == "/a/b" { - return errEarlyExpected - } - - delete(expected, fileInfo.Path()) - return nil - }) - if len(expected) != fileCount-1 { - t.Error("Walk failed to terminate with error") - } - if err != errEarlyExpected { - if err == nil { - t.Fatalf("expected an error due to early termination") - } else { - t.Error(err.Error()) - } - } - - err = Walk(ctx, d, "/nonexistent", func(fileInfo driver.FileInfo) error { - return nil - }) - if err == nil { - t.Errorf("Expected missing file err") - } - -} - -func TestWalk(t *testing.T) { - d, expected, ctx := testFS(t) - var traversed []string - err := Walk(ctx, d, "/", func(fileInfo driver.FileInfo) error { - filePath := fileInfo.Path() - filetype, ok := expected[filePath] - if !ok { - t.Fatalf("Unexpected file in walk: %q", filePath) - } - - if fileInfo.IsDir() { - if filetype != "dir" { - t.Errorf("Unexpected file type: %q", filePath) - } - } else { - if filetype != "file" { - t.Errorf("Unexpected file type: %q", filePath) - } - - // each file has its own path as the contents. If the length - // doesn't match the path length, fail. - if fileInfo.Size() != int64(len(fileInfo.Path())) { - t.Fatalf("unexpected size for %q: %v != %v", - fileInfo.Path(), fileInfo.Size(), len(fileInfo.Path())) - } - } - delete(expected, filePath) - traversed = append(traversed, filePath) - return nil - }) - if len(expected) > 0 { - t.Errorf("Missed files in walk: %q", expected) - } - - if !sort.StringsAreSorted(traversed) { - t.Errorf("result should be sorted: %v", traversed) - } - - if err != nil { - t.Fatalf(err.Error()) - } -} - -func TestWalkSkipDir(t *testing.T) { - d, expected, ctx := testFS(t) - err := Walk(ctx, d, "/", func(fileInfo driver.FileInfo) error { - filePath := fileInfo.Path() - if filePath == "/a/b" { - // skip processing /a/b/c and /a/b/c/d - return driver.ErrSkipDir - } - delete(expected, filePath) - return nil - }) - if err != nil { - t.Fatalf(err.Error()) - } - if _, ok := expected["/a/b/c"]; !ok { - t.Errorf("/a/b/c not skipped") - } - if _, ok := expected["/a/b/c/d"]; !ok { - t.Errorf("/a/b/c/d not skipped") - } - if _, ok := expected["/a/b/c/e"]; !ok { - t.Errorf("/a/b/c/e not skipped") - } - -}