mirror of
https://github.com/navidrome/navidrome.git
synced 2025-12-23 23:18:05 -05:00
fix(scanner) artist stats not refreshing during quick scan and after missing file deletion (#4269)
* Fix artist not being marked as touched during quick scans When a new album is added during quick scans, artists were not being marked as 'touched' due to media files having older modification times than the scan completion time. Changes: - Add 'updated_at' to artist Put() columns in scanner to ensure timestamp is set when artists are processed - Simplify RefreshStats query to check artist.updated_at directly instead of complex media file joins - Artists from new albums now properly get refreshed in later phases This fixes the issue where newly added albums would have incomplete artist information after quick scans. * fix(missing): refresh artist stats in background after deleting missing files Signed-off-by: Deluan <deluan@navidrome.org> * fix(request): add InternalAuth to user context Signed-off-by: Deluan <deluan@navidrome.org> * Add comprehensive test for artist stats update during quick scans - Add test that verifies artist statistics are correctly updated when new files are added during incremental scans - Test ensures both overall stats (AlbumCount, SongCount) and role-specific stats are properly refreshed - Validates fix for artist stats not being refreshed during quick scans when new albums are added - Uses real artist repository instead of mock to verify actual stats calculation behavior --------- Signed-off-by: Deluan <deluan@navidrome.org>
This commit is contained in:
@@ -29,6 +29,7 @@ var allKeys = []contextKey{
|
||||
Transcoding,
|
||||
ClientUniqueId,
|
||||
ReverseProxyIp,
|
||||
InternalAuth,
|
||||
}
|
||||
|
||||
func WithUser(ctx context.Context, u model.User) context.Context {
|
||||
|
||||
@@ -303,12 +303,11 @@ func (r *artistRepository) RefreshStats(allArtists bool) (int64, error) {
|
||||
}
|
||||
log.Debug(r.ctx, "RefreshStats: Refreshing all artists.", "count", len(allTouchedArtistIDs))
|
||||
} else {
|
||||
// Only refresh artists with updated media files
|
||||
// Only refresh artists with updated timestamps
|
||||
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)
|
||||
SELECT DISTINCT id
|
||||
FROM artist
|
||||
WHERE updated_at > (SELECT last_scan_at FROM library ORDER BY last_scan_at ASC LIMIT 1)
|
||||
`
|
||||
if err := r.db.NewQuery(touchedArtistsQuerySQL).Column(&allTouchedArtistIDs); err != nil {
|
||||
return 0, fmt.Errorf("fetching touched artist IDs: %w", err)
|
||||
|
||||
@@ -345,7 +345,7 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error)
|
||||
// Save all new/modified artists to DB. Their information will be incomplete, but they will be refreshed later
|
||||
for i := range entry.artists {
|
||||
err = artistRepo.Put(&entry.artists[i], "name",
|
||||
"mbz_artist_id", "sort_artist_name", "order_artist_name", "full_text")
|
||||
"mbz_artist_id", "sort_artist_name", "order_artist_name", "full_text", "updated_at")
|
||||
if err != nil {
|
||||
log.Error(p.ctx, "Scanner: Error persisting artist to DB", "folder", entry.path, "artist", entry.artists[i].Name, err)
|
||||
return err
|
||||
|
||||
@@ -615,6 +615,8 @@ var _ = Describe("Scanner", Ordered, func() {
|
||||
|
||||
Describe("RefreshStats", func() {
|
||||
var refreshStatsCalls []bool
|
||||
var fsys storagetest.FakeFS
|
||||
var help func(...map[string]any) *fstest.MapFile
|
||||
|
||||
BeforeEach(func() {
|
||||
refreshStatsCalls = nil
|
||||
@@ -627,9 +629,9 @@ var _ = Describe("Scanner", Ordered, func() {
|
||||
}
|
||||
|
||||
// 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")),
|
||||
help = template(_t{"albumartist": "The Beatles", "album": "Help!", "year": 1965})
|
||||
fsys = createFS(fstest.MapFS{
|
||||
"The Beatles/Help!/01 - Help!.mp3": help(track(1, "Help!")),
|
||||
})
|
||||
})
|
||||
|
||||
@@ -648,12 +650,7 @@ var _ = Describe("Scanner", Ordered, func() {
|
||||
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
|
||||
fsys.Add("The Beatles/Help!/02 - The Night Before.mp3", help(track(2, "The Night Before")))
|
||||
|
||||
// Do an incremental scan
|
||||
Expect(runScanner(ctx, false)).To(Succeed())
|
||||
@@ -661,6 +658,52 @@ var _ = Describe("Scanner", Ordered, func() {
|
||||
Expect(refreshStatsCalls).To(HaveLen(1))
|
||||
Expect(refreshStatsCalls[0]).To(BeFalse(), "RefreshStats should be called with allArtists=false for incremental scans")
|
||||
})
|
||||
|
||||
It("should update artist stats during quick scans when new albums are added", func() {
|
||||
// Don't use the mocked artist repo for this test - we need the real one
|
||||
ds.MockedArtist = nil
|
||||
|
||||
By("Initial scan with one album")
|
||||
Expect(runScanner(ctx, true)).To(Succeed())
|
||||
|
||||
// Verify initial artist stats - should have 1 album, 1 song
|
||||
artists, err := ds.Artist(ctx).GetAll(model.QueryOptions{
|
||||
Filters: squirrel.Eq{"name": "The Beatles"},
|
||||
})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(artists).To(HaveLen(1))
|
||||
artist := artists[0]
|
||||
Expect(artist.AlbumCount).To(Equal(1)) // 1 album
|
||||
Expect(artist.SongCount).To(Equal(1)) // 1 song
|
||||
|
||||
By("Adding files to an existing directory during incremental scan")
|
||||
// Add more files to the existing Help! album - this should trigger artist stats update during incremental scan
|
||||
fsys.Add("The Beatles/Help!/02 - The Night Before.mp3", help(track(2, "The Night Before")))
|
||||
fsys.Add("The Beatles/Help!/03 - You've Got to Hide Your Love Away.mp3", help(track(3, "You've Got to Hide Your Love Away")))
|
||||
|
||||
// Do a quick scan (incremental)
|
||||
Expect(runScanner(ctx, false)).To(Succeed())
|
||||
|
||||
By("Verifying artist stats were updated correctly")
|
||||
// Fetch the artist again to check updated stats
|
||||
artists, err = ds.Artist(ctx).GetAll(model.QueryOptions{
|
||||
Filters: squirrel.Eq{"name": "The Beatles"},
|
||||
})
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(artists).To(HaveLen(1))
|
||||
updatedArtist := artists[0]
|
||||
|
||||
// Should now have 1 album and 3 songs total
|
||||
// This is the key test - that artist stats are updated during quick scans
|
||||
Expect(updatedArtist.AlbumCount).To(Equal(1)) // 1 album
|
||||
Expect(updatedArtist.SongCount).To(Equal(3)) // 3 songs
|
||||
|
||||
// Also verify that role-specific stats are updated (albumartist role)
|
||||
Expect(updatedArtist.Stats).To(HaveKey(model.RoleAlbumArtist))
|
||||
albumArtistStats := updatedArtist.Stats[model.RoleAlbumArtist]
|
||||
Expect(albumArtistStats.AlbumCount).To(Equal(1)) // 1 album
|
||||
Expect(albumArtistStats.SongCount).To(Equal(3)) // 3 songs
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
|
||||
@@ -10,6 +10,7 @@ import (
|
||||
"github.com/deluan/rest"
|
||||
"github.com/navidrome/navidrome/log"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/model/request"
|
||||
"github.com/navidrome/navidrome/utils/req"
|
||||
)
|
||||
|
||||
@@ -89,6 +90,17 @@ func deleteMissingFiles(ds model.DataStore, w http.ResponseWriter, r *http.Reque
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
|
||||
// Refresh artist stats in background after deleting missing files
|
||||
go func() {
|
||||
bgCtx := request.AddValues(context.Background(), r.Context())
|
||||
if _, err := ds.Artist(bgCtx).RefreshStats(true); err != nil {
|
||||
log.Error(bgCtx, "Error refreshing artist stats after deleting missing files", err)
|
||||
} else {
|
||||
log.Debug(bgCtx, "Successfully refreshed artist stats after deleting missing files")
|
||||
}
|
||||
}()
|
||||
|
||||
writeDeleteManyResponse(w, r, ids)
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user