mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-31 07:30:32 +00:00
fix(scanner): preserve artist created_at across PID changes
persistArtist now calls repo.CopyAttributes(prevID, a.ID, "created_at") after ReassignAnnotation, mirroring persistAlbum. Without this, every PID.Artist config change reset the artist's created_at to scan time, breaking 'recently added artists' views and any consumer that relies on the original first-seen timestamp. Adds CopyAttributes to ArtistRepository (mirroring AlbumRepository's same-name method) with the same zero-poisoning guard for created_at.
This commit is contained in:
parent
c898f0e2a9
commit
363b93ef25
@ -89,6 +89,7 @@ type ArtistRepository interface {
|
||||
GetIndex(includeMissing bool, libraryIds []int, roles ...Role) (ArtistIndexes, error)
|
||||
|
||||
// The following methods are used exclusively by the scanner:
|
||||
CopyAttributes(fromID, toID string, columns ...string) error
|
||||
RefreshPlayCounts() (int64, error)
|
||||
RefreshStats(allArtists bool) (int64, error)
|
||||
|
||||
|
||||
@ -224,6 +224,34 @@ func (r *artistRepository) Put(a *model.Artist, colsToUpdate ...string) error {
|
||||
return err
|
||||
}
|
||||
|
||||
// CopyAttributes copies the named columns from the row identified by fromID
|
||||
// to the row identified by toID. Used by the scanner after a PID change to
|
||||
// preserve attributes (notably created_at) across artist-ID migrations.
|
||||
func (r *artistRepository) CopyAttributes(fromID, toID string, columns ...string) error {
|
||||
var from dbx.NullStringMap
|
||||
err := r.queryOne(Select(columns...).From(r.tableName).Where(Eq{"id": fromID}), &from)
|
||||
if err != nil {
|
||||
return fmt.Errorf("getting artist to copy fields from: %w", err)
|
||||
}
|
||||
to := make(map[string]any)
|
||||
for _, col := range columns {
|
||||
v := from[col]
|
||||
// created_at on the previous row may be zero/poisoned (e.g. legacy
|
||||
// rows or rows where it was never populated). Skip in that case so we
|
||||
// don't propagate a bad value forward on every metadata-driven ID
|
||||
// change. Matches the behavior in album_repository.CopyAttributes.
|
||||
if col == "created_at" && (!v.Valid || v.String == "" || strings.HasPrefix(v.String, "0001-")) {
|
||||
continue
|
||||
}
|
||||
to[col] = v
|
||||
}
|
||||
if len(to) == 0 {
|
||||
return nil
|
||||
}
|
||||
_, err = r.executeSQL(Update(r.tableName).SetMap(to).Where(Eq{"id": toID}))
|
||||
return err
|
||||
}
|
||||
|
||||
func (r *artistRepository) UpdateExternalInfo(a *model.Artist) error {
|
||||
dba := &dbArtist{Artist: a}
|
||||
_, err := r.put(a.ID, dba,
|
||||
|
||||
@ -496,6 +496,16 @@ func (p *phaseFolders) persistArtist(repo model.ArtistRepository, a *model.Artis
|
||||
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))
|
||||
}
|
||||
|
||||
// Keep created_at field from previous instance of the artist. Without this,
|
||||
// the just-inserted row carries the scan timestamp, breaking "recently
|
||||
// added" semantics on every PID config change.
|
||||
if err := repo.CopyAttributes(prevID, a.ID, "created_at"); err != nil {
|
||||
if !errors.Is(err, model.ErrNotFound) {
|
||||
log.Warn(p.ctx, "Scanner: Could not copy artist fields", "from", prevID, "to", a.ID, "artist", a.Name, err)
|
||||
p.state.sendWarning(fmt.Sprintf("Could not copy artist fields from %s to %s ('%s'): %v", prevID, a.ID, a.Name, err))
|
||||
}
|
||||
}
|
||||
delete(idMap, a.ID)
|
||||
return nil
|
||||
}
|
||||
|
||||
@ -45,5 +45,23 @@ var _ = Describe("phaseFolders", func() {
|
||||
Expect(p.persistArtist(artRepo, &artist, idMap)).To(Succeed())
|
||||
Expect(artRepo.ReassignAnnotationCalls).To(BeEmpty())
|
||||
})
|
||||
|
||||
It("calls CopyAttributes(created_at) 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.CopyAttributesCalls).To(HaveKeyWithValue(oldID, newID))
|
||||
})
|
||||
|
||||
It("does not call CopyAttributes 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.CopyAttributesCalls).To(BeEmpty())
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
@ -20,6 +20,7 @@ type MockArtistRepo struct {
|
||||
Err bool
|
||||
Options model.QueryOptions
|
||||
ReassignAnnotationCalls map[string]string // prevID -> newID
|
||||
CopyAttributesCalls map[string]string // fromID -> toID
|
||||
}
|
||||
|
||||
func (m *MockArtistRepo) SetError(err bool) {
|
||||
@ -171,4 +172,17 @@ func (m *MockArtistRepo) ReassignAnnotation(prevID string, newID string) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// CopyAttributes is a no-op in the mock; tests that need to verify call
|
||||
// observation can extend this.
|
||||
func (m *MockArtistRepo) CopyAttributes(fromID, toID string, columns ...string) error {
|
||||
if m.Err {
|
||||
return errors.New("unexpected error")
|
||||
}
|
||||
if m.CopyAttributesCalls == nil {
|
||||
m.CopyAttributesCalls = make(map[string]string)
|
||||
}
|
||||
m.CopyAttributesCalls[fromID] = toID
|
||||
return nil
|
||||
}
|
||||
|
||||
var _ model.ArtistRepository = (*MockArtistRepo)(nil)
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user