diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 52bed9fde..2dd5c6639 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -140,10 +140,8 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] { // Use selective folder loading if target folders are specified if len(job.targetFolders) > 0 { log.Debug(p.ctx, "Scanner: Loading specific folders only (non-recursive)", "lib", job.lib.Name, "numTargets", len(job.targetFolders)) - outputChan, err = loadSpecificFolders(p.ctx, job, job.targetFolders) - } else { - outputChan, err = walkDirTree(p.ctx, job) } + outputChan, err = walkDirTree(p.ctx, job, job.targetFolders...) if err != nil { log.Warn(p.ctx, "Scanner: Error scanning library", "lib", job.lib.Name, err) diff --git a/scanner/walk_dir_tree.go b/scanner/walk_dir_tree.go index 59c7000e5..8500824cf 100644 --- a/scanner/walk_dir_tree.go +++ b/scanner/walk_dir_tree.go @@ -15,27 +15,19 @@ import ( "github.com/navidrome/navidrome/utils" ) -func walkDirTree(ctx context.Context, job *scanJob) (<-chan *folderEntry, error) { +// walkDirTree recursively walks the directory tree starting from the given targetFolders. +// If no targetFolders are provided, it starts from the root folder ("."). +// It returns a channel of folderEntry pointers representing each folder found. +func walkDirTree(ctx context.Context, job *scanJob, targetFolders ...string) (<-chan *folderEntry, error) { results := make(chan *folderEntry) + folders := targetFolders + if len(targetFolders) == 0 { + // No specific folders provided, scan the root folder + folders = []string{"."} + } go func() { defer close(results) - checker := newIgnoreChecker(job.fs) - err := walkFolder(ctx, job, ".", checker, results) - if err != nil { - log.Error(ctx, "Scanner: There were errors reading directories from filesystem", "path", job.lib.Path, err) - return - } - log.Debug(ctx, "Scanner: Finished reading folders", "lib", job.lib.Name, "path", job.lib.Path, "numFolders", job.numFolders.Load()) - }() - return results, nil -} - -// 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() { - defer close(results) - for _, folderPath := range targetFolders { + for _, folderPath := range folders { if utils.IsCtxDone(ctx) { return } diff --git a/scanner/walk_dir_tree_test.go b/scanner/walk_dir_tree_test.go index 840cd5dd2..5cd1f3ab6 100644 --- a/scanner/walk_dir_tree_test.go +++ b/scanner/walk_dir_tree_test.go @@ -25,167 +25,163 @@ var _ = Describe("walk_dir_tree", func() { ctx context.Context ) - BeforeEach(func() { - DeferCleanup(configtest.SetupConfig()) - ctx = GinkgoT().Context() - fsys = &mockMusicFS{ - FS: fstest.MapFS{ - "root/a/.ndignore": {Data: []byte("ignored/*")}, - "root/a/f1.mp3": {}, - "root/a/f2.mp3": {}, - "root/a/ignored/bad.mp3": {}, - "root/b/cover.jpg": {}, - "root/c/f3": {}, - "root/d": {}, - "root/d/.ndignore": {}, - "root/d/f1.mp3": {}, - "root/d/f2.mp3": {}, - "root/d/f3.mp3": {}, - "root/e/original/f1.mp3": {}, - "root/e/symlink": {Mode: fs.ModeSymlink, Data: []byte("original")}, + Context("full library", func() { + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + ctx = GinkgoT().Context() + fsys = &mockMusicFS{ + FS: fstest.MapFS{ + "root/a/.ndignore": {Data: []byte("ignored/*")}, + "root/a/f1.mp3": {}, + "root/a/f2.mp3": {}, + "root/a/ignored/bad.mp3": {}, + "root/b/cover.jpg": {}, + "root/c/f3": {}, + "root/d": {}, + "root/d/.ndignore": {}, + "root/d/f1.mp3": {}, + "root/d/f2.mp3": {}, + "root/d/f3.mp3": {}, + "root/e/original/f1.mp3": {}, + "root/e/symlink": {Mode: fs.ModeSymlink, Data: []byte("original")}, + }, + } + job = &scanJob{ + fs: fsys, + lib: model.Library{Path: "/music"}, + } + }) + + // Helper function to call walkDirTree and collect folders from the results channel + getFolders := func() map[string]*folderEntry { + results, err := walkDirTree(ctx, job) + 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() + return folders + } + + DescribeTable("symlink handling", + func(followSymlinks bool, expectedFolderCount int) { + conf.Server.Scanner.FollowSymlinks = followSymlinks + folders := getFolders() + + Expect(folders).To(HaveLen(expectedFolderCount + 2)) // +2 for `.` and `root` + + // Basic folder structure checks + Expect(folders["root/a"].audioFiles).To(SatisfyAll( + HaveLen(2), + HaveKey("f1.mp3"), + HaveKey("f2.mp3"), + )) + Expect(folders["root/a"].imageFiles).To(BeEmpty()) + Expect(folders["root/b"].audioFiles).To(BeEmpty()) + Expect(folders["root/b"].imageFiles).To(SatisfyAll( + HaveLen(1), + HaveKey("cover.jpg"), + )) + Expect(folders["root/c"].audioFiles).To(BeEmpty()) + Expect(folders["root/c"].imageFiles).To(BeEmpty()) + Expect(folders).ToNot(HaveKey("root/d")) + + // Symlink specific checks + if followSymlinks { + Expect(folders["root/e/symlink"].audioFiles).To(HaveLen(1)) + } else { + Expect(folders).ToNot(HaveKey("root/e/symlink")) + } }, - } - job = &scanJob{ - fs: fsys, - lib: model.Library{Path: "/music"}, - } + Entry("with symlinks enabled", true, 7), + Entry("with symlinks disabled", false, 6), + ) }) - // Helper function to call walkDirTree and collect folders from the results channel - getFolders := func() map[string]*folderEntry { - results, err := walkDirTree(ctx, job) - Expect(err).ToNot(HaveOccurred()) - - folders := map[string]*folderEntry{} - g := errgroup.Group{} - g.Go(func() error { - for folder := range results { - folders[folder.path] = folder + Context("with target folders", func() { + 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"}, } - return nil }) - _ = g.Wait() - return folders - } - DescribeTable("symlink handling", - func(followSymlinks bool, expectedFolderCount int) { - conf.Server.Scanner.FollowSymlinks = followSymlinks - folders := getFolders() + It("should recursively walk all subdirectories of target folders", func() { + results, err := walkDirTree(ctx, job, "Artist") + Expect(err).ToNot(HaveOccurred()) - Expect(folders).To(HaveLen(expectedFolderCount + 2)) // +2 for `.` and `root` + folders := map[string]*folderEntry{} + g := errgroup.Group{} + g.Go(func() error { + for folder := range results { + folders[folder.path] = folder + } + return nil + }) + _ = g.Wait() - // Basic folder structure checks - Expect(folders["root/a"].audioFiles).To(SatisfyAll( - HaveLen(2), - HaveKey("f1.mp3"), - HaveKey("f2.mp3"), + // 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"), )) - Expect(folders["root/a"].imageFiles).To(BeEmpty()) - Expect(folders["root/b"].audioFiles).To(BeEmpty()) - Expect(folders["root/b"].imageFiles).To(SatisfyAll( - HaveLen(1), - HaveKey("cover.jpg"), + + // 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 := walkDirTree(ctx, job, "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"), )) - Expect(folders["root/c"].audioFiles).To(BeEmpty()) - Expect(folders["root/c"].imageFiles).To(BeEmpty()) - Expect(folders).ToNot(HaveKey("root/d")) - // Symlink specific checks - if followSymlinks { - Expect(folders["root/e/symlink"].audioFiles).To(HaveLen(1)) - } else { - Expect(folders).ToNot(HaveKey("root/e/symlink")) - } - }, - Entry("with symlinks enabled", true, 7), - Entry("with symlinks disabled", false, 6), - ) - }) - - 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 + // Should not include other folders + Expect(folders).ToNot(HaveKey("Artist")) + Expect(folders).ToNot(HaveKey("Artist/Album2")) + Expect(folders).ToNot(HaveKey("Artist/Album2/Sub")) }) - _ = 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")) }) })