fix(scanner): ensure full scans update the DB (#4252)

* fix: ensure full scan refreshes all artist stats

After PR #4059, full scans were not forcing a refresh of all artists.
This change ensures that during full scans, all artist stats are refreshed
instead of only those with recently updated media files.

Changes:
- Set changesDetected=true at start of full scans to ensure maintenance operations run
- Add allArtists parameter to RefreshStats() method
- Pass fullScan state to RefreshStats to control refresh scope
- Update mock repository to match new interface

Fixes #4246
Related to PR #4059

* fix: add tests for full and incremental scans

Signed-off-by: Deluan <deluan@navidrome.org>

---------

Signed-off-by: Deluan <deluan@navidrome.org>

model/artist.go

@@ -82,7 +82,7 @@ type ArtistRepository interface {
 
 	// The following methods are used exclusively by the scanner:
 	RefreshPlayCounts() (int64, error)
-	RefreshStats() (int64, error)
+	RefreshStats(allArtists bool) (int64, error)
 
 	AnnotatedRepository
 	SearchableRepository[Artists]

persistence/artist_repository.go

@@ -292,25 +292,34 @@ on conflict (user_id, item_id, item_type) do update
 }
 
 // RefreshStats updates the stats field for artists whose associated media files were updated after the oldest recorded library scan time.
-// It processes artists in batches to handle potentially large updates.
-func (r *artistRepository) RefreshStats() (int64, error) {
-	touchedArtistsQuerySQL := `
+// When allArtists is true, it refreshes stats for all artists. It processes artists in batches to handle potentially large updates.
+func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) {
+	var allTouchedArtistIDs []string
+	if allArtists {
+		// Refresh stats for all artists
+		allArtistsQuerySQL := `SELECT DISTINCT id FROM artist WHERE id <> ''`
+		if err := r.db.NewQuery(allArtistsQuerySQL).Column(&allTouchedArtistIDs); err != nil {
+			return 0, fmt.Errorf("fetching all artist IDs: %w", err)
+		}
+		log.Debug(r.ctx, "RefreshStats: Refreshing all artists.", "count", len(allTouchedArtistIDs))
+	} else {
+		// Only refresh artists with updated media files
+		touchedArtistsQuerySQL := `
         SELECT DISTINCT mfa.artist_id
         FROM media_file_artists mfa
         JOIN media_file mf ON mfa.media_file_id = mf.id
         WHERE mf.updated_at > (SELECT last_scan_at FROM library ORDER BY last_scan_at ASC LIMIT 1)
         `
-
-	var allTouchedArtistIDs []string
-	if err := r.db.NewQuery(touchedArtistsQuerySQL).Column(&allTouchedArtistIDs); err != nil {
-		return 0, fmt.Errorf("fetching touched artist IDs: %w", err)
+		if err := r.db.NewQuery(touchedArtistsQuerySQL).Column(&allTouchedArtistIDs); err != nil {
+			return 0, fmt.Errorf("fetching touched artist IDs: %w", err)
+		}
+		log.Debug(r.ctx, "RefreshStats: Refreshing touched artists.", "count", len(allTouchedArtistIDs))
 	}
 
 	if len(allTouchedArtistIDs) == 0 {
 		log.Debug(r.ctx, "RefreshStats: No artists to update.")
 		return 0, nil
 	}
-	log.Debug(r.ctx, "RefreshStats: Found artists to update.", "count", len(allTouchedArtistIDs))
 
 	// Template for the batch update with placeholder markers that we'll replace
 	batchUpdateStatsSQL := `

scanner/scanner.go

@@ -45,14 +45,25 @@ func (s *scanState) sendError(err error) {
 }
 
 func (s *scannerImpl) scanAll(ctx context.Context, fullScan bool, progress chan<- *ProgressInfo) {
-	state := scanState{progress: progress, fullScan: fullScan}
+	startTime := time.Now()
+
+	state := scanState{
+		progress:        progress,
+		fullScan:        fullScan,
+		changesDetected: atomic.Bool{},
+	}
+
+	// Set changesDetected to true for full scans to ensure all maintenance operations run
+	if fullScan {
+		state.changesDetected.Store(true)
+	}
+
 	libs, err := s.ds.Library(ctx).GetAll()
 	if err != nil {
 		state.sendWarning(fmt.Sprintf("getting libraries: %s", err))
 		return
 	}
 
-	startTime := time.Now()
 	log.Info(ctx, "Scanner: Starting scan", "fullScan", state.fullScan, "numLibraries", len(libs))
 
 	// Store scan type and start time
@@ -148,7 +159,7 @@ func (s *scannerImpl) runRefreshStats(ctx context.Context, state *scanState) fun
 			return nil
 		}
 		start := time.Now()
-		stats, err := s.ds.Artist(ctx).RefreshStats()
+		stats, err := s.ds.Artist(ctx).RefreshStats(state.fullScan)
 		if err != nil {
 			log.Error(ctx, "Scanner: Error refreshing artists stats", err)
 			return fmt.Errorf("refreshing artists stats: %w", err)

scanner/scanner_test.go

@@ -612,6 +612,56 @@ var _ = Describe("Scanner", Ordered, func() {
 			})
 		})
 	})
+
+	Describe("RefreshStats", func() {
+		var refreshStatsCalls []bool
+
+		BeforeEach(func() {
+			refreshStatsCalls = nil
+
+			// Create a mock artist repository that tracks RefreshStats calls
+			originalArtistRepo := ds.RealDS.Artist(ctx)
+			ds.MockedArtist = &testArtistRepo{
+				ArtistRepository: originalArtistRepo,
+				callTracker:      &refreshStatsCalls,
+			}
+
+			// Create a simple filesystem for testing
+			revolver := template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966})
+			createFS(fstest.MapFS{
+				"The Beatles/Revolver/01 - Taxman.mp3": revolver(track(1, "Taxman")),
+			})
+		})
+
+		It("should call RefreshStats with allArtists=true for full scans", func() {
+			Expect(runScanner(ctx, true)).To(Succeed())
+
+			Expect(refreshStatsCalls).To(HaveLen(1))
+			Expect(refreshStatsCalls[0]).To(BeTrue(), "RefreshStats should be called with allArtists=true for full scans")
+		})
+
+		It("should call RefreshStats with allArtists=false for incremental scans", func() {
+			// First do a full scan to set up the data
+			Expect(runScanner(ctx, true)).To(Succeed())
+
+			// Reset the tracker to only track the incremental scan
+			refreshStatsCalls = nil
+
+			// Add a new file to trigger changes detection
+			revolver := template(_t{"albumartist": "The Beatles", "album": "Revolver", "year": 1966})
+			fsys := createFS(fstest.MapFS{
+				"The Beatles/Revolver/01 - Taxman.mp3":        revolver(track(1, "Taxman")),
+				"The Beatles/Revolver/02 - Eleanor Rigby.mp3": revolver(track(2, "Eleanor Rigby")),
+			})
+			_ = fsys
+
+			// Do an incremental scan
+			Expect(runScanner(ctx, false)).To(Succeed())
+
+			Expect(refreshStatsCalls).To(HaveLen(1))
+			Expect(refreshStatsCalls[0]).To(BeFalse(), "RefreshStats should be called with allArtists=false for incremental scans")
+		})
+	})
 })
 
 func createFindByPath(ctx context.Context, ds model.DataStore) func(string) (*model.MediaFile, error) {
@@ -638,3 +688,13 @@ func (m *mockMediaFileRepo) GetMissingAndMatching(libId int) (model.MediaFileCur
 	}
 	return m.MediaFileRepository.GetMissingAndMatching(libId)
 }
+
+type testArtistRepo struct {
+	model.ArtistRepository
+	callTracker *[]bool
+}
+
+func (m *testArtistRepo) RefreshStats(allArtists bool) (int64, error) {
+	*m.callTracker = append(*m.callTracker, allArtists)
+	return m.ArtistRepository.RefreshStats(allArtists)
+}

tests/mock_artist_repo.go

@@ -94,4 +94,18 @@ func (m *MockArtistRepo) UpdateExternalInfo(artist *model.Artist) error {
 	return nil
 }
 
+func (m *MockArtistRepo) RefreshStats(allArtists bool) (int64, error) {
+	if m.Err {
+		return 0, errors.New("mock repo error")
+	}
+	return int64(len(m.Data)), nil
+}
+
+func (m *MockArtistRepo) RefreshPlayCounts() (int64, error) {
+	if m.Err {
+		return 0, errors.New("mock repo error")
+	}
+	return int64(len(m.Data)), nil
+}
+
 var _ model.ArtistRepository = (*MockArtistRepo)(nil)