diff --git a/TODO.md b/TODO.md index 62d1aee..b5307d3 100644 --- a/TODO.md +++ b/TODO.md @@ -22,6 +22,17 @@ the tag exists and is exercised; what is left is merging `next` to # Completed Steps +- 2026-10-06: Made a restore path argument select only that path and + what is beneath it + ([issue #223](https://git.eeqj.de/sneak/vaultik/issues/223)). The + lookup matched with SQL `LIKE`, so `/home/u/doc` also restored + `doc2`, `DOC` and `doc.txt.bak`, and a `_` or `%` in the path acted + as a wildcard. A backup used the same lookup to load the known files + of each configured path, so files of a longer sibling path were + counted as deleted. `FileRepository.ListUnderPath`, which replaces + `ListByPrefix`, returns the file at the path and every file whose + path starts with the path plus `/`, compared case-sensitively. + - 2026-10-06: Made restore return an error instead of panicking on a malformed snapshot database ([issue #231](https://git.eeqj.de/sneak/vaultik/issues/231)). A chunk diff --git a/internal/database/files.go b/internal/database/files.go index c30c07e..02bd26c 100644 --- a/internal/database/files.go +++ b/internal/database/files.go @@ -228,19 +228,24 @@ func (r *FileRepository) DeleteByID( return nil } -// ListByPrefix returns all files whose path starts with prefix, ordered by -// path. -func (r *FileRepository) ListByPrefix( - ctx context.Context, prefix string, +// ListUnderPath returns the file at path and every file beneath it, +// ordered by path. Paths are compared case-sensitively, and a trailing +// slash on path is ignored, so "/" lists every file. +func (r *FileRepository) ListUnderPath( + ctx context.Context, path string, ) ([]*File, error) { + path = strings.TrimRight(path, "/") + dirPrefix := path + "/" + + // LIKE would ignore ASCII case and treat _ and % in path as wildcards. query := ` SELECT id, path, source_path, mtime, size, mode, uid, gid, link_target FROM files - WHERE path LIKE ? || '%' + WHERE path = ? OR substr(path, 1, length(?)) = ? ORDER BY path ` - rows, err := r.db.conn.QueryContext(ctx, query, prefix) + rows, err := r.db.conn.QueryContext(ctx, query, path, dirPrefix, dirPrefix) if err != nil { return nil, fmt.Errorf("querying files: %w", err) } diff --git a/internal/database/files_test.go b/internal/database/files_test.go index 2d4823b..23f33b1 100644 --- a/internal/database/files_test.go +++ b/internal/database/files_test.go @@ -5,10 +5,12 @@ import ( "database/sql" "errors" "os" + "slices" "testing" "time" "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/types" ) // errTestRollback is the sentinel returned from transaction bodies to @@ -134,6 +136,82 @@ func TestFileRepositoryListDelete(t *testing.T) { } } +func TestFileRepositoryListUnderPath(t *testing.T) { + t.Parallel() + + db, cleanup := setupTestDB(t) + defer cleanup() + + ctx := context.Background() + repo := database.NewFileRepository(db) + + const ( + docDir = "/home/u/doc" + docFile = "/home/u/doc/a.txt" + ) + + // In path order, so the root case can expect all of them as listed. + paths := []string{ + "/home/u/50%/x.txt", + "/home/u/50percent/y.txt", + "/home/u/DOC/c.txt", + "/home/u/a_b/x.txt", + "/home/u/axb/y.txt", + docDir, + "/home/u/doc.txt.bak", + docFile, + "/home/u/doc/sub/b.txt", + "/home/u/doc2/b.txt", + } + + for _, path := range paths { + err := repo.Create(ctx, nil, &database.File{ + Path: types.FilePath(path), + MTime: time.Now().Truncate(time.Second), + Mode: 0644, + }) + if err != nil { + t.Fatalf("failed to create %s: %v", path, err) + } + } + + docTree := []string{docDir, docFile, "/home/u/doc/sub/b.txt"} + + tests := []struct { + name string + path string + want []string + }{ + {"directory", docDir, docTree}, + {"directory with trailing slash", docDir + "/", docTree}, + {"directory differing only in case", "/home/u/DOC", + []string{"/home/u/DOC/c.txt"}}, + {"file", docFile, []string{docFile}}, + {"underscore is literal", "/home/u/a_b", + []string{"/home/u/a_b/x.txt"}}, + {"percent is literal", "/home/u/50%", + []string{"/home/u/50%/x.txt"}}, + {"root", "/", paths}, + } + + for _, tt := range tests { + files, err := repo.ListUnderPath(ctx, tt.path) + if err != nil { + t.Fatalf("%s: failed to list files: %v", tt.name, err) + } + + got := make([]string, 0, len(files)) + for _, f := range files { + got = append(got, f.Path.String()) + } + + if !slices.Equal(got, tt.want) { + t.Errorf("%s: listing %q got %q, want %q", + tt.name, tt.path, got, tt.want) + } + } +} + func TestFileRepositorySymlink(t *testing.T) { t.Parallel() diff --git a/internal/database/repository_comprehensive_test.go b/internal/database/repository_comprehensive_test.go index 4799458..53c5372 100644 --- a/internal/database/repository_comprehensive_test.go +++ b/internal/database/repository_comprehensive_test.go @@ -824,7 +824,7 @@ func TestTransactionIsolation(t *testing.T) { } // Verify the file was not created (transaction rolled back) - files, err := repos.Files.ListByPrefix(ctx, "/tx-test") + files, err := repos.Files.ListUnderPath(ctx, "/tx-test.txt") if err != nil { t.Fatal(err) } @@ -916,7 +916,7 @@ func TestConcurrentOrphanedCleanup(t *testing.T) { } // Verify correct files were deleted - files, err := repos.Files.ListByPrefix(ctx, "/concurrent-") + files, err := repos.Files.ListAll(ctx) if err != nil { t.Fatal(err) } diff --git a/internal/database/repository_debug_test.go b/internal/database/repository_debug_test.go index 4a4e2aa..9c9ec02 100644 --- a/internal/database/repository_debug_test.go +++ b/internal/database/repository_debug_test.go @@ -147,7 +147,7 @@ func TestOrphanedFileCleanupDebug(t *testing.T) { t.Logf("Files count after cleanup: %d", count) // List remaining files - files, err := repos.Files.ListByPrefix(ctx, "/") + files, err := repos.Files.ListUnderPath(ctx, "/") if err != nil { t.Fatal(err) } diff --git a/internal/database/repository_edge_cases_test.go b/internal/database/repository_edge_cases_test.go index 3e8cfb2..a4cf9f7 100644 --- a/internal/database/repository_edge_cases_test.go +++ b/internal/database/repository_edge_cases_test.go @@ -442,12 +442,12 @@ func TestLargeDatasets(t *testing.T) { createLargeDatasetFiles(t, repos, snapshot.ID.String(), fileCount) }) - // Test ListByPrefix performance + // Test ListUnderPath performance //nolint:paralleltest // phases share one database and are order-dependent - t.Run("list by prefix performance", func(t *testing.T) { + t.Run("list under path performance", func(t *testing.T) { start := time.Now() - files, err := repos.Files.ListByPrefix(ctx, "/large/") + files, err := repos.Files.ListUnderPath(ctx, "/large/") if err != nil { t.Fatal(err) } @@ -472,7 +472,7 @@ func TestLargeDatasets(t *testing.T) { t.Logf("Cleaned up orphaned files in %v", time.Since(start)) // Verify correct number remain - files, err := repos.Files.ListByPrefix(ctx, "/large/") + files, err := repos.Files.ListUnderPath(ctx, "/large/") if err != nil { t.Fatal(err) } diff --git a/internal/snapshot/backup_test.go b/internal/snapshot/backup_test.go index 80e0b38..f970cfb 100644 --- a/internal/snapshot/backup_test.go +++ b/internal/snapshot/backup_test.go @@ -71,7 +71,7 @@ func verifyBackupFiles( ) { t.Helper() - files, err := repos.Files.ListByPrefix(ctx, "") + files, err := repos.Files.ListAll(ctx) if err != nil { t.Fatalf("Failed to list files: %v", err) } diff --git a/internal/snapshot/scanner.go b/internal/snapshot/scanner.go index f7eb40e..7800891 100644 --- a/internal/snapshot/scanner.go +++ b/internal/snapshot/scanner.go @@ -456,14 +456,16 @@ func (s *Scanner) finalizeScanResult(ctx context.Context, result *ScanResult) { result.EndTime = time.Now().UTC() } -// loadKnownFiles loads all known files from the database into a map for fast lookup -// This avoids per-file database queries during the scan phase +// loadKnownFiles loads the known files at and beneath path from the +// database into a map for fast lookup. Every loaded file the scan does +// not find is counted as deleted. This avoids per-file database queries +// during the scan phase. func (s *Scanner) loadKnownFiles( ctx context.Context, path string, ) (map[string]*database.File, error) { - files, err := s.repos.Files.ListByPrefix(ctx, path) + files, err := s.repos.Files.ListUnderPath(ctx, path) if err != nil { - return nil, fmt.Errorf("listing files by prefix: %w", err) + return nil, fmt.Errorf("listing files under %s: %w", path, err) } result := make(map[string]*database.File, len(files)) diff --git a/internal/snapshot/scanner_test.go b/internal/snapshot/scanner_test.go index a28b58c..632d045 100644 --- a/internal/snapshot/scanner_test.go +++ b/internal/snapshot/scanner_test.go @@ -71,7 +71,7 @@ func verifySimpleScanDatabase( t.Helper() // Verify files in database - includes regular files and directories - files, err := repos.Files.ListByPrefix(ctx, "/source") + files, err := repos.Files.ListUnderPath(ctx, "/source") if err != nil { t.Fatalf("failed to list files: %v", err) } diff --git a/internal/vaultik/integration_test.go b/internal/vaultik/integration_test.go index bf74e60..5bb5483 100644 --- a/internal/vaultik/integration_test.go +++ b/internal/vaultik/integration_test.go @@ -249,7 +249,7 @@ func verifyEndToEndBackupState( assert.Positive(t, blobUploads, "Should upload at least one blob") // Verify files in database - files, err := repos.Files.ListByPrefix(ctx, "/home/user") + files, err := repos.Files.ListUnderPath(ctx, "/home/user") require.NoError(t, err) // Count only regular files (not directories) regularFiles := 0 diff --git a/internal/vaultik/restore.go b/internal/vaultik/restore.go index 03129a5..c5843e8 100644 --- a/internal/vaultik/restore.go +++ b/internal/vaultik/restore.go @@ -830,10 +830,9 @@ func (v *Vaultik) getFilesToRestore( // Normalize the filter path filter = filepath.Clean(filter) - // Get files with this prefix - files, err := repos.Files.ListByPrefix(ctx, filter) + files, err := repos.Files.ListUnderPath(ctx, filter) if err != nil { - return nil, fmt.Errorf("listing files with prefix %s: %w", filter, err) + return nil, fmt.Errorf("listing files under %s: %w", filter, err) } for _, file := range files {