mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
feat(scanner): mirror album re-PID flow for artists
When PID.Artist spec changes, existing artist IDs differ from newly-computed ones. Without remapping, all annotations (starred, ratings, play counts) become orphaned. This change builds an artistIDMap during phase 1 and reassigns annotations on persist, mirroring the existing albumIDMap flow. The migration logic is gated on the previous spec being non-empty and different from the current spec, so the default "name" spec (byte-identical to the historical hardcoded artistID) triggers no remapping on upgrade.
This commit is contained in:
parent
46cb9f8d58
commit
8b5e50a1aa
@ -124,6 +124,12 @@ func computeArtistPID(p model.Participant, spec string, hash hashFunc) string {
|
||||
return hash(pid)
|
||||
}
|
||||
|
||||
// ComputeArtistPID is the exported entry point for callers outside this package
|
||||
// (e.g. the scanner) that need to compute an artist PID under a specific spec.
|
||||
func ComputeArtistPID(p model.Participant, spec string) string {
|
||||
return computeArtistPID(p, spec, id.NewHash)
|
||||
}
|
||||
|
||||
func getArtistPIDAttr(p model.Participant, attr string, hash hashFunc) string {
|
||||
switch strings.TrimSpace(strings.ToLower(attr)) {
|
||||
case "name":
|
||||
|
||||
@ -17,14 +17,15 @@ import (
|
||||
|
||||
func newFolderEntry(job *scanJob, id, path string, updTime time.Time, hash string) *folderEntry {
|
||||
f := &folderEntry{
|
||||
id: id,
|
||||
job: job,
|
||||
path: path,
|
||||
audioFiles: make(map[string]fs.DirEntry),
|
||||
imageFiles: make(map[string]fs.DirEntry),
|
||||
albumIDMap: make(map[string]string),
|
||||
updTime: updTime,
|
||||
prevHash: hash,
|
||||
id: id,
|
||||
job: job,
|
||||
path: path,
|
||||
audioFiles: make(map[string]fs.DirEntry),
|
||||
imageFiles: make(map[string]fs.DirEntry),
|
||||
albumIDMap: make(map[string]string),
|
||||
artistIDMap: make(map[string]string),
|
||||
updTime: updTime,
|
||||
prevHash: hash,
|
||||
}
|
||||
return f
|
||||
}
|
||||
@ -46,6 +47,7 @@ type folderEntry struct {
|
||||
albums model.Albums
|
||||
albumIDMap map[string]string
|
||||
artists model.Artists
|
||||
artistIDMap map[string]string
|
||||
tags model.TagList
|
||||
missingTracks []*model.MediaFile
|
||||
}
|
||||
|
||||
@ -123,11 +123,12 @@ func (j *scanJob) createFolderEntry(path string) *folderEntry {
|
||||
// The phaseFolders struct implements the phase interface, providing methods to produce
|
||||
// folder entries, process folders, persist changes to the database, and log the results.
|
||||
type phaseFolders struct {
|
||||
jobs []*scanJob
|
||||
ds model.DataStore
|
||||
ctx context.Context
|
||||
state *scanState
|
||||
prevAlbumPIDConf string
|
||||
jobs []*scanJob
|
||||
ds model.DataStore
|
||||
ctx context.Context
|
||||
state *scanState
|
||||
prevAlbumPIDConf string
|
||||
prevArtistPIDConf string
|
||||
}
|
||||
|
||||
func (p *phaseFolders) description() string {
|
||||
@ -141,6 +142,10 @@ func (p *phaseFolders) producer() ppl.Producer[*folderEntry] {
|
||||
if err != nil {
|
||||
return fmt.Errorf("getting album PID conf: %w", err)
|
||||
}
|
||||
p.prevArtistPIDConf, err = p.ds.Property(p.ctx).DefaultGet(consts.PIDArtistKey, "")
|
||||
if err != nil {
|
||||
return fmt.Errorf("getting artist PID conf: %w", err)
|
||||
}
|
||||
|
||||
// TODO Parallelize multiple job when we have multiple libraries
|
||||
var total int64
|
||||
@ -298,6 +303,21 @@ func (p *phaseFolders) loadTagsFromFiles(entry *folderEntry, toImport map[string
|
||||
if prevAlbumID != track.AlbumID && !ok {
|
||||
entry.albumIDMap[track.AlbumID] = prevAlbumID
|
||||
}
|
||||
|
||||
// Build artistIDMap: for each participant on this track, if the previous
|
||||
// spec produces a different ID than the current one, record the mapping.
|
||||
if p.prevArtistPIDConf != "" && p.prevArtistPIDConf != conf.Server.PID.Artist {
|
||||
for _, list := range track.Participants {
|
||||
for _, part := range list {
|
||||
prevID := metadata.ComputeArtistPID(part, p.prevArtistPIDConf)
|
||||
if prevID != part.ID {
|
||||
if _, already := entry.artistIDMap[part.ID]; !already {
|
||||
entry.artistIDMap[part.ID] = prevID
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
entry.tracks = tracks
|
||||
@ -359,9 +379,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", "updated_at")
|
||||
if err != nil {
|
||||
if err = p.persistArtist(artistRepo, &entry.artists[i], entry.artistIDMap); err != nil {
|
||||
log.Error(p.ctx, "Scanner: Error persisting artist to DB", "folder", entry.path, "artist", entry.artists[i].Name, err)
|
||||
return err
|
||||
}
|
||||
@ -462,6 +480,26 @@ func (p *phaseFolders) persistAlbum(repo model.AlbumRepository, a *model.Album,
|
||||
return nil
|
||||
}
|
||||
|
||||
// persistArtist persists the given artist and reassigns annotations from the
|
||||
// previous artist ID (if recorded in idMap). Mirrors persistAlbum.
|
||||
func (p *phaseFolders) persistArtist(repo model.ArtistRepository, a *model.Artist, idMap map[string]string) error {
|
||||
if err := repo.Put(a, "name", "mbz_artist_id", "sort_artist_name",
|
||||
"order_artist_name", "full_text", "updated_at"); err != nil {
|
||||
return fmt.Errorf("persisting artist %s: %w", a.ID, err)
|
||||
}
|
||||
prevID := idMap[a.ID]
|
||||
if prevID == "" {
|
||||
return nil
|
||||
}
|
||||
log.Trace(p.ctx, "Reassigning artist annotations", "from", prevID, "to", a.ID, "artist", a.Name)
|
||||
if err := repo.ReassignAnnotation(prevID, a.ID); err != nil {
|
||||
log.Warn(p.ctx, "Scanner: Could not reassign artist annotations", "from", prevID, "to", a.ID, "artist", a.Name, err)
|
||||
p.state.sendWarning(fmt.Sprintf("Could not reassign artist annotations from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err))
|
||||
}
|
||||
delete(idMap, a.ID)
|
||||
return nil
|
||||
}
|
||||
|
||||
func (p *phaseFolders) logFolder(entry *folderEntry) (*folderEntry, error) {
|
||||
logCall := log.Info
|
||||
if entry.isEmpty() {
|
||||
|
||||
49
scanner/phase_1_folders_test.go
Normal file
49
scanner/phase_1_folders_test.go
Normal file
@ -0,0 +1,49 @@
|
||||
package scanner
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"github.com/navidrome/navidrome/conf/configtest"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
var _ = Describe("phaseFolders", func() {
|
||||
Describe("Artist re-PID migration", func() {
|
||||
var (
|
||||
artRepo *tests.MockArtistRepo
|
||||
p *phaseFolders
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
DeferCleanup(configtest.SetupConfig())
|
||||
artRepo = tests.CreateMockArtistRepo()
|
||||
if artRepo.ReassignAnnotationCalls == nil {
|
||||
artRepo.ReassignAnnotationCalls = map[string]string{}
|
||||
}
|
||||
p = &phaseFolders{ctx: context.Background(), state: &scanState{}}
|
||||
})
|
||||
|
||||
It("calls ReassignAnnotation when persisting an artist whose ID changed", func() {
|
||||
oldID := "old-id"
|
||||
newID := "new-id"
|
||||
idMap := map[string]string{newID: oldID}
|
||||
artist := model.Artist{ID: newID, Name: "Foo"}
|
||||
|
||||
Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed())
|
||||
Expect(artRepo.ReassignAnnotationCalls).To(HaveKeyWithValue(oldID, newID))
|
||||
// Mapping should be removed after successful reassignment
|
||||
Expect(idMap).ToNot(HaveKey(newID))
|
||||
})
|
||||
|
||||
It("does not call ReassignAnnotation when no mapping exists", func() {
|
||||
idMap := map[string]string{}
|
||||
artist := model.Artist{ID: "some-id", Name: "Foo"}
|
||||
|
||||
Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed())
|
||||
Expect(artRepo.ReassignAnnotationCalls).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
})
|
||||
@ -16,9 +16,10 @@ func CreateMockArtistRepo() *MockArtistRepo {
|
||||
|
||||
type MockArtistRepo struct {
|
||||
model.ArtistRepository
|
||||
Data map[string]*model.Artist
|
||||
Err bool
|
||||
Options model.QueryOptions
|
||||
Data map[string]*model.Artist
|
||||
Err bool
|
||||
Options model.QueryOptions
|
||||
ReassignAnnotationCalls map[string]string // prevID -> newID
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) SetError(err bool) {
|
||||
@ -157,4 +158,17 @@ func (m *MockArtistRepo) Search(q string, options ...model.QueryOptions) (model.
|
||||
return allArtists, err
|
||||
}
|
||||
|
||||
// ReassignAnnotation reassigns annotations from one artist to another
|
||||
func (m *MockArtistRepo) ReassignAnnotation(prevID string, newID string) error {
|
||||
if m.Err {
|
||||
return errors.New("unexpected error")
|
||||
}
|
||||
// Mock implementation - track the reassignment calls
|
||||
if m.ReassignAnnotationCalls == nil {
|
||||
m.ReassignAnnotationCalls = make(map[string]string)
|
||||
}
|
||||
m.ReassignAnnotationCalls[prevID] = newID
|
||||
return nil
|
||||
}
|
||||
|
||||
var _ model.ArtistRepository = (*MockArtistRepo)(nil)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user