Fixed the fallback implementation of WalkFilesFallback that wasn't working & added a Files Removed test for WalkFilesFallback.

registry/storage/driver/walk.go

@@ -22,6 +22,20 @@ type WalkFn func(fileInfo FileInfo) error
 // to a directory, the directory will not be entered and Walk
 // will continue the traversal.  If fileInfo refers to a normal file, processing stops
 func WalkFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error {
+	return doWalk(ctx, driver, from, true, f)
+}
+
+// WalkFilesFallback traverses a filesystem defined within driver, starting
+// from the given path, calling f on each file. It uses the List method and Stat to drive itself.
+// If an error is returned from WalkFn, processing stops
+func WalkFilesFallback(ctx context.Context, driver StorageDriver, from string, f WalkFn) error {
+	return doWalk(ctx, driver, from, false, f)
+}
+
+// WalkFilesFallback traverses a filesystem defined within driver, starting
+// from the given path, calling f on each file. It uses the List method and Stat to drive itself.
+// If an error is returned from WalkFn, processing stops
+func doWalk(ctx context.Context, driver StorageDriver, from string, walkDir bool, f WalkFn) error {
 	children, err := driver.List(ctx, from)
 	if err != nil {
 		return err
@@ -43,12 +57,15 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk
 				return err
 			}
 		}
-		err = f(fileInfo)
+		err = nil
+		if !fileInfo.IsDir() || walkDir {
+			err = f(fileInfo)
+		}
 		if err == nil && fileInfo.IsDir() {
-			if err := WalkFallback(ctx, driver, child, f); err != nil {
+			if err := doWalk(ctx, driver, child, walkDir, f); err != nil {
 				return err
 			}
-		} else if err == ErrSkipDir {
+		} else if err == ErrSkipDir && walkDir {
 			// Stop iteration if it's a file, otherwise noop if it's a directory
 			if !fileInfo.IsDir() {
 				return nil
@@ -59,41 +76,3 @@ func WalkFallback(ctx context.Context, driver StorageDriver, from string, f Walk
 	}
 	return nil
 }
-
-// WalkFilesFallback traverses a filesystem defined within driver, starting
-// from the given path, calling f on each file. It uses the List method and Stat to drive itself.
-// If an error is returned from WalkFn, processing stops
-func WalkFilesFallback(ctx context.Context, driver StorageDriver, from string, f 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 {
-			switch err.(type) {
-			case PathNotFoundError:
-				// repository was removed in between listing and enumeration. Ignore it.
-				logrus.WithField("path", child).Infof("ignoring deleted path")
-				continue
-			default:
-				return err
-			}
-		}
-		if !fileInfo.IsDir() {
-			err = f(fileInfo)
-			if err != nil {
-				return err
-			}
-		}
-		if err := WalkFallback(ctx, driver, child, f); err != nil {
-			return err
-		}
-	}
-	return nil
-}

registry/storage/driver/walk_test.go

@@ -45,3 +45,23 @@ func TestWalkFileRemoved(t *testing.T) {
 		t.Fatalf(err.Error())
 	}
 }
+
+func TestWalkFilesFileRemoved(t *testing.T) {
+	d := &changingFileSystem{
+		fileset: []string{"zoidberg", "bender"},
+		keptFiles: map[string]bool{
+			"zoidberg": true,
+		},
+	}
+	infos := []FileInfo{}
+	err := WalkFilesFallback(context.Background(), d, "", func(fileInfo FileInfo) error {
+		infos = append(infos, fileInfo)
+		return nil
+	})
+	if len(infos) != 1 || infos[0].Path() != "zoidberg" {
+		t.Errorf(fmt.Sprintf("unexpected path set during walk: %s", infos))
+	}
+	if err != nil {
+		t.Fatalf(err.Error())
+	}
+}