diff --git a/persistence/folder_repository.go b/persistence/folder_repository.go index 1a4caae57..74edf2c12 100644 --- a/persistence/folder_repository.go +++ b/persistence/folder_repository.go @@ -4,7 +4,9 @@ import ( "context" "encoding/json" "fmt" + "os" "slices" + "strings" "time" . "github.com/Masterminds/squirrel" @@ -97,16 +99,37 @@ func (r folderRepository) GetFolderUpdateInfo(lib model.Library, targetPaths ... Eq{"missing": false}, } - // If specific paths are requested, generate folder IDs and filter by them + // If specific paths are requested, include those folders and all their descendants if len(targetPaths) > 0 { + // Collect folder IDs for exact target folders and path conditions for descendants folderIDs := make([]string, 0, len(targetPaths)) - for _, path := range targetPaths { - if path == "" { - path = "." + pathConditions := make(Or, 0, len(targetPaths)*2) + + for _, targetPath := range targetPaths { + if targetPath == "" || targetPath == "." { + // Root path - include everything in this library + pathConditions = Or{} + folderIDs = nil + break } - folderIDs = append(folderIDs, model.FolderID(lib, path)) + // Clean the path to normalize it. Paths stored in the folder table do not have leading/trailing slashes. + cleanPath := strings.Trim(targetPath, string(os.PathSeparator)) + + // Include the target folder itself by ID + folderIDs = append(folderIDs, model.FolderID(lib, cleanPath)) + + // Include all descendants: folders whose path field equals or starts with the target path + // Note: Folder.Path is the directory path, so children have path = targetPath + pathConditions = append(pathConditions, Eq{"path": cleanPath}) + pathConditions = append(pathConditions, Like{"path": cleanPath + "/%"}) + } + + // Combine conditions: exact folder IDs OR descendant path patterns + if len(folderIDs) > 0 { + where = append(where, Or{Eq{"id": folderIDs}, pathConditions}) + } else if len(pathConditions) > 0 { + where = append(where, pathConditions) } - where = append(where, Eq{"id": folderIDs}) } sq := r.newSelect().Columns("id", "updated_at", "hash").Where(where) diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 20554fe97..b33814fdc 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -76,7 +76,7 @@ type scanJob struct { fs storage.MusicFS cw artwork.CacheWarmer lastUpdates map[string]model.FolderUpdateInfo - targetFolders []string // Specific folders to scan (non-recursive) + targetFolders []string // Specific folders to scan (including all descendants) lock sync.Mutex numFolders atomic.Int64 } diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 1e0614573..ecad6f501 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -719,7 +719,7 @@ var _ = Describe("Scanner", Ordered, func() { }) Describe("ScanFolders", func() { - It("scans only specified folders without recursion", func() { + It("scans specified folders recursively including all subdirectories", func() { rock := template(_t{"albumartist": "Rock Artist", "album": "Rock Album"}) jazz := template(_t{"albumartist": "Jazz Artist", "album": "Jazz Album"}) pop := template(_t{"albumartist": "Pop Artist", "album": "Pop Album"}) @@ -735,7 +735,7 @@ var _ = Describe("Scanner", Ordered, func() { // Use the existing library from BeforeEach // (lib is already created with the path "fake:///music") - // Scan only the "rock" and "jazz" folders (not their subdirectories or pop) + // Scan only the "rock" and "jazz" folders (including their subdirectories) targets := []scanner.ScanTarget{ {LibraryID: lib.ID, FolderPath: "rock"}, {LibraryID: lib.ID, FolderPath: "jazz"}, @@ -745,31 +745,29 @@ var _ = Describe("Scanner", Ordered, func() { Expect(err).ToNot(HaveOccurred()) Expect(warnings).To(BeEmpty()) - // Verify only track1, track2, and track4 were imported (not track3, track5, or track6) + // Verify all tracks in rock and jazz folders (including subdirectories) were imported allFiles, err := ds.MediaFile(ctx).GetAll() Expect(err).ToNot(HaveOccurred()) - // Should have exactly 3 tracks (rock/track1, rock/track2, jazz/track4) - Expect(allFiles).To(HaveLen(3)) + // Should have 5 tracks (all rock and jazz tracks including subdirectories) + Expect(allFiles).To(HaveLen(5)) // Get the file paths paths := slice.Map(allFiles, func(mf model.MediaFile) string { return filepath.ToSlash(mf.Path) }) - // Verify the correct files were scanned + // Verify the correct files were scanned (including subdirectories) Expect(paths).To(ContainElements( "rock/track1.mp3", "rock/track2.mp3", + "rock/subdir/track3.mp3", "jazz/track4.mp3", + "jazz/subdir/track5.mp3", )) - // Verify files in subdirectories and pop folder were NOT scanned - Expect(paths).ToNot(ContainElements( - "rock/subdir/track3.mp3", - "jazz/subdir/track5.mp3", - "pop/track6.mp3", - )) + // Verify files in the pop folder were NOT scanned + Expect(paths).ToNot(ContainElement("pop/track6.mp3")) }) }) }) diff --git a/scanner/selective_scan_test.go b/scanner/selective_scan_test.go new file mode 100644 index 000000000..a706af159 --- /dev/null +++ b/scanner/selective_scan_test.go @@ -0,0 +1,240 @@ +package scanner_test + +import ( + "context" + "path/filepath" + "testing/fstest" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/core/metrics" + "github.com/navidrome/navidrome/core/storage/storagetest" + "github.com/navidrome/navidrome/db" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + "github.com/navidrome/navidrome/persistence" + "github.com/navidrome/navidrome/scanner" + "github.com/navidrome/navidrome/server/events" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Selective Scan - Deleted Child Folders", Ordered, func() { + var ctx context.Context + var lib model.Library + var ds model.DataStore + var s scanner.Scanner + var fsys storagetest.FakeFS + + BeforeAll(func() { + ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "123", IsAdmin: true}) + tmpDir := GinkgoT().TempDir() + conf.Server.DbPath = filepath.Join(tmpDir, "test-selective-scan.db?_journal_mode=WAL") + log.Warn("Using DB at " + conf.Server.DbPath) + db.Db().SetMaxOpenConns(1) + }) + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.MusicFolder = "fake:///music" + conf.Server.DevExternalScanner = false + + db.Init(ctx) + DeferCleanup(func() { + Expect(tests.ClearDB()).To(Succeed()) + }) + + ds = persistence.New(db.Db()) + + // Create the admin user in the database to match the context + adminUser := model.User{ + ID: "123", + UserName: "admin", + Name: "Admin User", + IsAdmin: true, + NewPassword: "password", + } + Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) + + s = scanner.New(ctx, ds, artwork.NoopCacheWarmer(), events.NoopBroker(), + core.NewPlaylists(ds), metrics.NewNoopInstance()) + + lib = model.Library{ID: 1, Name: "Fake Library", Path: "fake:///music"} + Expect(ds.Library(ctx).Put(&lib)).To(Succeed()) + + // Initialize fake filesystem + fsys = storagetest.FakeFS{} + storagetest.Register("fake", &fsys) + }) + + Context("when a child folder is deleted", func() { + var ( + revolver, help func(...map[string]any) *fstest.MapFile + artistFolderID string + album1FolderID string + album2FolderID string + album1TrackIDs []string + album2TrackIDs []string + ) + + BeforeEach(func() { + // Setup template functions for creating test files + revolver = storagetest.Template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966}) + help = storagetest.Template(_t{"albumartist": "The Beatles", "album": "Help!", "year": 1965}) + + // Initial filesystem with nested folders + fsys.SetFiles(fstest.MapFS{ + "The Beatles/Revolver/01 - Taxman.mp3": revolver(storagetest.Track(1, "Taxman")), + "The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(storagetest.Track(2, "Eleanor Rigby")), + "The Beatles/Help!/01 - Help!.mp3": help(storagetest.Track(1, "Help!")), + "The Beatles/Help!/02 - The Night Before.mp3": help(storagetest.Track(2, "The Night Before")), + }) + + // First scan - import everything + _, err := s.ScanAll(ctx, true) + Expect(err).ToNot(HaveOccurred()) + + // Verify initial state - all folders exist + folders, err := ds.Folder(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"library_id": lib.ID}}) + Expect(err).ToNot(HaveOccurred()) + Expect(folders).To(HaveLen(4)) // root, Artist, Album1, Album2 + + // Store folder IDs for later verification + for _, f := range folders { + switch f.Name { + case "The Beatles": + artistFolderID = f.ID + case "Revolver": + album1FolderID = f.ID + case "Help!": + album2FolderID = f.ID + } + } + + // Verify all tracks exist + allTracks, err := ds.MediaFile(ctx).GetAll() + Expect(err).ToNot(HaveOccurred()) + Expect(allTracks).To(HaveLen(4)) + + // Store track IDs for later verification + for _, t := range allTracks { + if t.Album == "Revolver" { + album1TrackIDs = append(album1TrackIDs, t.ID) + } else if t.Album == "Help!" { + album2TrackIDs = append(album2TrackIDs, t.ID) + } + } + + // Verify no tracks are missing initially + for _, t := range allTracks { + Expect(t.Missing).To(BeFalse()) + } + }) + + It("should mark child folder and its tracks as missing when parent is scanned", func() { + // Delete the child folder (Help!) from the filesystem + fsys.SetFiles(fstest.MapFS{ + "The Beatles/Revolver/01 - Taxman.mp3": revolver(storagetest.Track(1, "Taxman")), + "The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(storagetest.Track(2, "Eleanor Rigby")), + // "The Beatles/Help!" folder and its contents are DELETED + }) + + // Run selective scan on the parent folder (Artist) + // This simulates what the watcher does when a child folder is deleted + _, err := s.ScanFolders(ctx, false, []scanner.ScanTarget{ + {LibraryID: lib.ID, FolderPath: "The Beatles"}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify the deleted child folder is now marked as missing + deletedFolder, err := ds.Folder(ctx).Get(album2FolderID) + Expect(err).ToNot(HaveOccurred()) + Expect(deletedFolder.Missing).To(BeTrue(), "Deleted child folder should be marked as missing") + + // Verify the deleted folder's tracks are marked as missing + for _, trackID := range album2TrackIDs { + track, err := ds.MediaFile(ctx).Get(trackID) + Expect(err).ToNot(HaveOccurred()) + Expect(track.Missing).To(BeTrue(), "Track in deleted folder should be marked as missing") + } + + // Verify the parent folder is still present and not marked as missing + parentFolder, err := ds.Folder(ctx).Get(artistFolderID) + Expect(err).ToNot(HaveOccurred()) + Expect(parentFolder.Missing).To(BeFalse(), "Parent folder should not be marked as missing") + + // Verify the sibling folder and its tracks are still present and not missing + siblingFolder, err := ds.Folder(ctx).Get(album1FolderID) + Expect(err).ToNot(HaveOccurred()) + Expect(siblingFolder.Missing).To(BeFalse(), "Sibling folder should not be marked as missing") + + for _, trackID := range album1TrackIDs { + track, err := ds.MediaFile(ctx).Get(trackID) + Expect(err).ToNot(HaveOccurred()) + Expect(track.Missing).To(BeFalse(), "Track in sibling folder should not be marked as missing") + } + }) + + It("should mark deeply nested child folders as missing", func() { + // Add a deeply nested folder structure + fsys.SetFiles(fstest.MapFS{ + "The Beatles/Revolver/01 - Taxman.mp3": revolver(storagetest.Track(1, "Taxman")), + "The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(storagetest.Track(2, "Eleanor Rigby")), + "The Beatles/Help!/01 - Help!.mp3": help(storagetest.Track(1, "Help!")), + "The Beatles/Help!/02 - The Night Before.mp3": help(storagetest.Track(2, "The Night Before")), + "The Beatles/Help!/Bonus/01 - Bonus Track.mp3": help(storagetest.Track(99, "Bonus Track")), + "The Beatles/Help!/Bonus/Nested/01 - Deep Track.mp3": help(storagetest.Track(100, "Deep Track")), + }) + + // Rescan to import the new nested structure + _, err := s.ScanAll(ctx, true) + Expect(err).ToNot(HaveOccurred()) + + // Verify nested folders were created + allFolders, err := ds.Folder(ctx).GetAll(model.QueryOptions{Filters: squirrel.Eq{"library_id": lib.ID}}) + Expect(err).ToNot(HaveOccurred()) + Expect(len(allFolders)).To(BeNumerically(">", 4), "Should have more folders with nested structure") + + // Now delete the entire Help! folder including nested children + fsys.SetFiles(fstest.MapFS{ + "The Beatles/Revolver/01 - Taxman.mp3": revolver(storagetest.Track(1, "Taxman")), + "The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(storagetest.Track(2, "Eleanor Rigby")), + // All Help! subfolders are deleted + }) + + // Run selective scan on parent + _, err = s.ScanFolders(ctx, false, []scanner.ScanTarget{ + {LibraryID: lib.ID, FolderPath: "The Beatles"}, + }) + Expect(err).ToNot(HaveOccurred()) + + // Verify all Help! folders (including nested ones) are marked as missing + missingFolders, err := ds.Folder(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.And{ + squirrel.Eq{"library_id": lib.ID}, + squirrel.Eq{"missing": true}, + }, + }) + Expect(err).ToNot(HaveOccurred()) + Expect(len(missingFolders)).To(BeNumerically(">", 0), "At least one folder should be marked as missing") + + // Verify all tracks in deleted folders are marked as missing + allTracks, err := ds.MediaFile(ctx).GetAll() + Expect(err).ToNot(HaveOccurred()) + Expect(allTracks).To(HaveLen(6)) + + for _, track := range allTracks { + if track.Album == "Help!" { + Expect(track.Missing).To(BeTrue(), "All tracks in deleted Help! folder should be marked as missing") + } else if track.Album == "Revolver" { + Expect(track.Missing).To(BeFalse(), "Tracks in Revolver folder should not be marked as missing") + } + } + }) + }) +}) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index d431fef23..59c7000e5 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -30,7 +30,7 @@ func walkDirTree(ctx context.Context, job *scanJob) (<-chan *folderEntry, error) return results, nil } -// loadSpecificFolders loads only the specified folders without recursing into subdirectories +// loadSpecificFolders loads the specified folders and recursively walks their subdirectories func loadSpecificFolders(ctx context.Context, job *scanJob, targetFolders []string) (<-chan *folderEntry, error) { results := make(chan *folderEntry) go func() { @@ -44,22 +44,14 @@ func loadSpecificFolders(ctx context.Context, job *scanJob, targetFolders []stri checker := newIgnoreChecker(job.fs) _ = checker.PushAllParents(ctx, folderPath) - // Load only this specific folder (no recursion) - folder, _, err := loadDir(ctx, job, folderPath, checker) + // Recursively walk this folder and all its children + err := walkFolder(ctx, job, folderPath, checker, results) if err != nil { - log.Warn(ctx, "Scanner: Error loading target folder. Skipping", "path", folderPath, err) + log.Error(ctx, "Scanner: Error walking target folder", "path", folderPath, err) continue } - - folder.path = path.Clean(folderPath) - folder.elapsed.Start() - log.Trace(ctx, "Scanner: Found target directory", " path", folder.path, "audioFiles", maps.Keys(folder.audioFiles), - "images", maps.Keys(folder.imageFiles), "playlists", folder.numPlaylists, "imagesUpdatedAt", folder.imagesUpdatedAt, - "updTime", folder.updTime, "modTime", folder.modTime) - - results <- folder } - log.Debug(ctx, "Scanner: Finished reading target folders", "lib", job.lib.Name, "path", job.lib.Path, "numFolders", len(targetFolders)) + log.Debug(ctx, "Scanner: Finished reading target folders", "lib", job.lib.Name, "path", job.lib.Path, "numFolders", job.numFolders.Load()) }() return results, nil } diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 1cab8a0b7..840cd5dd2 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -103,6 +103,92 @@ var _ = Describe("walk_dir_tree", func() { ) }) + Describe("loadSpecificFolders", func() { + var ( + fsys storage.MusicFS + job *scanJob + ctx context.Context + ) + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + ctx = GinkgoT().Context() + fsys = &mockMusicFS{ + FS: fstest.MapFS{ + "Artist/Album1/track1.mp3": {}, + "Artist/Album1/track2.mp3": {}, + "Artist/Album2/track1.mp3": {}, + "Artist/Album2/track2.mp3": {}, + "Artist/Album2/Sub/track3.mp3": {}, + "OtherArtist/Album3/track1.mp3": {}, + }, + } + job = &scanJob{ + fs: fsys, + lib: model.Library{Path: "/music"}, + } + }) + + It("should recursively walk all subdirectories of target folders", func() { + results, err := loadSpecificFolders(ctx, job, []string{"Artist"}) + Expect(err).ToNot(HaveOccurred()) + + folders := map[string]*folderEntry{} + g := errgroup.Group{} + g.Go(func() error { + for folder := range results { + folders[folder.path] = folder + } + return nil + }) + _ = g.Wait() + + // Should include the target folder and all its descendants + Expect(folders).To(SatisfyAll( + HaveKey("Artist"), + HaveKey("Artist/Album1"), + HaveKey("Artist/Album2"), + HaveKey("Artist/Album2/Sub"), + )) + + // Should not include folders outside the target + Expect(folders).ToNot(HaveKey("OtherArtist")) + Expect(folders).ToNot(HaveKey("OtherArtist/Album3")) + + // Verify audio files are present + Expect(folders["Artist/Album1"].audioFiles).To(HaveLen(2)) + Expect(folders["Artist/Album2"].audioFiles).To(HaveLen(2)) + Expect(folders["Artist/Album2/Sub"].audioFiles).To(HaveLen(1)) + }) + + It("should handle multiple target folders", func() { + results, err := loadSpecificFolders(ctx, job, []string{"Artist/Album1", "OtherArtist"}) + Expect(err).ToNot(HaveOccurred()) + + folders := map[string]*folderEntry{} + g := errgroup.Group{} + g.Go(func() error { + for folder := range results { + folders[folder.path] = folder + } + return nil + }) + _ = g.Wait() + + // Should include both target folders and their descendants + Expect(folders).To(SatisfyAll( + HaveKey("Artist/Album1"), + HaveKey("OtherArtist"), + HaveKey("OtherArtist/Album3"), + )) + + // Should not include other folders + Expect(folders).ToNot(HaveKey("Artist")) + Expect(folders).ToNot(HaveKey("Artist/Album2")) + Expect(folders).ToNot(HaveKey("Artist/Album2/Sub")) + }) + }) + Describe("helper functions", func() { dir, _ := os.Getwd() fsys := os.DirFS(dir)