List a missing file:// destination directory as an error (closes #220)
The file backend listed a destination directory that does not exist as an empty store. With the volume unplugged, snapshot list reported every local snapshot as missing from the store, snapshot remove said it had removed metadata it never reached, and prune dropped every local snapshot record. List and ListStream now fail when the destination directory is missing, so those commands take their existing path for a store that cannot be listed. A missing prefix under an existing directory is still an empty listing, and a first backup still creates the directory. Three tests listed a file:// destination nothing had created; they now create it. Model: opus-5-5
This commit was merged in pull request #238.
This commit is contained in:
@@ -22,11 +22,11 @@ type FileStorer struct {
|
||||
//
|
||||
// Construction is intentionally cheap and does not touch the filesystem.
|
||||
// The basePath is recorded; the directory is created lazily on first
|
||||
// write. Reads (Get/Stat/List) tolerate a missing basePath — a missing
|
||||
// or unmounted destination during `snapshot list` should NOT block the
|
||||
// command, it should degrade to "no remote snapshots reachable" with a
|
||||
// warning. Write operations (Put/PutWithProgress) call MkdirAll for the
|
||||
// write. Write operations (Put/PutWithProgress) call MkdirAll for the
|
||||
// per-blob parent directory, which also covers basePath on first use.
|
||||
// Get and Stat report a key under a missing basePath as ErrNotFound.
|
||||
// List and ListStream fail on a missing basePath, because listing it as
|
||||
// an empty store would make `prune` drop every local snapshot record.
|
||||
//
|
||||
// Uses the real OS filesystem by default; call SetFilesystem to
|
||||
// override for testing.
|
||||
@@ -119,13 +119,20 @@ func (f *FileStorer) Delete(_ context.Context, key string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// List returns all keys with the given prefix.
|
||||
// List returns all keys with the given prefix. It fails when the
|
||||
// destination directory is missing; a missing prefix under it is an
|
||||
// empty listing.
|
||||
func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error) {
|
||||
var keys []string
|
||||
|
||||
_, err := f.fs.Stat(f.basePath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("checking destination directory: %w", err)
|
||||
}
|
||||
|
||||
basePath := f.fullPath(prefix)
|
||||
|
||||
// Check if base path exists
|
||||
// Check if the prefix exists
|
||||
exists, err := afero.Exists(f.fs, basePath)
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("checking path: %w", err)
|
||||
@@ -167,16 +174,24 @@ func (f *FileStorer) List(ctx context.Context, prefix string) ([]string, error)
|
||||
return keys, nil
|
||||
}
|
||||
|
||||
// ListStream returns a channel of ObjectInfo for large result sets.
|
||||
// ListStream returns a channel of ObjectInfo for large result sets. Like
|
||||
// List, it sends an error when the destination directory is missing.
|
||||
func (f *FileStorer) ListStream(ctx context.Context, prefix string) <-chan ObjectInfo {
|
||||
ch := make(chan ObjectInfo)
|
||||
|
||||
go func() {
|
||||
defer close(ch)
|
||||
|
||||
_, err := f.fs.Stat(f.basePath)
|
||||
if err != nil {
|
||||
ch <- ObjectInfo{Err: fmt.Errorf("checking destination directory: %w", err)}
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
basePath := f.fullPath(prefix)
|
||||
|
||||
// Check if base path exists
|
||||
// Check if the prefix exists
|
||||
exists, err := afero.Exists(f.fs, basePath)
|
||||
if err != nil {
|
||||
ch <- ObjectInfo{Err: fmt.Errorf("checking path: %w", err)}
|
||||
|
||||
@@ -1,6 +1,10 @@
|
||||
package storage_test
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"io/fs"
|
||||
"path/filepath"
|
||||
"testing"
|
||||
|
||||
"sneak.berlin/go/vaultik/internal/storage"
|
||||
@@ -25,3 +29,34 @@ func TestFileStorer(t *testing.T) {
|
||||
t.Parallel()
|
||||
runStorerConformance(t, newFileStorer)
|
||||
}
|
||||
|
||||
// TestFileStorerListMissingDestination checks that List and ListStream
|
||||
// fail when the destination directory does not exist, instead of
|
||||
// reporting an empty store.
|
||||
func TestFileStorerListMissingDestination(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
ctx := context.Background()
|
||||
|
||||
s, err := storage.NewFileStorer(filepath.Join(t.TempDir(), "unmounted"))
|
||||
if err != nil {
|
||||
t.Fatalf("NewFileStorer: %v", err)
|
||||
}
|
||||
|
||||
keys, err := s.List(ctx, "metadata/")
|
||||
if !errors.Is(err, fs.ErrNotExist) {
|
||||
t.Errorf("List = %v, %v; want a not-exist error", keys, err)
|
||||
}
|
||||
|
||||
var streamErr error
|
||||
|
||||
for object := range s.ListStream(ctx, "metadata/") {
|
||||
if object.Err != nil {
|
||||
streamErr = object.Err
|
||||
}
|
||||
}
|
||||
|
||||
if !errors.Is(streamErr, fs.ErrNotExist) {
|
||||
t.Errorf("ListStream error = %v, want a not-exist error", streamErr)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user