Revert "optimize catalog last param"

This reverts commit 65f4ce4d93.

Signed-off-by: James Hewitt <james.hewitt@uk.ibm.com>

registry/storage/catalog.go

@@ -5,17 +5,12 @@ import (
 	"errors"
 	"io"
 	"path"
-	"sort"
 	"strings"
 
 	"github.com/distribution/distribution/v3/reference"
 	"github.com/distribution/distribution/v3/registry/storage/driver"
 )
 
-var (
-	ErrStopRec = errors.New("Stopped the recursion for getting repositories")
-)
-
 // Returns a list, or partial list, of repositories in the registry.
 // Because it's a quite expensive operation, it should only be used when building up
 // an initial set of repositories.
@@ -32,17 +27,20 @@ func (reg *registry) Repositories(ctx context.Context, repos []string, last stri
 		return 0, err
 	}
 
-	err = reg.getRepositories(ctx, root, last, func(repoPath string) error {
+	err = reg.blobStore.driver.Walk(ctx, root, func(fileInfo driver.FileInfo) error {
-		// this is placed before the append,
+		err := handleRepository(fileInfo, root, last, func(repoPath string) error {
-		// so that we will get a extra repo if
+			foundRepos = append(foundRepos, repoPath)
-		// any. This assures that we do not return
+			return nil
-		// io.EOF without it being the last record.
+		})
-		if len(foundRepos) == len(repos) {
+		if err != nil {
-			finishedWalk = true
+			return err
-			return ErrStopRec
 		}
 
-		foundRepos = append(foundRepos, repoPath)
+		// if we've filled our array, no need to walk any further
+		if len(foundRepos) == len(repos) {
+			finishedWalk = true
+			return driver.ErrSkipDir
+		}
 
 		return nil
 	})
@@ -66,7 +64,11 @@ func (reg *registry) Enumerate(ctx context.Context, ingester func(string) error)
 		return err
 	}
 
-	return reg.getRepositories(ctx, root, "", ingester)
+	err = reg.blobStore.driver.Walk(ctx, root, func(fileInfo driver.FileInfo) error {
+		return handleRepository(fileInfo, root, "", ingester)
+	})
+
+	return err
 }
 
 // Remove removes a repository from storage
@@ -79,95 +81,78 @@ func (reg *registry) Remove(ctx context.Context, name reference.Named) error {
 	return reg.driver.Delete(ctx, repoDir)
 }
 
-// getRepositories is a helper function for getRepositoriesRec calls
+// lessPath returns true if one path a is less than path b.
-// the function fn with a repository path, if the current path looked
+//
-// at is a repository and is lexicographically after last. It is possible
+// A component-wise comparison is done, rather than the lexical comparison of
-// to return driver.ErrSkipDir, if there is no interest in any repositories
+// strings.
-// under the given `repoPath`, or call ErrStopRec if the recursion should stop.
+func lessPath(a, b string) bool {
-func (reg *registry) getRepositories(ctx context.Context, root, last string, fn func(repoPath string) error) error {
+	// we provide this behavior by making separator always sort first.
-	midFn := fn
+	return compareReplaceInline(a, b, '/', '\x00') < 0
-
-	// middleware func to exclude the `last` repo
-	// only use it, if there is set a last.
-	if last != "" {
-		midFn = func(repoPath string) error {
-			if repoPath != last {
-				return fn(repoPath)
-			}
-			return nil
-		}
-	}
-
-	// call our recursive func, with the midFn and the start path
-	// of where we want to find repositories.
-	err := reg.getRepositoriesRec(ctx, root, root, last, midFn)
-	if err == ErrStopRec {
-		return nil
-	}
-	return err
 }
 
-// getRepositoriesRec recurse through all folders it the `lookPath`,
+// compareReplaceInline modifies runtime.cmpstring to replace old with new
-// there it will try to find repositories. See getRepositories for more.
+// during a byte-wise comparison.
-func (reg *registry) getRepositoriesRec(ctx context.Context, root, lookPath, last string, fn func(repoPath string) error) error {
+func compareReplaceInline(s1, s2 string, old, new byte) int {
-	// ensure that the current path is a dir, otherwise we just return
+	// TODO(stevvooe): We are missing an optimization when the s1 and s2 have
-	if f, err := reg.blobStore.driver.Stat(ctx, lookPath); err != nil || !f.IsDir() {
+	// the exact same slice header. It will make the code unsafe but can
-		if err != nil {
+	// provide some extra performance.
-			return err
+
-		}
+	l := len(s1)
-		return nil
+	if len(s2) < l {
+		l = len(s2)
 	}
 
-	// get children in the current path
+	for i := 0; i < l; i++ {
-	children, err := reg.blobStore.driver.List(ctx, lookPath)
+		c1, c2 := s1[i], s2[i]
-	if err != nil {
+		if c1 == old {
-		return err
+			c1 = new
-	}
-
-	// sort this, so that it will be added in the correct order
-	sort.Strings(children)
-
-	if last != "" {
-		splitLasts := strings.Split(last, "/")
-
-		// call the next iteration of getRepositoriesRec if any, but
-		// exclude the current one.
-		if len(splitLasts) > 1 {
-			if err := reg.getRepositoriesRec(ctx, root, lookPath+"/"+splitLasts[0], strings.Join(splitLasts[1:], "/"), fn); err != nil {
-				return err
-			}
 		}
 
-		// find current last path in our children
+		if c2 == old {
-		n := sort.SearchStrings(children, lookPath+"/"+splitLasts[0])
+			c2 = new
-		if n == len(children) || children[n] != lookPath+"/"+splitLasts[0] {
-			return errors.New("the provided 'last' repositories does not exists")
 		}
 
-		// if this is not a final `last` (there are more `/` left)
+		if c1 < c2 {
-		// then exclude the current index, else include it
+			return -1
-		if len(splitLasts) > 1 {
+		}
-			children = children[n+1:]
+
-		} else {
+		if c1 > c2 {
-			children = children[n:]
+			return +1
 		}
 	}
 
-	for _, child := range children {
+	if len(s1) < len(s2) {
-		_, file := path.Split(child)
+		return -1
+	}
 
-		if file == "_manifest" {
+	if len(s1) > len(s2) {
-			if err := fn(strings.TrimPrefix(lookPath, root+"/")); err != nil {
+		return +1
-				if err == driver.ErrSkipDir {
+	}
-					break
+
-				}
+	return 0
-				return err
+}
-			}
+
-		} else if file[0] != '_' {
+// handleRepository calls function fn with a repository path if fileInfo
-			if err := reg.getRepositoriesRec(ctx, root, child, "", fn); err != nil {
+// has a path of a repository under root and that it is lexographically
+// after last. Otherwise, it will return ErrSkipDir. This should be used
+// with Walk to do handling with repositories in a storage.
+func handleRepository(fileInfo driver.FileInfo, root, last string, fn func(repoPath string) error) error {
+	filePath := fileInfo.Path()
+
+	// lop the base path off
+	repo := filePath[len(root)+1:]
+
+	_, file := path.Split(repo)
+	if file == "_manifests" {
+		repo = strings.TrimSuffix(repo, "/_manifests")
+		if lessPath(last, repo) {
+			if err := fn(repo); err != nil {
 				return err
 			}
 		}
+		return driver.ErrSkipDir
+	} else if strings.HasPrefix(file, "_") {
+		return driver.ErrSkipDir
 	}
 
 	return nil

registry/storage/catalog_test.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"fmt"
 	"io"
+	"math/rand"
 	"testing"
 
 	"github.com/distribution/distribution/v3"
@@ -240,7 +241,6 @@ func TestCatalogWalkError(t *testing.T) {
 		t.Errorf("Expected catalog driver list error")
 	}
 }
-<<<<<<< HEAD
 
 func BenchmarkPathCompareEqual(B *testing.B) {
 	B.StopTimer()
@@ -323,5 +323,3 @@ func randomFilename(length int64) string {
 	}
 	return string(b)
 }
-=======
->>>>>>> 27bd92bd (fix bug in catalog last param and optimized it)