diff --git a/backend/walk.go b/backend/walk.go index 75e2cde1..767004fc 100644 --- a/backend/walk.go +++ b/backend/walk.go @@ -19,6 +19,7 @@ import ( "errors" "fmt" "io/fs" + "path" "slices" "strings" "syscall" @@ -27,6 +28,11 @@ import ( "github.com/versity/versitygw/s3response" ) +const ( + pathSeparator = "/" + pathDot = "." +) + type WalkResults struct { CommonPrefixes []types.CommonPrefix Objects []s3response.Object @@ -34,6 +40,26 @@ type WalkResults struct { NextMarker string } +// walkState holds the state needed during directory traversal +type walkState struct { + ctx context.Context + fileSystem fs.FS + prefix string + delimiter string + marker string + max int32 + getObj GetObjFunc + skipdirs []string + + // Mutable state + cpmap cpMap + objects []s3response.Object + pastMax bool + newMarker string + truncated bool + walkErr error +} + type GetObjFunc func(path string, d fs.DirEntry) (s3response.Object, error) var ErrSkipObj = errors.New("skip this object") @@ -69,9 +95,6 @@ func (c cpMap) CpArray() []types.CommonPrefix { // Walk walks the supplied fs.FS and returns results compatible with list // objects responses func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker string, max int32, getObj GetObjFunc, skipdirs []string) (WalkResults, error) { - cpmap := cpMap{} - var objects []s3response.Object - // if max is 0, it should return empty non-truncated result if max == 0 { return WalkResults{ @@ -79,231 +102,31 @@ func Walk(ctx context.Context, fileSystem fs.FS, prefix, delimiter, marker strin }, nil } - var pastMarker bool - if marker == "" { - pastMarker = true + state := &walkState{ + ctx: ctx, + fileSystem: fileSystem, + prefix: prefix, + delimiter: delimiter, + marker: marker, + max: max, + getObj: getObj, + skipdirs: skipdirs, + cpmap: cpMap{}, } - var pastMax bool - var newMarker string - var truncated bool - - root := "." - if strings.Contains(prefix, "/") { - idx := strings.LastIndex(prefix, "/") - if idx > 0 { - root = prefix[:idx] - } - } - - err := WalkDir(fileSystem, root, func(path string, d fs.DirEntry, err error) error { - if err != nil { - return err - } - if ctx.Err() != nil { - return ctx.Err() - } - // Ignore the root directory - if path == "." { - return nil - } - if contains(d.Name(), skipdirs) { - return fs.SkipDir - } - - // After this point, return skipflag instead of nil - // so we can skip a directory without an early return - var skipflag error - if d.IsDir() { - // If prefix is defined and the directory does not match prefix, - // do not descend into the directory because nothing will - // match this prefix. Make sure to append the / at the end of - // directories since this is implied as a directory path name. - // If path is a prefix of prefix, then path could still be - // building to match. So only skip if path isn't a prefix of prefix - // and prefix isn't a prefix of path. - if prefix != "" && - !strings.HasPrefix(path+"/", prefix) && - !strings.HasPrefix(prefix, path+"/") { - return fs.SkipDir - } - - // Don't recurse into subdirectories which contain the delimiter - // after reaching the prefix - if delimiter != "" && - strings.HasPrefix(path+"/", prefix) && - strings.Contains(strings.TrimPrefix(path+"/", prefix), delimiter) { - skipflag = fs.SkipDir - } else { - if delimiter == "" { - dirobj, err := getObj(path+"/", d) - if err == ErrSkipObj { - return skipflag - } - if err != nil { - return fmt.Errorf("directory to object %q: %w", path, err) - } - if pastMax { - truncated = true - return fs.SkipAll - } - objects = append(objects, dirobj) - if (len(objects) + cpmap.Len()) == int(max) { - newMarker = path - pastMax = true - } - - return skipflag - } - - // TODO: can we do better here rather than a second readdir - // per directory? - ents, err := fs.ReadDir(fileSystem, path) - if err != nil { - return fmt.Errorf("readdir %q: %w", path, err) - } - - if len(ents) != 0 { - return skipflag - } - } - path += "/" - } - - if !pastMarker { - if path == marker { - pastMarker = true - return skipflag - } - if path < marker { - return skipflag - } - } - - // If object doesn't have prefix, don't include in results. - if prefix != "" && !strings.HasPrefix(path, prefix) { - return skipflag - } - - if delimiter == "" { - // If no delimiter specified, then all files with matching - // prefix are included in results - obj, err := getObj(path, d) - if err == ErrSkipObj { - return skipflag - } - if err != nil { - return fmt.Errorf("file to object %q: %w", path, err) - } - if pastMax { - truncated = true - return fs.SkipAll - } - - objects = append(objects, obj) - - if (len(objects) + cpmap.Len()) == int(max) { - newMarker = path - pastMax = true - } - - return skipflag - } - - // Since delimiter is specified, we only want results that - // do not contain the delimiter beyond the prefix. If the - // delimiter exists past the prefix, then the substring - // between the prefix and delimiter is part of common prefixes. - // - // For example: - // prefix = A/ - // delimiter = / - // and objects: - // A/file - // A/B/file - // B/C - // would return: - // objects: A/file - // common prefix: A/B/ - // - // Note: No objects are included past the common prefix since - // these are all rolled up into the common prefix. - // Note: The delimiter can be anything, so we have to operate on - // the full path without any assumptions on posix directory hierarchy - // here. Usually the delimiter will be "/", but thats not required. - suffix := strings.TrimPrefix(path, prefix) - before, _, found := strings.Cut(suffix, delimiter) - if !found { - obj, err := getObj(path, d) - if err == ErrSkipObj { - return skipflag - } - if err != nil { - return fmt.Errorf("file to object %q: %w", path, err) - } - if pastMax { - truncated = true - return fs.SkipAll - } - objects = append(objects, obj) - if (len(objects) + cpmap.Len()) == int(max) { - newMarker = path - pastMax = true - } - return skipflag - } - - // Common prefixes are a set, so should not have duplicates. - // These are abstractly a "directory", so need to include the - // delimiter at the end when we add to the map. - cprefNoDelim := prefix + before - cpref := prefix + before + delimiter - if cpref == marker { - pastMarker = true - return skipflag - } - - if marker != "" && strings.HasPrefix(marker, cprefNoDelim) { - // skip common prefixes that are before the marker - return skipflag - } - - if pastMax { - truncated = true - return fs.SkipAll - } - cpmap.Add(cpref) - if (len(objects) + cpmap.Len()) == int(max) { - newMarker = cpref - pastMax = true - } - - return skipflag - }) - if err != nil { - // suppress file not found caused by user's prefix - if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { - return WalkResults{}, nil - } - return WalkResults{}, err - } - - if !truncated { - newMarker = "" + qwErr := quickWalk(state) + if qwErr != nil { + return WalkResults{}, qwErr } return WalkResults{ - CommonPrefixes: cpmap.CpArray(), - Objects: objects, - Truncated: truncated, - NextMarker: newMarker, + CommonPrefixes: state.cpmap.CpArray(), + Objects: state.objects, + Truncated: state.truncated, + NextMarker: state.newMarker, }, nil } -func contains(a string, strs []string) bool { - return slices.Contains(strs, a) -} - type WalkVersioningResults struct { CommonPrefixes []types.CommonPrefix ObjectVersions []s3response.ObjectVersion @@ -339,7 +162,7 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM pastVersionIdMarker := versionIdMarker == "" - err := WalkDir(fileSystem, ".", func(path string, d fs.DirEntry, err error) error { + err := walkDirRoot(fileSystem, ".", func(path string, d fs.DirEntry, err error) error { if err != nil { return err } @@ -350,7 +173,7 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM if path == "." { return nil } - if contains(d.Name(), skipdirs) { + if slices.Contains(skipdirs, d.Name()) { return fs.SkipDir } @@ -512,3 +335,318 @@ func WalkVersions(ctx context.Context, fileSystem fs.FS, prefix, delimiter, keyM NextVersionIdMarker: nextVersionIdMarker, }, nil } + +func readDir(path string, walkstate *walkState) { + err := walkstate.ctx.Err() + if err != nil { + walkstate.walkErr = err + return + } + + entries, err := fs.ReadDir(walkstate.fileSystem, path) + if err != nil { + // Suppress not-found / not-a-dir errors: they indicate either a + // user-supplied prefix that doesn't exist on disk, or a directory + // that was removed concurrently during the walk. In both cases, + // treat the subtree as empty rather than surfacing an error. + if errors.Is(err, fs.ErrNotExist) || errors.Is(err, syscall.ENOTDIR) { + return + } + walkstate.walkErr = fmt.Errorf("readdir %q: %w", path, err) + return + } + + readDirEntries(path, entries, walkstate) +} + +func readDirEntries(path string, entries []fs.DirEntry, walkstate *walkState) { + + entriesIndex := 0 + maxEntries := len(entries) + + for entriesIndex < maxEntries { + err := walkstate.ctx.Err() + if err != nil { + walkstate.walkErr = err + return + } + + if shouldSkip(entries[entriesIndex].Name(), walkstate) { + entriesIndex++ + continue + } + if walkstate.pastMax { + walkstate.truncated = true + return + } + if entries[entriesIndex].IsDir() { + // processDir returns the index of the next unprocessed entry + // (the first sibling that lexically follows the directory); + // the directory itself is fully consumed inside processDir, + // so we must NOT increment entriesIndex after this call. + entriesIndex = processDir( + entries, + entriesIndex, + maxEntries, + path, + walkstate, + ) + } else { + + // this is a leaf entry so check if it needs to go + // in common prefixes, if not then add to results list + currentObjectName := makeFullObjectName(path, entries[entriesIndex].Name()) + if currentObjectName > walkstate.marker { + if !checkAndMaybeAddCommonPrefix(currentObjectName, walkstate) { + addContentEntry(currentObjectName, entries[entriesIndex], walkstate) + } + } + entriesIndex++ + } + } +} + +func processDir( + entries []fs.DirEntry, + currentEntry int, + maxEntries int, + path string, + walkstate *walkState, +) int { + currentDirectoryName := entries[currentEntry].Name() + "/" + + // check for later entries that should lexically sort before this directory + entriesIndex := currentEntry + 1 + for entriesIndex < maxEntries { + err := walkstate.ctx.Err() + if err != nil { + walkstate.walkErr = err + return entriesIndex + } + + if walkstate.pastMax { + walkstate.truncated = true + return entriesIndex + } + + if entries[entriesIndex].Name() < currentDirectoryName { + // this entry precedes + // if it is a directory, we + // need to call recursively to + // check for further preceding entries + if shouldSkip(entries[entriesIndex].Name(), walkstate) { + entriesIndex++ + continue + } + if entries[entriesIndex].IsDir() { + entriesIndex = processDir( + entries, + entriesIndex, + maxEntries, + path, + walkstate, + ) + } else { + // this is a leaf entry so check if it needs to go + // in common prefixes, if not then add to results list + currentObjectName := makeFullObjectName(path, entries[entriesIndex].Name()) + if currentObjectName > walkstate.marker { + if !checkAndMaybeAddCommonPrefix(currentObjectName, walkstate) { + addContentEntry(currentObjectName, entries[entriesIndex], walkstate) + } + } + entriesIndex++ + } + } else { + // this entry does not precede, so no further ones will + // we can resume the dir that was current when we were called + break + } + } + + // now read and process the dir we were originally called for + if walkstate.pastMax { + walkstate.truncated = true + return entriesIndex + } + + // add directory to common prefixes if required + fullDirectoryName := makeFullObjectName(path, entries[currentEntry].Name()) + fullObjectName := fullDirectoryName + "/" + if fullObjectName > walkstate.marker { + if checkAndMaybeAddCommonPrefix(fullObjectName, walkstate) { + // we can cut short here UNLESS the current directory + // forms part of the prefix - in which case we must keep + // searching it in case the prefix is below + if !strings.HasPrefix(walkstate.prefix, fullObjectName) || walkstate.pastMax { + return entriesIndex + } + } else { + dirobj, err := walkstate.getObj(fullObjectName, entries[currentEntry]) + if err == ErrSkipObj { + // Directory exists in the filesystem but is not an object. + } else if err != nil { + walkstate.walkErr = fmt.Errorf("directory to object %q: %w", fullObjectName, err) + return entriesIndex + } else { + walkstate.addObject(dirobj, fullObjectName) + } + } + } else { + // if this directory is before startAfter, + // we can assume all its children are also before startAfter + // UNLESS this directory and startAfter share a + // common prefix + if !(strings.HasPrefix(fullObjectName, walkstate.marker) || + strings.HasPrefix(walkstate.marker, fullObjectName)) { + return entriesIndex + } + } + if walkstate.pastMax { + walkstate.truncated = true + return entriesIndex + } + readDir(fullDirectoryName, walkstate) + return entriesIndex +} + +func makeFullObjectName(path string, entry string) string { + if path == "." { + return entry + } + return path + "/" + entry +} + +// checkAndMaybeAddCommonPrefix returns true if the object +// has been added as a common prefix or discarded because +// it doesn't match the prefix at all +// +// otherwise returns false, and the caller should add it to +// the full results set +func checkAndMaybeAddCommonPrefix(objectName string, walkstate *walkState, +) bool { + // check prefix + if walkstate.prefix != "" && !strings.HasPrefix(objectName, walkstate.prefix) { + return true // doesn't match prefix - discard + } + + // check delimiter + if walkstate.delimiter != "" { + objectNameAfterPrefix := strings.TrimPrefix(objectName, walkstate.prefix) // remove prefix from name + objectNameBeforeDelimiter, _, delimiterFound := strings.Cut(objectNameAfterPrefix, walkstate.delimiter) + if delimiterFound { + // this object contributes to the common prefix list, + // but not the full results + commonPrefix := walkstate.prefix + objectNameBeforeDelimiter + walkstate.delimiter + if commonPrefix > walkstate.marker { + walkstate.addCommonPrefix(commonPrefix) + } + return true + } + } + + return false +} + +func shouldSkip(name string, walkstate *walkState) bool { + return slices.Contains(walkstate.skipdirs, name) +} + +func addContentEntry(objectName string, dirEntry fs.DirEntry, walkstate *walkState) bool { + obj, err := walkstate.getObj(objectName, dirEntry) + if err == ErrSkipObj { + return false + } + if err != nil { + walkstate.walkErr = fmt.Errorf("file to object %q: %w", objectName, err) + return false + } + + walkstate.addObject(obj, objectName) + return true +} + +// addObject adds an object to the results and checks if limits are reached +func (w *walkState) addObject(obj s3response.Object, path string) bool { + w.objects = append(w.objects, obj) + if len(w.objects)+w.cpmap.Len() >= int(w.max) { + w.newMarker = path + w.pastMax = true + // Don't set truncated here - wait until we know there are more items + return true + } + return false +} + +// addCommonPrefix adds a common prefix and checks if limits are reached +func (w *walkState) addCommonPrefix(cpref string) bool { + w.cpmap.Add(cpref) + if len(w.objects)+w.cpmap.Len() >= int(w.max) { + w.newMarker = cpref + w.pastMax = true + // Don't set truncated here - wait until we know there are more items + return true + } + return false +} + +func quickWalk(walkstate *walkState) error { + rootDir := pathDot + var rootEntries []fs.DirEntry + useRootEntries := false + + // see if we can jump straight into the prefix if it is a valid non-empty directory + if strings.Contains(walkstate.prefix, pathSeparator) { + if idx := strings.LastIndex(walkstate.prefix, pathSeparator); idx > 0 { + prefixDir := walkstate.prefix[:idx] + // If successful and non-empty, start from prefixDir and reuse entries + // to avoid a second ReadDir call. + entries, err := fs.ReadDir(walkstate.fileSystem, prefixDir) + if err == nil && len(entries) > 0 { + rootDir = prefixDir + rootEntries = entries + useRootEntries = true + } + } + } + + if useRootEntries { + // keep fast-path semantics but still honor skipdirs for the prefixed root + if !shouldSkip(path.Base(rootDir), walkstate) { + // Mirror processDir: offer the root directory itself to getObj whenever + // its key sorts after the marker, using checkAndMaybeAddCommonPrefix to + // handle the delimiter case. This replaces the previous guards that + // incorrectly skipped "a/" when hasSubdirs==true or marker!="". + fullObjectName := rootDir + pathSeparator + if fullObjectName > walkstate.marker { + if !checkAndMaybeAddCommonPrefix(fullObjectName, walkstate) { + rootInfo, err := fs.Stat(walkstate.fileSystem, rootDir) + if err != nil { + walkstate.walkErr = fmt.Errorf("stat %q: %w", rootDir, err) + } else { + dirobj, err := walkstate.getObj(fullObjectName, fs.FileInfoToDirEntry(rootInfo)) + if err == ErrSkipObj { + // Directory exists in the filesystem but is not an object. + } else if err != nil { + walkstate.walkErr = fmt.Errorf("directory to object %q: %w", fullObjectName, err) + } else { + walkstate.addObject(dirobj, fullObjectName) + } + } + } + } + if walkstate.walkErr == nil { + readDirEntries(rootDir, rootEntries, walkstate) + } + } + } else { + readDir(rootDir, walkstate) + } + if walkstate.walkErr != nil { + return walkstate.walkErr + } + if !walkstate.truncated { + walkstate.newMarker = "" + } + return nil +} diff --git a/backend/walk_bench_test.go b/backend/walk_bench_test.go new file mode 100644 index 00000000..809b7634 --- /dev/null +++ b/backend/walk_bench_test.go @@ -0,0 +1,533 @@ +// Copyright 2026 Versity Software +// This file is licensed under the Apache License, Version 2.0 +// (the "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +// # Walk benchmarks +// +// ## Quick start – run all small benchmarks +// +// go test -bench='^BenchmarkWalk[^LH]' -benchtime=6x -count=6 ./backend/ +// +// ## Run by tier +// +// Small (5 k files): +// +// go test -bench='^BenchmarkWalk[^LH]' -benchtime=6x -count=6 ./backend/ +// +// Large (100 k files): +// +// go test -bench='^BenchmarkWalkLarge' -benchtime=3x ./backend/ +// +// Huge (500 k files): +// +// go test -bench='^BenchmarkWalkHuge' -benchtime=3x ./backend/ +// +// Note: Large and Huge benchmarks create fixture files under /tmp on first run. +// The fixtures are reused on subsequent runs as long as +// /tmp/versitygw_walk_bench/large is present. Remove that directory to force a +// full rebuild: +// +// rm -rf /tmp/versitygw_walk_bench/large +// +// ## Comparing against main using benchstat +// +// Install benchstat if needed: +// +// go install golang.org/x/perf/cmd/benchstat@latest +// +// 1. Record results on the current branch: +// +// go test -bench='^BenchmarkWalk[^LH]' -benchtime=3x -count=6 ./backend/ > /tmp/new.txt +// +// 2. Create a worktree for main and record results there: +// +// git worktree add /tmp/versitygw_main main +// cp backend/walk_bench_test.go /tmp/versitygw_main/backend/ +// (cd /tmp/versitygw_main && go test -bench='^BenchmarkWalk[^LH]' -benchtime=3x -count=6 ./backend/ > /tmp/old.txt) +// git worktree remove --force /tmp/versitygw_main +// +// 3. Compare: +// +// benchstat /tmp/old.txt /tmp/new.txt +// +// Use -count=6 (minimum) for statistically meaningful p-values. +// Use -benchtime=3x (at least 3 iterations per count) to reduce per-run noise. +// For Large/Huge tiers use -benchtime=3x -count=1 (each op already takes seconds). +package backend_test + +import ( + "context" + "fmt" + "io/fs" + "os" + "path/filepath" + "runtime" + "sync" + "testing" + "time" + + "github.com/versity/versitygw/backend" + "github.com/versity/versitygw/s3response" +) + +// benchRoot is the /tmp directory shared across all benchmarks in the run. +// It is created once by setupBenchDir and once (lazily) by setupLargeBenchDir. +const benchRoot = "/tmp/versitygw_walk_bench" + +// Tier sizes: +// +// Small – 5000 files per tree +// Large – 100000 files per tree +// Huge – 500000 files flat only +var ( + benchSetupOnce sync.Once + benchLargeSetupOnce sync.Once +) + +// parallelMkfile creates a batch of file paths in parallel using a worker +// pool sized to GOMAXPROCS, so setup time scales with CPU count. +func parallelMkfile(b *testing.B, paths []string) { + b.Helper() + workers := runtime.GOMAXPROCS(0) + ch := make(chan string, workers*4) + var wg sync.WaitGroup + var mu sync.Mutex + var firstErr error + for range workers { + wg.Add(1) + go func() { + defer wg.Done() + for path := range ch { + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + mu.Lock() + if firstErr == nil { + firstErr = err + } + mu.Unlock() + continue + } + f, err := os.Create(path) + if err != nil { + mu.Lock() + if firstErr == nil { + firstErr = err + } + mu.Unlock() + continue + } + _ = f.Close() + } + }() + } + for _, p := range paths { + ch <- p + } + close(ch) + wg.Wait() + if firstErr != nil { + b.Fatalf("bench setup: %v", firstErr) + } +} + +// Structure summary (files under benchRoot): +// +// small/flat/ – 5000 files directly in the directory +// small/deep/ – 100 top-level dirs × 50 files each = 5000 files (2 levels) +// small/wide/ – 200 top-level dirs, 5 subdirs × 5 files = 5000 files (3 levels) +// small/mixed/ – 10 prefix dirs × 100 files = 1000 files +// +// large/flat/ – 100000 files flat +// large/deep/ – 1000 dirs × 100 files = 100000 files +// large/wide/ – 500 dirs × 10 subdirs × 20 files = 100000 files +// large/mixed/ – 100 prefix dirs × 1 000 files = 100000 files +// large/flat_huge/ – 500000 files flat +func setupBenchDir(b *testing.B) { + b.Helper() + benchSetupOnce.Do(func() { + // Reuse fixtures from a previous run if they exist. + sentinel := filepath.Join(benchRoot, "small", "flat", "file04999.txt") + if _, err := os.Stat(sentinel); err == nil { + return + } + _ = os.RemoveAll(filepath.Join(benchRoot, "small")) + + var paths []string + + // small/flat/ – 5000 files + for i := range 5000 { + paths = append(paths, filepath.Join(benchRoot, "small", "flat", fmt.Sprintf("file%05d.txt", i))) + } + + // small/deep/ – 100 dirs × 50 files + for i := range 100 { + for j := range 50 { + paths = append(paths, filepath.Join(benchRoot, "small", "deep", + fmt.Sprintf("dir%03d", i), + fmt.Sprintf("file%03d.txt", j))) + } + } + + // small/wide/ – 200 dirs × 5 subdirs × 5 files + for i := range 200 { + for j := range 5 { + for k := range 5 { + paths = append(paths, filepath.Join(benchRoot, "small", "wide", + fmt.Sprintf("dir%03d", i), + fmt.Sprintf("sub%02d", j), + fmt.Sprintf("file%02d.txt", k))) + } + } + } + + // small/mixed/ – 10 prefix dirs × 100 files + for i := range 10 { + for j := range 100 { + paths = append(paths, filepath.Join(benchRoot, "small", "mixed", + fmt.Sprintf("prefix%02d", i), + fmt.Sprintf("file%04d.txt", j))) + } + } + + parallelMkfile(b, paths) + }) +} + +func setupLargeBenchDir(b *testing.B) { + b.Helper() + setupBenchDir(b) // ensure small/ exists too + benchLargeSetupOnce.Do(func() { + // Reuse fixtures from a previous run if they exist. + sentinel := filepath.Join(benchRoot, "large", "flat_huge", "file0499999.txt") + if _, err := os.Stat(sentinel); err == nil { + return + } + _ = os.RemoveAll(filepath.Join(benchRoot, "large")) + var paths []string + + // large/flat/ – 100000 files flat + for i := range 100_000 { + paths = append(paths, filepath.Join(benchRoot, "large", "flat", fmt.Sprintf("file%06d.txt", i))) + } + + // large/deep/ – 1000 dirs × 100 files = 100000 files + for i := range 1000 { + for j := range 100 { + paths = append(paths, filepath.Join(benchRoot, "large", "deep", + fmt.Sprintf("dir%04d", i), + fmt.Sprintf("file%03d.txt", j))) + } + } + + // large/wide/ – 500 dirs × 10 subdirs × 20 files = 100000 files + for i := range 500 { + for j := range 10 { + for k := range 20 { + paths = append(paths, filepath.Join(benchRoot, "large", "wide", + fmt.Sprintf("dir%03d", i), + fmt.Sprintf("sub%02d", j), + fmt.Sprintf("file%02d.txt", k))) + } + } + } + + // large/mixed/ – 100 prefix dirs × 1000 files = 100000 files + for i := range 100 { + for j := range 1000 { + paths = append(paths, filepath.Join(benchRoot, "large", "mixed", + fmt.Sprintf("prefix%03d", i), + fmt.Sprintf("file%05d.txt", j))) + } + } + + // large/flat_huge/ – 500000 files flat + for i := range 500_000 { + paths = append(paths, filepath.Join(benchRoot, "large", "flat_huge", fmt.Sprintf("file%07d.txt", i))) + } + + b.Log("creating large bench fixtures (this runs once per /tmp clean)...") + parallelMkfile(b, paths) + }) +} + +// benchGetObj is a lightweight GetObjFunc that mimics what the POSIX backend +// does: stat the entry and populate the minimum Object fields, returning +// ErrSkipObj for bare directories (no stored etag = no explicit S3 PUT). +func benchGetObj(path string, d fs.DirEntry) (s3response.Object, error) { + if d.IsDir() { + return s3response.Object{}, backend.ErrSkipObj + } + fi, err := d.Info() + if err != nil { + return s3response.Object{}, err + } + sz := fi.Size() + mt := fi.ModTime() + etag := path // cheap stand-in; avoids md5 cost skewing the walk measurement + return s3response.Object{ + Key: &path, + Size: &sz, + LastModified: &mt, + ETag: &etag, + }, nil +} + +// runWalk is a convenience wrapper so each benchmark loop is one line. +func runWalk(b *testing.B, fsys fs.FS, prefix, delim, marker string, max int32) { + b.Helper() + _, err := backend.Walk(context.Background(), fsys, prefix, delim, marker, max, benchGetObj, nil) + if err != nil { + b.Fatalf("Walk: %v", err) + } +} + +// ── Small (5 k files) ───────────────────────────────────────────────────────── + +// BenchmarkWalkFlat lists all 5000 files in a single flat directory. +func BenchmarkWalkFlat(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "flat")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 10000) + } +} + +// BenchmarkWalkFlatDelim lists with a "/" delimiter (all files collapse into +// the root; tests the common-prefix fast-path for a flat bucket). +func BenchmarkWalkFlatDelim(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "flat")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 10000) + } +} + +// BenchmarkWalkFlatPaged benchmarks paginated listing (1000 objects per page, +// walking through 5 pages of the flat directory). +func BenchmarkWalkFlatPaged(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "flat")) + for b.Loop() { + marker := "" + for { + res, err := backend.Walk(context.Background(), + fsys, "", "", marker, 1000, benchGetObj, nil) + if err != nil { + b.Fatalf("Walk: %v", err) + } + if !res.Truncated { + break + } + marker = res.NextMarker + } + } +} + +// BenchmarkWalkDeep lists all files recursively through 2 levels of dirs. +func BenchmarkWalkDeep(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "deep")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 10000) + } +} + +// BenchmarkWalkDeepDelim lists with "/" delimiter, returning 100 common +// prefixes instead of descending into every directory. +func BenchmarkWalkDeepDelim(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "deep")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 10000) + } +} + +// BenchmarkWalkWide lists all 5000 files across a wide tree (3 levels). +func BenchmarkWalkWide(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "wide")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 10000) + } +} + +// BenchmarkWalkWideDelim collapses the wide tree to the top-level dirs. +func BenchmarkWalkWideDelim(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small", "wide")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 10000) + } +} + +// BenchmarkWalkWithPrefix exercises the prefix-optimisation. +func BenchmarkWalkWithPrefix(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small")) + for b.Loop() { + runWalk(b, fsys, "mixed/prefix05/", "", "", 1000) + } +} + +// BenchmarkWalkWithPrefixDelim lists with a prefix and delimiter. +func BenchmarkWalkWithPrefixDelim(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small")) + for b.Loop() { + runWalk(b, fsys, "mixed/", "/", "", 1000) + } +} + +// BenchmarkWalkWithMarker benchmarks resuming a listing mid-way through the +// mixed/ prefix (simulates the second page of a paginated request). +func BenchmarkWalkWithMarker(b *testing.B) { + setupBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "small")) + marker := "mixed/prefix05/file0050.txt" + for b.Loop() { + runWalk(b, fsys, "mixed/", "", marker, 1000) + } +} + +// ── Large (100 k files) ────────────────────────────────────────── +// Run with: go test -bench=WalkLarge -benchtime=10s ./backend/ + +// BenchmarkWalkLargeFlat lists 100000 files in a flat directory. +func BenchmarkWalkLargeFlat(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "flat")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 200000) + } +} + +// BenchmarkWalkLargeFlatDelim collapses 100000 flat files under a "/" delimiter. +func BenchmarkWalkLargeFlatDelim(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "flat")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 200000) + } +} + +// BenchmarkWalkLargeFlatPaged pages through 100000 flat files, 1000 per page. +func BenchmarkWalkLargeFlatPaged(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "flat")) + for b.Loop() { + marker := "" + for { + res, err := backend.Walk(context.Background(), + fsys, "", "", marker, 1000, benchGetObj, nil) + if err != nil { + b.Fatalf("Walk: %v", err) + } + if !res.Truncated { + break + } + marker = res.NextMarker + } + } +} + +// BenchmarkWalkLargeDeep fully recurses through 1000 dirs × 100 files. +func BenchmarkWalkLargeDeep(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "deep")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 200000) + } +} + +// BenchmarkWalkLargeDeepDelim lists 1000 common-prefixes without recursing. +func BenchmarkWalkLargeDeepDelim(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "deep")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 200000) + } +} + +// BenchmarkWalkLargeWide fully recurses a 3-level wide tree (100000 files). +func BenchmarkWalkLargeWide(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "wide")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 200000) + } +} + +// BenchmarkWalkLargeWideDelim collapses the wide tree to its 500 top-level dirs. +func BenchmarkWalkLargeWideDelim(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "wide")) + for b.Loop() { + runWalk(b, fsys, "", "/", "", 200000) + } +} + +// BenchmarkWalkLargeWithPrefix jumps directly into a subdirectory via prefix. +func BenchmarkWalkLargeWithPrefix(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large")) + for b.Loop() { + runWalk(b, fsys, "mixed/prefix050/", "", "", 5000) + } +} + +// BenchmarkWalkLargeWithMarker resumes a listing mid-way through a 100000-file tree. +func BenchmarkWalkLargeWithMarker(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large")) + marker := "mixed/prefix050/file00500.txt" + for b.Loop() { + runWalk(b, fsys, "mixed/", "", marker, 5000) + } +} + +// ── Huge (500 k files) ──────────────────────────────────────────── +// Run with: go test -bench=WalkHuge -benchtime=3x ./backend/ +// (use -benchtime=Nx to limit iterations given each op is >1 s) + +// BenchmarkWalkHugeFlat fully lists 500000 files in a single directory. +func BenchmarkWalkHugeFlat(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "flat_huge")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 1_000_000) + } +} + +// BenchmarkWalkHugeFlatMarker resumes a listing mid-way through 500000 flat +// files returning the next 1000. +func BenchmarkWalkHugeFlatMarker(b *testing.B) { + setupLargeBenchDir(b) + fsys := os.DirFS(filepath.Join(benchRoot, "large", "flat_huge")) + // Marker sits at roughly the 50% point of the flat listing. + marker := fmt.Sprintf("file%07d.txt", 250_000) + for b.Loop() { + runWalk(b, fsys, "", "", marker, 1000) + } +} + +// ── Timing sanity check ─────────────────────────────────────────────────────── + +// BenchmarkWalkSetupTime measures just the one-time setup cost. +func BenchmarkWalkSetupTime(b *testing.B) { + start := time.Now() + setupBenchDir(b) + b.ReportMetric(float64(time.Since(start).Milliseconds()), "setup_ms") + fsys := os.DirFS(filepath.Join(benchRoot, "small", "flat")) + for b.Loop() { + runWalk(b, fsys, "", "", "", 1) + } +} diff --git a/backend/walk_test.go b/backend/walk_test.go index 1e7627a2..42a00c2c 100644 --- a/backend/walk_test.go +++ b/backend/walk_test.go @@ -86,11 +86,27 @@ func getMD5(text string) string { return hex.EncodeToString(hash[:]) } +// getObjSkipDirs mimics the POSIX backend's FileToObj behavior for implicitly +// created directories: it returns ErrSkipObj so that directories which exist +// only as filesystem artefacts of slash-separated paths are not surfaced as +// S3 objects. This is necessary when testing non-"/" delimiters where +// fstest.MapFS creates intermediate directories from the "/" path separators +// embedded in object keys. +func getObjSkipDirs(path string, d fs.DirEntry) (s3response.Object, error) { + if d.IsDir() { + return s3response.Object{}, backend.ErrSkipObj + } + return getObj(path, d) +} + func TestWalk(t *testing.T) { tests := []walkTest{ { // test case from // https://docs.aws.amazon.com/AmazonS3/latest/userguide/using-prefixes.html + // All directories in this fsys are implicit filesystem artefacts (no S3 + // object was explicitly PUT at any directory path), so getObjSkipDirs is + // used to mirror the real POSIX backend behaviour. fsys: fstest.MapFS{ "sample.jpg": {}, "photos/2006/January/sample.jpg": {}, @@ -98,7 +114,7 @@ func TestWalk(t *testing.T) { "photos/2006/February/sample3.jpg": {}, "photos/2006/February/sample4.jpg": {}, }, - getobj: getObj, + getobj: getObjSkipDirs, cases: []testcase{ { name: "aws example", @@ -153,12 +169,18 @@ func TestWalk(t *testing.T) { }, { // non-standard delimiter + // fstest.MapFS implicitly creates filesystem directories for each + // "/"-separated path component (e.g. "photo|s", "200|6", ...) even + // though these are not S3 objects. On a real POSIX filesystem the + // backend's FileToObj returns ErrSkipObj for such implicit + // directories (they have no stored etag), so we use getObjSkipDirs + // here to reproduce that behaviour. fsys: fstest.MapFS{ "photo|s/200|6/Januar|y/sampl|e1.jpg": {}, "photo|s/200|6/Januar|y/sampl|e2.jpg": {}, "photo|s/200|6/Januar|y/sampl|e3.jpg": {}, }, - getobj: getObj, + getobj: getObjSkipDirs, cases: []testcase{ { name: "different delimiter 1", @@ -235,6 +257,175 @@ func TestWalk(t *testing.T) { }, }, }, + { + // test case prefix is a top level directory object that + // should be returned with the listing results + fsys: fstest.MapFS{ + "a/file.txt": {}, + }, + getobj: getObj, + cases: []testcase{ + { + name: "prefix is a directory object", + maxObjs: 1000, + prefix: "a/", + expected: backend.WalkResults{ + Objects: []s3response.Object{ + { + Key: backend.GetPtrFromString("a/"), + }, + { + Key: backend.GetPtrFromString("a/file.txt"), + }, + }, + }, + }, + }, + }, + { + fsys: fstest.MapFS{ + "f3": {}, + "f4": {}, + "f5": {}, + "f6": {}, + }, + getobj: getObj, + cases: []testcase{ + { + name: "custom delimiter with marker filtering", + maxObjs: 1000, + delimiter: "5", + marker: "f3", + expected: backend.WalkResults{ + Objects: []s3response.Object{ + { + Key: backend.GetPtrFromString("f4"), + }, + { + Key: backend.GetPtrFromString("f6"), + }, + }, + CommonPrefixes: []types.CommonPrefix{ + { + Prefix: backend.GetPtrFromString("f5"), + }, + }, + }, + }, + }, + }, + { + // nested-path variant of delimiter+marker behavior. + fsys: fstest.MapFS{ + "top/alpha/f3": {}, + "top/bravo/f4": {}, + "top/charlie/f5": {}, + "top/zulu/f6": {}, + }, + getobj: getObjSkipDirs, + cases: []testcase{ + { + name: "delimiter and marker across nested directories", + maxObjs: 1000, + prefix: "top/", + delimiter: "5", + marker: "top/alpha/f3", + expected: backend.WalkResults{ + Objects: []s3response.Object{ + { + Key: backend.GetPtrFromString("top/bravo/f4"), + }, + { + Key: backend.GetPtrFromString("top/zulu/f6"), + }, + }, + CommonPrefixes: []types.CommonPrefix{ + { + Prefix: backend.GetPtrFromString("top/charlie/f5"), + }, + }, + }, + }, + }, + }, + { + // When the fast-path jumps into a prefix directory + // (e.g. prefix="a/"), the prefix-directory object "a/" must itself be + // offered to getObj even when the directory contains subdirectories. + // AWS S3 behavior: if "a/" was explicitly PUT as a zero-byte object it + // appears in ListObjects results regardless of what children it has. + fsys: fstest.MapFS{ + "a/b/file.txt": {}, + "a/file1.txt": {}, + }, + getobj: getObj, + cases: []testcase{ + { + name: "prefix dir with subdirs is offered to getObj", + prefix: "a/", + maxObjs: 1000, + expected: backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("a/")}, + {Key: backend.GetPtrFromString("a/b/")}, + {Key: backend.GetPtrFromString("a/b/file.txt")}, + {Key: backend.GetPtrFromString("a/file1.txt")}, + }, + }, + }, + }, + }, + { + // When the fast-path jumps into a prefix directory, + // "a/" must still be offered to getObj when a marker is present but + // the marker sorts before "a/" — so "a/" is a valid result on this page. + // AWS S3 behavior: ListObjects(prefix="a/", marker="a") must return + // "a/" as the first key because "a" < "a/". + fsys: fstest.MapFS{ + "a/file.txt": {}, + }, + getobj: getObj, + cases: []testcase{ + { + // marker "a" < "a/" → "a/" belongs on this page + name: "prefix dir included when marker is before dir key", + prefix: "a/", + marker: "a", + maxObjs: 1000, + expected: backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("a/")}, + {Key: backend.GetPtrFromString("a/file.txt")}, + }, + }, + }, + { + // marker "a/" == "a/" → "a/" itself must be excluded + // (pagination: previous page ended at "a/", results start after it) + name: "prefix dir excluded when marker equals dir key", + prefix: "a/", + marker: "a/", + maxObjs: 1000, + expected: backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("a/file.txt")}, + }, + }, + }, + { + // marker "a/b" > "a/" → "a/" belongs on a previous page, must be excluded + name: "prefix dir excluded when marker is after dir key", + prefix: "a/", + marker: "a/b", + maxObjs: 1000, + expected: backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("a/file.txt")}, + }, + }, + }, + }, + }, } for _, tt := range tests { @@ -276,11 +467,11 @@ func compareCommonPrefix(a, b []types.CommonPrefix) bool { } for _, cp := range a { - if containsCommonPrefix(cp, b) { - return true + if !containsCommonPrefix(cp, b) { + return false } } - return false + return true } func containsCommonPrefix(c types.CommonPrefix, list []types.CommonPrefix) bool { @@ -313,11 +504,11 @@ func compareObjects(a, b []s3response.Object) bool { } for _, cp := range a { - if containsObject(cp, b) { - return true + if !containsObject(cp, b) { + return false } } - return false + return true } func containsObject(c s3response.Object, list []s3response.Object) bool { @@ -365,8 +556,14 @@ func (s *slowFS) ReadDir(name string) ([]fs.DirEntry, error) { } func TestWalkStop(t *testing.T) { + // Path must not start with "/" (invalid for fstest.MapFS) and must be deep + // enough that reading every level takes longer than walkTimeOut. + // readDirPause (100 ms) × 14 levels = 1.4 s > walkTimeOut (500 ms). + // An empty delimiter is used so that the walk recurses into every directory + // level rather than stopping at the first common-prefix boundary (which is + // what a "/" delimiter would do, completing in a single ReadDir call). s := &slowFS{MapFS: fstest.MapFS{ - "/a/b/c/d/e/f/g/h/i/g/k/l/m/n": &fstest.MapFile{}, + "a/b/c/d/e/f/g/h/i/j/k/l/m/n": &fstest.MapFile{}, }} ctx, cancel := context.WithTimeout(context.Background(), walkTimeOut) @@ -375,7 +572,7 @@ func TestWalkStop(t *testing.T) { var err error var wg sync.WaitGroup wg.Go(func() { - _, err = backend.Walk(ctx, s, "", "/", "", 1000, + _, err = backend.Walk(ctx, s, "", "", "", 1000, func(path string, d fs.DirEntry) (s3response.Object, error) { return s3response.Object{}, nil }, []string{}) @@ -392,6 +589,53 @@ func TestWalkStop(t *testing.T) { } } +func TestWalkDirectoryErrorPropagates(t *testing.T) { + fSys := fstest.MapFS{ + "dir/file.txt": {}, + } + + _, err := backend.Walk(context.Background(), fSys, "", "", "", 1000, + func(path string, d fs.DirEntry) (s3response.Object, error) { + if d.IsDir() { + return s3response.Object{}, fmt.Errorf("boom") + } + return getObj(path, d) + }, []string{}) + if err == nil { + t.Fatal("expected error, got nil") + } + if !strings.Contains(err.Error(), "directory to object \"dir/\": boom") { + t.Fatalf("unexpected error: %v", err) + } +} + +func TestWalkDirectoryErrSkipObjContinues(t *testing.T) { + fSys := fstest.MapFS{ + "a/file1.txt": {}, + "b.txt": {}, + } + + res, err := backend.Walk(context.Background(), fSys, "", "", "", 1000, + func(path string, d fs.DirEntry) (s3response.Object, error) { + if d.IsDir() { + return s3response.Object{}, backend.ErrSkipObj + } + return getObj(path, d) + }, []string{}) + if err != nil { + t.Fatalf("walk: %v", err) + } + + expected := backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("a/file1.txt")}, + {Key: backend.GetPtrFromString("b.txt")}, + }, + } + + compareResultsOrdered("dir errskipobj continues", res, expected, t) +} + // TestOrderWalk tests the lexicographic ordering of the object names // for the case where readdir sort order of a directory is different // than the lexicographic ordering of the full paths. The below has @@ -400,8 +644,46 @@ func TestWalkStop(t *testing.T) { // but if you consider the character that comes after a is "/", then // the "." should come before "/" in the lexicographic ordering: // a.b/, a/ +// +// getObjSkipDirs is used to mirror the real POSIX backend's behaviour: +// all directories here are implicit filesystem artefacts (no S3 object +// was explicitly PUT at any of these paths), so FileToObj returns +// ErrSkipObj for them. The ordering is still exercised through the leaf +// files: a.b/* sorts before a/*. func TestOrderWalk(t *testing.T) { tests := []walkTest{ + { + fsys: fstest.MapFS{ + "dir1/a/file1": {}, + "dir1/a/file2": {}, + "dir1/a/file3": {}, + "dir1/a.b/file1": {}, + "dir1/a.b/file2": {}, + "dir1/b": {}, + "dir1/b./a": {}, + "dir1/b..": {}, + }, + getobj: getObjSkipDirs, + cases: []testcase{ + { + name: "order test", + maxObjs: 1000, + prefix: "dir1/", + expected: backend.WalkResults{ + Objects: []s3response.Object{ + {Key: backend.GetPtrFromString("dir1/a.b/file1")}, + {Key: backend.GetPtrFromString("dir1/a.b/file2")}, + {Key: backend.GetPtrFromString("dir1/a/file1")}, + {Key: backend.GetPtrFromString("dir1/a/file2")}, + {Key: backend.GetPtrFromString("dir1/a/file3")}, + {Key: backend.GetPtrFromString("dir1/b")}, + {Key: backend.GetPtrFromString("dir1/b..")}, + {Key: backend.GetPtrFromString("dir1/b./a")}, + }, + }, + }, + }, + }, { fsys: fstest.MapFS{ "dir1/a/file1": {}, @@ -439,6 +721,12 @@ func TestOrderWalk(t *testing.T) { }, }, { + // The same fstest.MapFS implicit-directory problem as the + // non-standard delimiter tests in TestWalk: "|" is the S3 + // delimiter but "/" is the filesystem separator, so fstest + // creates implicit real directories (dir|1, dir|1/a, …) that + // should not appear as S3 objects. Use getObjSkipDirs to + // mirror the POSIX backend's behaviour. fsys: fstest.MapFS{ "dir|1/a/file1": {}, "dir|1/a/file2": {}, @@ -446,7 +734,7 @@ func TestOrderWalk(t *testing.T) { "dir|1/a.b/file1": {}, "dir|1/a.b/file2": {}, }, - getobj: getObj, + getobj: getObjSkipDirs, cases: []testcase{ { name: "order test delim", @@ -543,13 +831,15 @@ type markertestcase struct { func TestMarker(t *testing.T) { tests := []markerTest{ { + // dir is an implicit filesystem directory; use getObjSkipDirs to + // match the real POSIX backend (no etag stored → ErrSkipObj). fsys: fstest.MapFS{ "dir/sample2.jpg": {}, "dir/sample3.jpg": {}, "dir/sample4.jpg": {}, "dir/sample5.jpg": {}, }, - getobj: getObj, + getobj: getObjSkipDirs, cases: []markertestcase{ { name: "multi page marker", @@ -583,13 +873,15 @@ func TestMarker(t *testing.T) { }, }, { + // dir1 is an implicit filesystem directory; use getObjSkipDirs to + // match the real POSIX backend (no etag stored → ErrSkipObj). fsys: fstest.MapFS{ "dir1/subdir/file.txt": {}, "dir1/subdir.ext": {}, "dir1/subdir1.ext": {}, "dir1/subdir2.ext": {}, }, - getobj: getObj, + getobj: getObjSkipDirs, cases: []markertestcase{ { name: "integration test case 1", diff --git a/backend/walker.go b/backend/walker.go index ec6dd24c..6e9394fe 100644 --- a/backend/walker.go +++ b/backend/walker.go @@ -11,17 +11,17 @@ import ( "slices" ) -// WalkDirFunc is the type of the function called by [WalkDir] to visit +// WalkDirFunc is the type of the function called by [walkDirRoot] to visit // each file or directory. // -// The path argument contains the argument to [WalkDir] as a prefix. +// The path argument contains the argument to [walkDirRoot] as a prefix. // That is, if WalkDir is called with root argument "dir" and finds a file // named "a" in that directory, the walk function will be called with // argument "dir/a". // // The d argument is the [DirEntry] for the named path. // -// The error result returned by the function controls how [WalkDir] +// The error result returned by the function controls how [walkDirRoot] // continues. If the function returns the special value [SkipDir], WalkDir // skips the current directory (path if d.IsDir() is true, otherwise // path's parent directory). If the function returns the special value @@ -30,11 +30,11 @@ import ( // returns that error. // // The err argument reports an error related to path, signaling that -// [WalkDir] will not walk into that directory. The function can decide how +// [walkDirRoot] will not walk into that directory. The function can decide how // to handle that error; as described earlier, returning the error will // cause WalkDir to stop walking the entire tree. // -// [WalkDir] calls the function with a non-nil err argument in two cases. +// [walkDirRoot] calls the function with a non-nil err argument in two cases. // // First, if the initial [Stat] on the root directory fails, WalkDir // calls the function with path set to root, d set to nil, and err set to @@ -108,19 +108,19 @@ func walkDir(fsys fs.FS, name string, d fs.DirEntry, walkDirFn WalkDirFunc) erro return nil } -// WalkDir walks the file tree rooted at root, calling fn for each file or +// walkDirRoot walks the file tree rooted at root, calling fn for each file or // directory in the tree, including root. // // All errors that arise visiting files and directories are filtered by fn: // see the [fs.WalkDirFunc] documentation for details. // // The files are walked in lexical order, which makes the output deterministic -// but requires WalkDir to read an entire directory into memory before proceeding +// but requires walkDirRoot to read an entire directory into memory before proceeding // to walk that directory. // -// WalkDir does not follow symbolic links found in directories, +// walkDirRoot does not follow symbolic links found in directories, // but if root itself is a symbolic link, its target will be walked. -func WalkDir(fsys fs.FS, root string, fn WalkDirFunc) error { +func walkDirRoot(fsys fs.FS, root string, fn WalkDirFunc) error { info, err := fs.Stat(fsys, root) if err != nil { err = fn(root, nil, err)