Merge pull request #1227 from stevvooe/walk-sorted
storage: enforce sorted traversal during Walk
This commit is contained in:
commit
b66bb1287c
2 changed files with 50 additions and 11 deletions
|
@ -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
|
||||
|
@ -38,7 +44,9 @@ func Walk(ctx context.Context, driver storageDriver.StorageDriver, from string,
|
|||
}
|
||||
|
||||
if fileInfo.IsDir() && !skipDir {
|
||||
Walk(ctx, driver, child, f)
|
||||
if err := Walk(ctx, driver, child, f); err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
}
|
||||
return nil
|
||||
|
|
|
@ -2,6 +2,7 @@ package storage
|
|||
|
||||
import (
|
||||
"fmt"
|
||||
"sort"
|
||||
"testing"
|
||||
|
||||
"github.com/docker/distribution/context"
|
||||
|
@ -11,14 +12,7 @@ import (
|
|||
|
||||
func testFS(t *testing.T) (driver.StorageDriver, map[string]string, context.Context) {
|
||||
d := inmemory.New()
|
||||
c := []byte("")
|
||||
ctx := context.Background()
|
||||
if err := d.PutContent(ctx, "/a/b/c/d", c); err != nil {
|
||||
t.Fatalf("Unable to put to inmemory fs")
|
||||
}
|
||||
if err := d.PutContent(ctx, "/a/b/c/e", c); err != nil {
|
||||
t.Fatalf("Unable to put to inmemory fs")
|
||||
}
|
||||
|
||||
expected := map[string]string{
|
||||
"/a": "dir",
|
||||
|
@ -26,6 +20,22 @@ func testFS(t *testing.T) (driver.StorageDriver, map[string]string, context.Cont
|
|||
"/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
|
||||
|
@ -41,19 +51,26 @@ func TestWalkErrors(t *testing.T) {
|
|||
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 fmt.Errorf("Early termination")
|
||||
return errEarlyExpected
|
||||
}
|
||||
|
||||
delete(expected, fileInfo.Path())
|
||||
return nil
|
||||
})
|
||||
if len(expected) != fileCount-1 {
|
||||
t.Error("Walk failed to terminate with error")
|
||||
}
|
||||
if err != nil {
|
||||
t.Error(err.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, "/nonexistant", func(fileInfo driver.FileInfo) error {
|
||||
|
@ -67,6 +84,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]
|
||||
|
@ -82,13 +100,26 @@ func TestWalk(t *testing.T) {
|
|||
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())
|
||||
}
|
||||
|
|
Loading…
Reference in a new issue