diff --git a/cmd/server/tests/archive_multifolder_test.go b/cmd/server/tests/archive_multifolder_test.go new file mode 100644 index 00000000..14126c15 --- /dev/null +++ b/cmd/server/tests/archive_multifolder_test.go @@ -0,0 +1,87 @@ +package main + +// Regression tests for the archive reconciliation pass: a multi-folder +// library must be reconciled against the UNION of its folders' walks, not +// once per folder. The old per-folder loop listed every library item +// against each single folder's walk, so with two registered folders every +// item living under the other folder was flagged "missing (1/2)" on every +// scan (prod: 743/743 audiobooks stuck at count 1 — unarchivable only +// because each scan's walk kept clearing them), and with three or more +// folders the crossed marks would have reached the archive threshold +// mid-pass and archived real content. Folder-book roots (directory +// file_path) must also count as present across folder boundaries — the +// file walk never emits a directory. + +import ( + "context" + "fmt" + "path/filepath" + "testing" + "time" + + "bookhoard/internal/database" + "bookhoard/internal/services" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// scanTestFolders runs a synchronous scan over several folders — the +// multi-folder shape the archive pass must get right (scanTestFolder +// covers the single-folder case). +func scanTestFolders(t *testing.T, setup *TestServerSetup, adminID pgtype.UUID, folders []string) { + t.Helper() + scanner := services.NewMediaScanner(setup.DB) + defer func() { _ = scanner.Close() }() + scanner.SetAdminID(adminID) + require.NoError(t, scanner.SetFolders(folders, false)) + require.NoError(t, scanner.ScanFolders(context.Background())) +} + +func TestArchivePassMultiFolderLibraryKeepsEverythingPresent(t *testing.T) { + setup := setupTestServer(t) + t.Cleanup(func() { _ = setup.Close() }) + + libID, folderA := createTypedTestLibrary(t, setup, fmt.Sprintf("Test Archive MultiFolder %d", time.Now().UnixNano()), "audiobooks") + t.Cleanup(func() { deleteLibraries(t, setup, libID) }) // after setup.Close defer (LIFO): pool still open + + // Second registered folder in the SAME library — the shape the + // per-folder reconciliation got wrong. + folderB := t.TempDir() + _, err := setup.DB.AddLibraryFolder(context.Background(), database.AddLibraryFolderParams{ + LibraryID: libID, + FolderPath: folderB, + }) + require.NoError(t, err) + + admin, _ := moveTestAdminID(t, setup) + + // Folder A holds a lone single-file book; folder B holds a + // folder-of-tracks book whose file_path is a DIRECTORY. + writeTestTrack(t, filepath.Join(folderA, "Author A", "Lone Book", "lonely.mp3")) + writeTestTrack(t, filepath.Join(folderB, "Author B", "Folder Book", "01 First.mp3")) + writeTestTrack(t, filepath.Join(folderB, "Author B", "Folder Book", "02 Second.mp3")) + + // Two consecutive scans: the first proves no flags are raised, the + // second proves the flag/walk bounce (clear → re-flag) is gone too. + for scanNo := 1; scanNo <= 2; scanNo++ { + scanTestFolders(t, setup, admin, []string{folderA, folderB}) + + items := listLibraryAudioItems(t, setup, libID) + var singleFile, folderBook bool + for _, item := range items { + assert.False(t, item.ArchivedAt.Valid, "scan %d: item %q must not be archived", scanNo, item.FilePath) + assert.Equal(t, int32(0), item.MissingScanCount, "scan %d: item %q must not be flagged missing", scanNo, item.FilePath) + switch item.FilePath { + case "Author A/Lone Book/lonely.mp3": + singleFile = true + case "Author B/Folder Book": + folderBook = true + } + } + require.True(t, singleFile, "scan %d: single-file book must exist", scanNo) + require.True(t, folderBook, "scan %d: folder book must exist at its directory path", scanNo) + require.Len(t, items, 2, "scan %d: exactly the two books, no per-track strays", scanNo) + } +} diff --git a/internal/services/media_scanner.go b/internal/services/media_scanner.go index 6a563766..f3ca45b4 100644 --- a/internal/services/media_scanner.go +++ b/internal/services/media_scanner.go @@ -831,18 +831,47 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { // retention window (ARCHIVE_RETENTION_DAYS; 0 = manual purge only). // Every branch logs - the previous hard-delete cleanup failed silently // and left orphaned rows undetected. + // Group this scan's folders by owning library: the reconciliation is + // per LIBRARY, comparing every library item against the union of all + // its folders' walks. The old per-folder loop listed the whole + // library's items against each single folder's walk, so with two + // folders every item living under the other folder was flagged + // missing on every scan (prod: 743/743 audiobooks stuck at + // "missing (1/2)", unarchivable only because the walk kept clearing + // them) — and with three or more folders earlier iterations' marks + // crossed later ones' walks and archived real content mid-pass. + scannedSet := make(map[string]bool, len(s.folders)) + for _, scanned := range s.folders { + scannedSet[scanned] = true + } + type archiveScope struct { + libraryID pgtype.UUID + name string + folders []string + } + scopes := make([]archiveScope, 0, len(s.folders)) + scopeIndex := make(map[pgtype.UUID]int, len(s.folders)) for _, folder := range s.folders { + lib, err := s.db.GetLibraryByFolder(ctx, folder) + if err != nil { + fmt.Printf("[ARCHIVE] Warning: no library found for folder %s, skipping it in archive pass: %v\n", folder, err) + continue + } + idx, seen := scopeIndex[lib.LibraryID] + if !seen { + idx = len(scopes) + scopeIndex[lib.LibraryID] = idx + scopes = append(scopes, archiveScope{libraryID: lib.LibraryID, name: lib.Name}) + } + scopes[idx].folders = append(scopes[idx].folders, folder) + } + + for _, scope := range scopes { if ctx.Err() != nil { fmt.Printf("[ARCHIVE] Scan cancelled, stopping archive pass\n") return ctx.Err() } - - lib, err := s.db.GetLibraryByFolder(ctx, folder) - if err != nil { - fmt.Printf("[ARCHIVE] Warning: no library found for folder %s, skipping archive pass: %v\n", folder, err) - continue - } - libraryID := lib.LibraryID + libraryID := scope.libraryID // Partial-scope guard (per-folder scans, 2026-10): the // reconciliation compares EVERY library item against this scan's @@ -855,10 +884,6 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { fmt.Printf("[ARCHIVE] Warning: failed to list folders for library, skipping archive pass: %v\n", regErr) continue } - scannedSet := make(map[string]bool, len(s.folders)) - for _, scanned := range s.folders { - scannedSet[scanned] = true - } covered := 0 for _, rf := range registeredFolders { if scannedSet[rf.FolderPath] { @@ -867,7 +892,7 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { } if covered < len(registeredFolders) { fmt.Printf("[ARCHIVE] Skipping reconciliation for %s: scan covered %d of %d registered folders\n", - lib.Name, covered, len(registeredFolders)) + scope.name, covered, len(registeredFolders)) continue } @@ -877,26 +902,39 @@ func (s *MediaScanner) ScanFolders(ctx context.Context) error { continue } - // Build set of scanned file paths for this folder + // Build the set of scanned paths across ALL of this library's + // folders. scannedPaths := make(map[string]bool) - if err := filepath.WalkDir(folder, func(path string, d os.DirEntry, walkErr error) error { - if walkErr != nil { - return walkErr + walkFailed := false + for _, folder := range scope.folders { + if err := filepath.WalkDir(folder, func(path string, d os.DirEntry, walkErr error) error { + if walkErr != nil { + return walkErr + } + if !d.IsDir() && s.isScannableFile(path) { + scannedPaths[s.getRelativePath(path)] = true + } + return nil + }); err != nil { + fmt.Printf("[ARCHIVE] Warning: failed to walk directory %s, skipping archive pass: %v\n", folder, err) + walkFailed = true + break } - if !d.IsDir() && s.isScannableFile(path) { - scannedPaths[s.getRelativePath(path)] = true - } - return nil - }); err != nil { - fmt.Printf("[ARCHIVE] Warning: failed to walk directory %s, skipping archive pass: %v\n", folder, err) - continue // Skip to next folder to avoid false archivals + } + if walkFailed { + continue // Skip this library to avoid false archivals } // Folder-book roots count as present: the item's file_path is the - // DIRECTORY, which the file walk above never emits. + // DIRECTORY, which the file walk above never emits. Attribution is + // boundary-safe — a sibling folder whose name merely extends this + // one's must not contribute its roots here. for root := range s.audioBookRoots { - if strings.HasPrefix(root, folder) { - scannedPaths[s.getRelativePath(root)] = true + for _, folder := range scope.folders { + if folderContains(folder, root) { + scannedPaths[s.getRelativePath(root)] = true + break + } } }