From 93f92498ce26873fe40ddc71381e1f615cc58ceb Mon Sep 17 00:00:00 2001 From: Stephen J Day Date: Tue, 1 Dec 2015 16:24:31 -0800 Subject: [PATCH] storage: enforce sorted traversal during Walk Signed-off-by: Stephen J Day --- docs/storage/walk.go | 6 ++++++ docs/storage/walk_test.go | 8 ++++++++ 2 files changed, 14 insertions(+) diff --git a/docs/storage/walk.go b/docs/storage/walk.go index a27c2b03..d979796e 100644 --- a/docs/storage/walk.go +++ b/docs/storage/walk.go @@ -3,6 +3,7 @@ package storage import ( "errors" "fmt" + "sort" "github.com/docker/distribution/context" storageDriver "github.com/docker/distribution/registry/storage/driver" @@ -26,7 +27,12 @@ func Walk(ctx context.Context, driver storageDriver.StorageDriver, from string, 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 diff --git a/docs/storage/walk_test.go b/docs/storage/walk_test.go index 684155b2..5b922e0f 100644 --- a/docs/storage/walk_test.go +++ b/docs/storage/walk_test.go @@ -2,6 +2,7 @@ package storage import ( "fmt" + "sort" "testing" "github.com/docker/distribution/context" @@ -73,6 +74,7 @@ func TestWalkErrors(t *testing.T) { 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] @@ -90,11 +92,17 @@ func TestWalk(t *testing.T) { } } 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()) }