feat(scanner): add Scanner.PurgeMissing configuration option (#4107)

* Initial plan for issue * Add Scanner.PurgeMissing configuration option Co-authored-by: deluan <331353+deluan@users.noreply.github.com> * Remove GC call from phaseMissingTracks.purgeMissing method Co-authored-by: deluan <331353+deluan@users.noreply.github.com> * Address PR comments for Scanner.PurgeMissing feature Co-authored-by: deluan <331353+deluan@users.noreply.github.com> * Address PR comments and add DeleteAllMissing method Co-authored-by: deluan <331353+deluan@users.noreply.github.com> * refactor(scanner): simplify purgeMissing logic and improve error handling Signed-off-by: Deluan <deluan@navidrome.org> * fix configuration test Signed-off-by: Deluan <deluan@navidrome.org> --------- Signed-off-by: Deluan <deluan@navidrome.org> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: deluan <331353+deluan@users.noreply.github.com> Co-authored-by: Deluan <deluan@navidrome.org>
2025-05-22 20:50:15 -04:00
parent 4a2412eef7
commit 992c78376c
10 changed files with 292 additions and 24 deletions
@@ -6,6 +6,8 @@ import (
 	"sync/atomic"

 	ppl "github.com/google/go-pipeline/pkg/pipeline"
+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 )
@@ -182,7 +184,35 @@ func (p *phaseMissingTracks) finalize(err error) error {
 	if matched > 0 {
 		log.Info(p.ctx, "Scanner: Found moved files", "total", matched, err)
 	}
+	if err != nil {
+		return err
+	}
+
+	// Check if we should purge missing items
+	if conf.Server.Scanner.PurgeMissing == consts.PurgeMissingAlways || (conf.Server.Scanner.PurgeMissing == consts.PurgeMissingFull && p.state.fullScan) {
+		if err = p.purgeMissing(); err != nil {
+			log.Error(p.ctx, "Scanner: Error purging missing items", err)
+		}
+	}
+
 	return err
 }

+func (p *phaseMissingTracks) purgeMissing() error {
+	deletedCount, err := p.ds.MediaFile(p.ctx).DeleteAllMissing()
+	if err != nil {
+		return fmt.Errorf("error deleting missing files: %w", err)
+	}
+
+	if deletedCount > 0 {
+		log.Info(p.ctx, "Scanner: Purged missing items from the database", "mediaFiles", deletedCount)
+		// Set changesDetected to true so that garbage collection will run at the end of the scan process
+		p.state.changesDetected.Store(true)
+	} else {
+		log.Debug(p.ctx, "Scanner: No missing items to purge")
+	}
+
+	return nil
+}
+
 var _ phase[*missingTracks] = (*phaseMissingTracks)(nil)
@@ -4,6 +4,8 @@ import (
 	"context"
 	"time"

+	"github.com/navidrome/navidrome/conf"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
@@ -222,4 +224,66 @@ var _ = Describe("phaseMissingTracks", func() {
 			Expect(state.changesDetected.Load()).To(BeFalse())
 		})
 	})
+
+	Describe("finalize", func() {
+		It("should return nil if no error", func() {
+			err := phase.finalize(nil)
+			Expect(err).To(BeNil())
+			Expect(state.changesDetected.Load()).To(BeFalse())
+		})
+
+		It("should return the error if provided", func() {
+			err := phase.finalize(context.DeadlineExceeded)
+			Expect(err).To(Equal(context.DeadlineExceeded))
+			Expect(state.changesDetected.Load()).To(BeFalse())
+		})
+
+		When("PurgeMissing is 'always'", func() {
+			BeforeEach(func() {
+				conf.Server.Scanner.PurgeMissing = consts.PurgeMissingAlways
+				mr.CountAllValue = 3
+				mr.DeleteAllMissingValue = 3
+			})
+			It("should purge missing files", func() {
+				Expect(state.changesDetected.Load()).To(BeFalse())
+				err := phase.finalize(nil)
+				Expect(err).To(BeNil())
+				Expect(state.changesDetected.Load()).To(BeTrue())
+			})
+		})
+
+		When("PurgeMissing is 'full'", func() {
+			BeforeEach(func() {
+				conf.Server.Scanner.PurgeMissing = consts.PurgeMissingFull
+				mr.CountAllValue = 2
+				mr.DeleteAllMissingValue = 2
+			})
+			It("should not purge missing files if not a full scan", func() {
+				state.fullScan = false
+				err := phase.finalize(nil)
+				Expect(err).To(BeNil())
+				Expect(state.changesDetected.Load()).To(BeFalse())
+			})
+			It("should purge missing files if full scan", func() {
+				Expect(state.changesDetected.Load()).To(BeFalse())
+				state.fullScan = true
+				err := phase.finalize(nil)
+				Expect(err).To(BeNil())
+				Expect(state.changesDetected.Load()).To(BeTrue())
+			})
+		})
+
+		When("PurgeMissing is 'never'", func() {
+			BeforeEach(func() {
+				conf.Server.Scanner.PurgeMissing = consts.PurgeMissingNever
+				mr.CountAllValue = 1
+				mr.DeleteAllMissingValue = 1
+			})
+			It("should not purge missing files", func() {
+				err := phase.finalize(nil)
+				Expect(err).To(BeNil())
+				Expect(state.changesDetected.Load()).To(BeFalse())
+			})
+		})
+	})
 })
@@ -10,6 +10,7 @@ import (
 	"github.com/google/uuid"
 	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/conf/configtest"
+	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/core"
 	"github.com/navidrome/navidrome/core/artwork"
 	"github.com/navidrome/navidrome/core/metrics"
@@ -17,6 +18,7 @@ import (
 	"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"
@@ -47,6 +49,7 @@ var _ = Describe("Scanner", Ordered, func() {
 	}

 	BeforeAll(func() {
+		ctx = request.WithUser(GinkgoT().Context(), model.User{ID: "123", IsAdmin: true})
 		tmpDir := GinkgoT().TempDir()
 		conf.Server.DbPath = filepath.Join(tmpDir, "test-scanner.db?_journal_mode=WAL")
 		log.Warn("Using DB at " + conf.Server.DbPath)
@@ -54,7 +57,6 @@ var _ = Describe("Scanner", Ordered, func() {
 	})

 	BeforeEach(func() {
-		ctx = context.Background()
 		db.Init(ctx)
 		DeferCleanup(func() {
 			Expect(tests.ClearDB()).To(Succeed())
@@ -501,6 +503,113 @@ var _ = Describe("Scanner", Ordered, func() {
 			Expect(aa[0].MbzArtistID).To(Equal(beatlesMBID))
 			Expect(aa[0].SortArtistName).To(Equal("Beatles, The"))
 		})
+
+		Context("When PurgeMissing is configured", func() {
+			When("PurgeMissing is set to 'never'", func() {
+				BeforeEach(func() {
+					DeferCleanup(configtest.SetupConfig())
+					conf.Server.Scanner.PurgeMissing = consts.PurgeMissingNever
+				})
+
+				It("should mark files as missing but not delete them", func() {
+					By("Running initial scan")
+					Expect(runScanner(ctx, true)).To(Succeed())
+
+					By("Removing a file")
+					fsys.Remove("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+
+					By("Running another scan")
+					Expect(runScanner(ctx, true)).To(Succeed())
+
+					By("Checking files are marked as missing but not deleted")
+					count, err := ds.MediaFile(ctx).CountAll(model.QueryOptions{
+						Filters: squirrel.Eq{"missing": true},
+					})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(Equal(int64(1)))
+
+					mf, err := findByPath("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(mf.Missing).To(BeTrue())
+				})
+			})
+
+			When("PurgeMissing is set to 'always'", func() {
+				BeforeEach(func() {
+					conf.Server.Scanner.PurgeMissing = consts.PurgeMissingAlways
+				})
+
+				It("should purge missing files on any scan", func() {
+					By("Running initial scan")
+					Expect(runScanner(ctx, false)).To(Succeed())
+
+					By("Removing a file")
+					fsys.Remove("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+
+					By("Running an incremental scan")
+					Expect(runScanner(ctx, false)).To(Succeed())
+
+					By("Checking missing files are deleted")
+					count, err := ds.MediaFile(ctx).CountAll(model.QueryOptions{
+						Filters: squirrel.Eq{"missing": true},
+					})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(BeZero())
+
+					_, err = findByPath("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+					Expect(err).To(MatchError(model.ErrNotFound))
+				})
+			})
+
+			When("PurgeMissing is set to 'full'", func() {
+				BeforeEach(func() {
+					conf.Server.Scanner.PurgeMissing = consts.PurgeMissingFull
+				})
+
+				It("should not purge missing files on incremental scans", func() {
+					By("Running initial scan")
+					Expect(runScanner(ctx, true)).To(Succeed())
+
+					By("Removing a file")
+					fsys.Remove("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+
+					By("Running an incremental scan")
+					Expect(runScanner(ctx, false)).To(Succeed())
+
+					By("Checking files are marked as missing but not deleted")
+					count, err := ds.MediaFile(ctx).CountAll(model.QueryOptions{
+						Filters: squirrel.Eq{"missing": true},
+					})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(Equal(int64(1)))
+
+					mf, err := findByPath("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+					Expect(err).ToNot(HaveOccurred())
+					Expect(mf.Missing).To(BeTrue())
+				})
+
+				It("should purge missing files only on full scans", func() {
+					By("Running initial scan")
+					Expect(runScanner(ctx, true)).To(Succeed())
+
+					By("Removing a file")
+					fsys.Remove("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+
+					By("Running a full scan")
+					Expect(runScanner(ctx, true)).To(Succeed())
+
+					By("Checking missing files are deleted")
+					count, err := ds.MediaFile(ctx).CountAll(model.QueryOptions{
+						Filters: squirrel.Eq{"missing": true},
+					})
+					Expect(err).ToNot(HaveOccurred())
+					Expect(count).To(BeZero())
+
+					_, err = findByPath("The Beatles/Revolver/02 - Eleanor Rigby.mp3")
+					Expect(err).To(MatchError(model.ErrNotFound))
+				})
+			})
+		})
 	})
 })