From 363b93ef25e3893fe85d13b715c9378da6037286 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sun, 24 May 2026 21:34:47 -0300 Subject: [PATCH] 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. --- model/artist.go | 1 + persistence/artist_repository.go | 28 ++++++++++++++++++++++++++++ scanner/phase_1_folders.go | 10 ++++++++++ scanner/phase_1_folders_test.go | 18 ++++++++++++++++++ tests/mock_artist_repo.go | 14 ++++++++++++++ 5 files changed, 71 insertions(+) diff --git a/model/artist.go b/model/artist.go index 2085f0051..54671c3ef 100644 --- a/model/artist.go +++ b/model/artist.go @@ -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) diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index cfdc499e0..66276fc76 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -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, diff --git a/scanner/phase_1_folders.go b/scanner/phase_1_folders.go index 8aee6af25..aa67a68b6 100644 --- a/scanner/phase_1_folders.go +++ b/scanner/phase_1_folders.go @@ -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 } diff --git a/scanner/phase_1_folders_test.go b/scanner/phase_1_folders_test.go index f76b63bcb..f3861a430 100644 --- a/scanner/phase_1_folders_test.go +++ b/scanner/phase_1_folders_test.go @@ -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()) + }) }) }) diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index 950fea7de..198564bd0 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -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)