mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(scanner): carry covers from every old album merged during a scan
The phase-2 attribute copy ran under the per-target annotations guard, so when several old albums merged into one new album ID in a single scan, only the first old album's attributes were copied — a cover held by a later one was lost (and its file later reaped by the purge GC). Dedupe the created_at/cover copy per old→new pair instead; the empty-source guard in CopyAttributes keeps coverless old albums from overwriting anything.
This commit is contained in:
parent
83ae195c10
commit
cbc12034a4
@ -38,7 +38,8 @@ type phaseMissingTracks struct {
|
||||
totalMatched atomic.Uint32
|
||||
state *scanState
|
||||
processedAlbumAnnotations map[string]bool // Track processed album annotation reassignments
|
||||
annotationMutex sync.RWMutex // Protects processedAlbumAnnotations
|
||||
processedAlbumCopies map[string]bool // Track attribute copies, per old→new album pair
|
||||
annotationMutex sync.RWMutex // Protects the two maps above
|
||||
}
|
||||
|
||||
func createPhaseMissingTracks(ctx context.Context, state *scanState, ds model.DataStore) *phaseMissingTracks {
|
||||
@ -47,6 +48,7 @@ func createPhaseMissingTracks(ctx context.Context, state *scanState, ds model.Da
|
||||
ds: ds,
|
||||
state: state,
|
||||
processedAlbumAnnotations: make(map[string]bool),
|
||||
processedAlbumCopies: make(map[string]bool),
|
||||
}
|
||||
}
|
||||
|
||||
@ -313,14 +315,6 @@ func (p *phaseMissingTracks) moveMatched(target, missing model.MediaFile) error
|
||||
log.Warn(p.ctx, "Scanner: Could not reassign album annotations", "from", oldAlbumID, "to", newAlbumID, err)
|
||||
}
|
||||
|
||||
// Keep created_at (so moved albums don't resurface in "Recently Added")
|
||||
// and any manually uploaded cover from the previous album instance.
|
||||
if err := tx.Album(p.ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at", "uploaded_image", "cover_art_updated_at"); err != nil {
|
||||
if !errors.Is(err, model.ErrNotFound) {
|
||||
log.Warn(p.ctx, "Scanner: Could not copy album attributes", "from", oldAlbumID, "to", newAlbumID, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Note: RefreshPlayCounts will be called in later phases, so we don't need to call it here
|
||||
p.processedAlbumAnnotations[newAlbumID] = true
|
||||
}
|
||||
@ -328,6 +322,25 @@ func (p *phaseMissingTracks) moveMatched(target, missing model.MediaFile) error
|
||||
} else {
|
||||
log.Trace(p.ctx, "Scanner: Skipping album annotation reassignment", "from", oldAlbumID, "to", newAlbumID)
|
||||
}
|
||||
|
||||
// Copy created_at/cover per old→new pair (not per target): with several old albums
|
||||
// merging into one, any of them may hold the uploaded cover; empty sources never copy.
|
||||
pairKey := oldAlbumID + "\x00" + newAlbumID
|
||||
p.annotationMutex.RLock()
|
||||
copyDone := p.processedAlbumCopies[pairKey]
|
||||
p.annotationMutex.RUnlock()
|
||||
if !copyDone {
|
||||
p.annotationMutex.Lock()
|
||||
if !p.processedAlbumCopies[pairKey] {
|
||||
if err := tx.Album(p.ctx).CopyAttributes(oldAlbumID, newAlbumID, "created_at", "uploaded_image", "cover_art_updated_at"); err != nil {
|
||||
if !errors.Is(err, model.ErrNotFound) {
|
||||
log.Warn(p.ctx, "Scanner: Could not copy album attributes", "from", oldAlbumID, "to", newAlbumID, err)
|
||||
}
|
||||
}
|
||||
p.processedAlbumCopies[pairKey] = true
|
||||
}
|
||||
p.annotationMutex.Unlock()
|
||||
}
|
||||
}
|
||||
|
||||
p.state.changesDetected.Store(true)
|
||||
|
||||
@ -866,6 +866,32 @@ var _ = Describe("phaseMissingTracks", func() {
|
||||
Expect(newAlbum.UploadedImage).To(Equal("old-album_cover.jpg"))
|
||||
})
|
||||
|
||||
It("should preserve a cover held by any of several old albums merged into one", func() {
|
||||
// old-1 (no cover) merges first and marks the target as processed for annotations;
|
||||
// old-2 carries the cover and must still contribute it.
|
||||
missing1 := model.MediaFile{ID: "mg-1", PID: "MG1", Path: "lib1/a.mp3", AlbumID: "old-1", LibraryID: 1}
|
||||
matched1 := model.MediaFile{ID: "mt-1", PID: "MG1", Path: "lib2/a.mp3", AlbumID: "new-album", LibraryID: 1}
|
||||
missing2 := model.MediaFile{ID: "mg-2", PID: "MG2", Path: "lib1/b.mp3", AlbumID: "old-2", LibraryID: 1}
|
||||
matched2 := model.MediaFile{ID: "mt-2", PID: "MG2", Path: "lib2/b.mp3", AlbumID: "new-album", LibraryID: 1}
|
||||
|
||||
albumRepo.SetData(model.Albums{
|
||||
{ID: "old-1", LibraryID: 1},
|
||||
{ID: "old-2", LibraryID: 1, UploadedImage: "old-2_cover.jpg"},
|
||||
{ID: "new-album", LibraryID: 1},
|
||||
})
|
||||
|
||||
for _, mf := range []*model.MediaFile{&missing1, &matched1, &missing2, &matched2} {
|
||||
_ = ds.MediaFile(ctx).Put(mf)
|
||||
}
|
||||
|
||||
Expect(phase.moveMatched(matched1, missing1)).To(Succeed())
|
||||
Expect(phase.moveMatched(matched2, missing2)).To(Succeed())
|
||||
|
||||
newAlbum, err := albumRepo.Get("new-album")
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(newAlbum.UploadedImage).To(Equal("old-2_cover.jpg"))
|
||||
})
|
||||
|
||||
It("should not copy album created_at when album ID does not change", func() {
|
||||
originalTime := time.Date(2020, 1, 1, 0, 0, 0, 0, time.UTC)
|
||||
missingTrack := model.MediaFile{
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user