Merge 3ce7fc32bd56add470d57cba0fbaaf0cfdef0f57 into 5a3ac80a8a9dc5229a1dc0fd3105b88265517f3b

This commit is contained in:
Deluan Quintão 2026-06-28 16:08:48 -04:00 committed by GitHub
commit 5d4213f028
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 1010 additions and 122 deletions

View File

@ -175,7 +175,13 @@ func pidHashChanged(ds model.DataStore) (bool, error) {
if err != nil {
return false, err
}
return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) || !strings.EqualFold(pidTrack, conf.Server.PID.Track), nil
pidArtist, err := ds.Property(context.Background()).DefaultGet(consts.PIDArtistKey, "")
if err != nil {
return false, err
}
return !strings.EqualFold(pidAlbum, conf.Server.PID.Album) ||
!strings.EqualFold(pidTrack, conf.Server.PID.Track) ||
!strings.EqualFold(pidArtist, conf.Server.PID.Artist), nil
}
// runInitialScan runs an initial scan of the music library if needed.

View File

@ -243,8 +243,9 @@ type backupOptions struct {
}
type pidOptions struct {
Track string
Album string
Track string
Album string
Artist string
}
type inspectOptions struct {
@ -440,6 +441,7 @@ func Load(noConfigDump bool) {
// Make sure we don't have empty PIDs
Server.PID.Album = cmp.Or(Server.PID.Album, consts.DefaultAlbumPID)
Server.PID.Track = cmp.Or(Server.PID.Track, consts.DefaultTrackPID)
Server.PID.Artist = cmp.Or(Server.PID.Artist, consts.DefaultArtistPID)
// Parse LastFM.Language into Languages slice (comma-separated, with fallback to DefaultInfoLanguage)
Server.LastFM.Languages = parseLanguages(Server.LastFM.Language)
@ -853,6 +855,7 @@ func setViperDefaults() {
viper.SetDefault("backup.count", 0)
viper.SetDefault("pid.track", consts.DefaultTrackPID)
viper.SetDefault("pid.album", consts.DefaultAlbumPID)
viper.SetDefault("pid.artist", consts.DefaultArtistPID)
viper.SetDefault("inspect.enabled", true)
viper.SetDefault("inspect.maxrequests", 1)
viper.SetDefault("inspect.backloglimit", consts.RequestThrottleBacklogLimit)

View File

@ -125,10 +125,12 @@ const (
const (
//DefaultAlbumPID = "album_legacy"
DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate"
DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title"
PIDAlbumKey = "PIDAlbum"
PIDTrackKey = "PIDTrack"
DefaultAlbumPID = "musicbrainz_albumid|albumartistid,album,albumversion,releasedate"
DefaultTrackPID = "musicbrainz_trackid|albumid,discnumber,tracknumber,title"
DefaultArtistPID = "name"
PIDAlbumKey = "PIDAlbum"
PIDTrackKey = "PIDTrack"
PIDArtistKey = "PIDArtist"
)
const (

View File

@ -0,0 +1,11 @@
-- +goose Up
-- Backfill PIDArtist property to reflect the historical artist-ID computation.
-- Existing artist IDs were produced by the legacy hardcoded artistID() function,
-- which is byte-identical to computeArtistPID(p, "name", ...). Recording "name"
-- here ensures that on the next scan, prevArtistPIDConf is never empty — closing
-- the upgrade-time window where a user who pre-configured a non-default PID.Artist
-- would have artist IDs silently regenerated without annotation migration.
insert into property (id, value) values ('PIDArtist', 'name') on conflict do nothing;
-- +goose Down
delete from property where id = 'PIDArtist';

View File

@ -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)

View File

@ -7,64 +7,82 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/utils/str"
"golang.org/x/text/cases"
"golang.org/x/text/language"
)
type roleTags struct {
name model.TagName
sort model.TagName
mbid model.TagName
name model.TagName
sort model.TagName
mbid model.TagName
credit model.TagName
}
var roleMappings = map[model.Role]roleTags{
model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort, mbid: model.TagMusicBrainzComposerID},
model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort, mbid: model.TagMusicBrainzLyricistID},
model.RoleConductor: {name: model.TagConductor, mbid: model.TagMusicBrainzConductorID},
model.RoleArranger: {name: model.TagArranger, mbid: model.TagMusicBrainzArrangerID},
model.RoleDirector: {name: model.TagDirector, mbid: model.TagMusicBrainzDirectorID},
model.RoleProducer: {name: model.TagProducer, mbid: model.TagMusicBrainzProducerID},
model.RoleEngineer: {name: model.TagEngineer, mbid: model.TagMusicBrainzEngineerID},
model.RoleMixer: {name: model.TagMixer, mbid: model.TagMusicBrainzMixerID},
model.RoleRemixer: {name: model.TagRemixer, mbid: model.TagMusicBrainzRemixerID},
model.RoleDJMixer: {name: model.TagDJMixer, mbid: model.TagMusicBrainzDJMixerID},
model.RoleComposer: {name: model.TagComposer, sort: model.TagComposerSort, mbid: model.TagMusicBrainzComposerID, credit: model.TagComposerCredit},
model.RoleLyricist: {name: model.TagLyricist, sort: model.TagLyricistSort, mbid: model.TagMusicBrainzLyricistID, credit: model.TagLyricistCredit},
model.RoleConductor: {name: model.TagConductor, mbid: model.TagMusicBrainzConductorID, credit: model.TagConductorCredit},
model.RoleArranger: {name: model.TagArranger, mbid: model.TagMusicBrainzArrangerID, credit: model.TagArrangerCredit},
model.RoleDirector: {name: model.TagDirector, mbid: model.TagMusicBrainzDirectorID, credit: model.TagDirectorCredit},
model.RoleProducer: {name: model.TagProducer, mbid: model.TagMusicBrainzProducerID, credit: model.TagProducerCredit},
model.RoleEngineer: {name: model.TagEngineer, mbid: model.TagMusicBrainzEngineerID, credit: model.TagEngineerCredit},
model.RoleMixer: {name: model.TagMixer, mbid: model.TagMusicBrainzMixerID, credit: model.TagMixerCredit},
model.RoleRemixer: {name: model.TagRemixer, mbid: model.TagMusicBrainzRemixerID, credit: model.TagRemixerCredit},
model.RoleDJMixer: {name: model.TagDJMixer, mbid: model.TagMusicBrainzDJMixerID, credit: model.TagDJMixerCredit},
}
func (md Metadata) mapParticipants() model.Participants {
participants := make(model.Participants)
// Parse track artists
artists := md.parseArtists(
model.TagTrackArtist, model.TagTrackArtists,
model.TagTrackArtistSort, model.TagTrackArtistsSort,
model.TagMusicBrainzArtistID,
)
participants.Add(model.RoleArtist, artists...)
// Parse track artists. MBIDs go through getRoleValues to split on the same
// separators as the names, keeping positional alignment.
trackNames := md.getArtistValues(model.TagTrackArtist, model.TagTrackArtists)
if len(trackNames) == 0 {
trackNames = []string{consts.UnknownArtist}
}
trackSorts := md.getArtistValues(model.TagTrackArtistSort, model.TagTrackArtistsSort)
trackMbids := md.getRoleValues(model.TagMusicBrainzArtistID)
trackCredits := md.getArtistValues(model.TagTrackArtistCredit, model.TagTrackArtistsCredit)
trackArtistParticipants := md.buildParticipants(trackNames, trackSorts, trackMbids, trackCredits)
participants.AddParticipants(model.RoleArtist, trackArtistParticipants...)
// Parse album artists
albumArtists := md.parseArtists(
model.TagAlbumArtist, model.TagAlbumArtists,
model.TagAlbumArtistSort, model.TagAlbumArtistsSort,
model.TagMusicBrainzAlbumArtistID,
)
if len(albumArtists) == 1 && albumArtists[0].Name == consts.UnknownArtist {
if md.Bool(model.TagCompilation) {
albumArtists = md.buildArtists([]string{consts.VariousArtists}, nil, []string{consts.VariousArtistsMbzId})
} else {
albumArtists = artists
}
}
participants.Add(model.RoleAlbumArtist, albumArtists...)
albumNames := md.getArtistValues(model.TagAlbumArtist, model.TagAlbumArtists)
albumSorts := md.getArtistValues(model.TagAlbumArtistSort, model.TagAlbumArtistsSort)
albumMbids := md.getRoleValues(model.TagMusicBrainzAlbumArtistID)
albumCredits := md.getArtistValues(model.TagAlbumArtistCredit, model.TagAlbumArtistsCredit)
// Parse all other roles
// Treat "no albumartist tag" and "albumartist == UnknownArtist placeholder"
// as missing, matching the prior parseArtists path and defending against
// the placeholder round-tripping back into a tag.
albumArtistMissing := len(albumNames) == 0 ||
(len(albumNames) == 1 && albumNames[0] == consts.UnknownArtist)
var albumArtistParticipants []model.Participant
if albumArtistMissing {
if md.Bool(model.TagCompilation) {
albumArtistParticipants = md.buildParticipants(
[]string{consts.VariousArtists}, nil,
[]string{consts.VariousArtistsMbzId}, nil)
} else {
albumArtistParticipants = trackArtistParticipants
}
} else {
albumArtistParticipants = md.buildParticipants(albumNames, albumSorts, albumMbids, albumCredits)
}
participants.AddParticipants(model.RoleAlbumArtist, albumArtistParticipants...)
// All parallel lists go through getRoleValues so they split consistently
// with names (e.g. COMPOSER="A;B" + COMPOSER_CREDIT="AA;BB" → 2 each).
for role, info := range roleMappings {
names := md.getRoleValues(info.name)
if len(names) > 0 {
sorts := md.Strings(info.sort)
mbids := md.Strings(info.mbid)
artists := md.buildArtists(names, sorts, mbids)
participants.Add(role, artists...)
sorts := md.getRoleValues(info.sort)
mbids := md.getRoleValues(info.mbid)
credits := md.getRoleValues(info.credit)
participants.AddParticipants(role, md.buildParticipants(names, sorts, mbids, credits)...)
}
}
@ -100,12 +118,20 @@ func (md Metadata) processPerformers(participants model.Participants, rolesMbzId
subRole := titleCaser.String(performer.Key())
artist := model.Artist{
ID: md.artistID(name),
Name: name,
OrderArtistName: str.SanitizeFieldForSortingNoArticle(name),
MbzArtistID: md.getPerformerMbid(subRole, rolesMbzIdMap, roleIdx),
}
participants.AddWithSubRole(model.RolePerformer, subRole, artist)
artist.ID = computeArtistPID(
model.Participant{Artist: artist},
conf.Server.PID.Artist,
id.NewHash,
)
participants.AddParticipants(model.RolePerformer, model.Participant{
Artist: artist,
SubRole: subRole,
CreditedAs: name,
})
}
}
@ -138,25 +164,31 @@ func (md Metadata) syncMissingMbzIDs(participants model.Participants) {
}
}
func (md Metadata) parseArtists(
name model.TagName, names model.TagName, sort model.TagName,
sorts model.TagName, mbid model.TagName,
) []model.Artist {
nameValues := md.getArtistValues(name, names)
sortValues := md.getArtistValues(sort, sorts)
mbids := md.Strings(mbid)
if len(nameValues) == 0 {
nameValues = []string{consts.UnknownArtist}
// buildParticipants wraps each Artist into a Participant with CreditedAs
// populated. credits pairs positionally with names; a length mismatch falls
// back to the canonical name for every entry.
func (md Metadata) buildParticipants(names, sorts, mbids, credits []string) []model.Participant {
if len(credits) != 0 && len(credits) != len(names) {
credits = nil
}
return md.buildArtists(nameValues, sortValues, mbids)
artists := md.buildArtists(names, sorts, mbids)
out := make([]model.Participant, len(artists))
for i, a := range artists {
p := model.Participant{Artist: a}
if i < len(credits) && credits[i] != "" {
p.CreditedAs = credits[i]
} else {
p.CreditedAs = a.Name
}
out[i] = p
}
return out
}
func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist {
var artists []model.Artist
for i, name := range names {
id := md.artistID(name)
artist := model.Artist{
ID: id,
Name: name,
OrderArtistName: str.SanitizeFieldForSortingNoArticle(name),
}
@ -166,6 +198,12 @@ func (md Metadata) buildArtists(names, sorts, mbids []string) []model.Artist {
if i < len(mbids) {
artist.MbzArtistID = mbids[i]
}
// Assign ID after Sort/MBID are set so MBID/sort-based PID specs resolve.
artist.ID = computeArtistPID(
model.Participant{Artist: artist},
conf.Server.PID.Artist,
id.NewHash,
)
artists = append(artists, artist)
}
return artists

View File

@ -460,6 +460,26 @@ var _ = Describe("Participants", func() {
})
})
When("the COMPILATION tag is true and ALBUMARTIST is the UnknownArtist placeholder", func() {
BeforeEach(func() {
// Some rippers emit the literal '[Unknown Artist]' string
// when no album artist is set. The fallback must still
// route to Various Artists for compilations.
mf = toMediaFile(model.RawTags{
"COMPILATION": {"1"},
"ALBUMARTIST": {consts.UnknownArtist},
})
})
It("should substitute Various Artists as the album artist", func() {
participants := mf.Participants
Expect(participants).To(HaveKeyWithValue(model.RoleAlbumArtist, HaveLen(1)))
albumArtist := participants[model.RoleAlbumArtist][0]
Expect(albumArtist.Name).To(Equal("Various Artists"))
Expect(albumArtist.MbzArtistID).To(Equal(consts.VariousArtistsMbzId))
})
})
When("the COMPILATION tag is true and there are ALBUMARTIST tags", func() {
BeforeEach(func() {
mf = toMediaFile(model.RawTags{
@ -802,4 +822,105 @@ var _ = Describe("Participants", func() {
}
})
})
Describe("CreditedAs population", func() {
It("uses the canonical name as CreditedAs when no credit tag is present", func() {
mf = toMediaFile(model.RawTags{
"ARTISTS": {"Some Artist"},
})
artists := mf.Participants[model.RoleArtist]
Expect(artists).To(HaveLen(1))
Expect(artists[0].Name).To(Equal("Some Artist"))
Expect(artists[0].CreditedAs).To(Equal("Some Artist"))
})
It("uses the credit tag value when present, paired positionally", func() {
mf = toMediaFile(model.RawTags{
"ARTISTS": {"Planetary Assault Systems", "Other"},
"ARTISTSCREDIT": {"PAS", "Other"},
})
artists := mf.Participants[model.RoleArtist]
Expect(artists).To(HaveLen(2))
Expect(artists[0].Name).To(Equal("Planetary Assault Systems"))
Expect(artists[0].CreditedAs).To(Equal("PAS"))
Expect(artists[1].Name).To(Equal("Other"))
Expect(artists[1].CreditedAs).To(Equal("Other"))
})
It("falls back to canonical name when credit list length differs from name list", func() {
mf = toMediaFile(model.RawTags{
"ARTISTS": {"A", "B", "C"},
"ARTISTSCREDIT": {"only one"}, // mismatch
})
artists := mf.Participants[model.RoleArtist]
Expect(artists).To(HaveLen(3))
for _, a := range artists {
Expect(a.CreditedAs).To(Equal(a.Name))
}
})
It("populates CreditedAs for non-artist roles (composer)", func() {
mf = toMediaFile(model.RawTags{
"COMPOSER": {"Real Composer"},
"COMPOSERCREDIT": {"R. Composer"},
})
composers := mf.Participants[model.RoleComposer]
Expect(composers).To(HaveLen(1))
Expect(composers[0].CreditedAs).To(Equal("R. Composer"))
})
It("always populates CreditedAs (never empty)", func() {
mf = toMediaFile(model.RawTags{
"ARTIST": {"Track Artist"},
"ALBUMARTIST": {"Album Artist"},
"COMPOSER": {"A Composer"},
"PERFORMER:GUITAR": {"A Guitarist"},
})
for role, list := range mf.Participants {
for _, p := range list {
Expect(p.CreditedAs).NotTo(BeEmpty(), "role: %s, participant: %+v", role, p)
}
}
})
It("aligns composer credits with names when both use a role separator", func() {
// Names and credits both split on ';' under the roles.split config.
// Prior to the alignment fix, credits were read raw and would have
// stayed as one value while names became three, producing a length
// mismatch that silently dropped all credits.
mf = toMediaFile(model.RawTags{
"COMPOSER": {"Comp A;Comp B;Comp C"},
"COMPOSERCREDIT": {"CA;CB;CC"},
})
composers := mf.Participants[model.RoleComposer]
Expect(composers).To(HaveLen(3))
Expect(composers[0].Name).To(Equal("Comp A"))
Expect(composers[0].CreditedAs).To(Equal("CA"))
Expect(composers[1].CreditedAs).To(Equal("CB"))
Expect(composers[2].CreditedAs).To(Equal("CC"))
})
It("splits composer MBIDs to align with split composer names", func() {
// Both names and MBIDs use the role separator ';'. The fix routes
// MBIDs through getRoleValues so they split alongside names rather
// than staying as a single value (which would have produced 2 names
// and 1 MBID — broken positional alignment).
//
// We test via a single delimited canonical name string AND a single
// delimited MBID-pair string. Note: the tag-reader UUID validation
// happens at metadata parse time on individual values, so we hand
// multi-valued tag entries already split into separate slice
// entries (matching how a multi-value tag would surface).
mf = toMediaFile(model.RawTags{
"COMPOSER": {"Comp A;Comp B"},
"MUSICBRAINZ_COMPOSERID": {"11111111-1111-1111-1111-111111111111", "22222222-2222-2222-2222-222222222222"},
})
composers := mf.Participants[model.RoleComposer]
Expect(composers).To(HaveLen(2))
Expect(composers[0].Name).To(Equal("Comp A"))
Expect(composers[0].MbzArtistID).To(Equal("11111111-1111-1111-1111-111111111111"))
Expect(composers[1].Name).To(Equal("Comp B"))
Expect(composers[1].MbzArtistID).To(Equal("22222222-2222-2222-2222-222222222222"))
})
})
})

View File

@ -87,10 +87,62 @@ func (md Metadata) albumID(mf model.MediaFile, pidConf string) string {
return computePID(mf, md, pidConf, true, id.NewHash)
}
// BFR Must be configurable?
func (md Metadata) artistID(name string) string {
mf := model.MediaFile{AlbumArtist: name}
return computePID(mf, md, "albumartistid", false, id.NewHash)
// computeArtistPID computes a persistent ID for a single participant using the
// given spec. The spec grammar is the same pipe/comma form used by Album/Track
// PIDs. Attributes recognised:
// - name → hash(clear(lower(participant.Name))) — the inner
// hash matches the historical 'albumartistid' path
// so the default spec "name" produces byte-identical
// IDs to the previous hardcoded artistID().
// - musicbrainz_artistid → participant.MbzArtistID (opaque identifier — no
// normalization).
// - sort_name → participant.SortArtistName (raw — sort forms are
// already canonicalized by taggers, and folding case
// could collapse legitimately distinct sort variants).
//
// Unlike album/track PIDs, no library prefix is applied: artists are shared
// across libraries by design.
func computeArtistPID(p model.Participant, spec string, hash hashFunc) string {
pid := ""
fields := strings.SplitSeq(spec, "|")
for field := range fields {
attributes := strings.Split(field, ",")
values := make([]string, len(attributes))
hasValue := false
for i, attr := range attributes {
v := getArtistPIDAttr(p, attr, hash)
if v != "" {
hasValue = true
}
values[i] = v
}
if hasValue {
pid += strings.Join(values, "\\")
break
}
}
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":
if p.Name == "" {
return ""
}
return hash(str.Clear(strings.ToLower(p.Name)))
case "musicbrainz_artistid":
return p.MbzArtistID
case "sort_name":
return p.SortArtistName
}
return ""
}
func (md Metadata) mapTrackTitle() string {

View File

@ -5,7 +5,9 @@ import (
"github.com/navidrome/navidrome/conf"
"github.com/navidrome/navidrome/conf/configtest"
"github.com/navidrome/navidrome/consts"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/navidrome/navidrome/tests"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
@ -291,3 +293,107 @@ var _ = Describe("getPID", func() {
})
})
})
var _ = Describe("computeArtistPID", func() {
var p model.Participant
BeforeEach(func() {
DeferCleanup(configtest.SetupConfig())
p = model.Participant{Artist: model.Artist{Name: "The Beatles"}}
})
Context("default spec 'name'", func() {
BeforeEach(func() { conf.Server.PID.Artist = consts.DefaultArtistPID })
It("produces the same ID as the legacy artistID(name) hardcoded function", func() {
// Legacy formula: hash(hash(clear(lower(name))))
// It came from artistID(name) routing through computePID(spec="albumartistid"),
// which inside getPIDAttr returns hash(clear(lower(mf.AlbumArtist))), then
// computePID hashes that again.
mfWithAlbumArtist := model.MediaFile{AlbumArtist: p.Name}
legacyMD := Metadata{}
legacyID := computePID(mfWithAlbumArtist, legacyMD, "albumartistid", false, id.NewHash)
newID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)
Expect(newID).To(Equal(legacyID))
})
It("normalizes name (clear(lower(...))) before hashing", func() {
// Different casing / punctuation must collapse to the same ID.
p2 := model.Participant{Artist: model.Artist{Name: "the beatles"}}
Expect(computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)).
To(Equal(computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash)))
})
})
Context("spec 'musicbrainz_artistid|name'", func() {
BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|name" })
It("uses MBID when present", func() {
p.MbzArtistID = "mbid-1"
withMBID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)
p2 := model.Participant{Artist: model.Artist{Name: "Different", MbzArtistID: "mbid-1"}}
withMBIDOtherName := computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash)
Expect(withMBID).To(Equal(withMBIDOtherName))
})
It("falls back to normalized name when MBID is missing", func() {
withoutMBID := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)
byName := computeArtistPID(p, "name", id.NewHash)
Expect(withoutMBID).To(Equal(byName))
})
})
Context("composite spec 'musicbrainz_artistid|sort_name,name'", func() {
BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|sort_name,name" })
It("hashes sort+name composite when MBID is absent", func() {
p.SortArtistName = "Beatles, The"
withSort := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)
p2 := p
p2.SortArtistName = "Different Sort"
withDifferentSort := computeArtistPID(p2, conf.Server.PID.Artist, id.NewHash)
Expect(withSort).NotTo(Equal(withDifferentSort))
})
})
Context("spec 'sort_name|name' with empty sort_name", func() {
BeforeEach(func() { conf.Server.PID.Artist = "sort_name|name" })
It("falls through to name when SortArtistName is empty", func() {
// sort_name is empty → first pipe alternative produces no value →
// engine falls through to name.
fallback := computeArtistPID(p, conf.Server.PID.Artist, id.NewHash)
byName := computeArtistPID(p, "name", id.NewHash)
Expect(fallback).To(Equal(byName))
})
})
Context("empty inputs", func() {
BeforeEach(func() { conf.Server.PID.Artist = "musicbrainz_artistid|name" })
It("does not panic on empty participant", func() {
empty := model.Participant{Artist: model.Artist{Name: ""}}
Expect(func() { computeArtistPID(empty, conf.Server.PID.Artist, id.NewHash) }).
NotTo(Panic())
})
})
It("buildArtists produces the same Artist.ID as the legacy artistID() under default PID.Artist", func() {
DeferCleanup(configtest.SetupConfig())
conf.Server.PID.Artist = consts.DefaultArtistPID
name := "Some Artist"
md := Metadata{}
artists := md.buildArtists([]string{name}, nil, nil)
// Legacy result computed independently via the old "albumartistid" path:
legacyMF := model.MediaFile{AlbumArtist: name}
expected := computePID(legacyMF, Metadata{}, "albumartistid", false, id.NewHash)
Expect(artists).To(HaveLen(1))
Expect(artists[0].ID).To(Equal(expected))
})
})

View File

@ -77,7 +77,12 @@ func RoleFromString(role string) Role {
type Participant struct {
Artist
SubRole string `json:"subRole,omitempty"`
SubRole string `json:"subRole,omitempty"`
CreditedAs string `json:"creditedAs,omitempty"`
}
func (p Participant) DisplayName() string {
return cmp.Or(p.CreditedAs, p.Name)
}
type ParticipantList []Participant
@ -91,6 +96,18 @@ func (p ParticipantList) Join(sep string) string {
}), sep)
}
// JoinCredited joins the credited names of the participants with sep.
// Falls back to Name when CreditedAs is empty.
func (p ParticipantList) JoinCredited(sep string) string {
return strings.Join(slice.Map(p, func(part Participant) string {
n := part.DisplayName()
if part.SubRole != "" {
return n + " (" + part.SubRole + ")"
}
return n
}), sep)
}
type Participants map[Role]ParticipantList
// Add adds the artists to the role, ignoring duplicates.
@ -109,6 +126,12 @@ func (p Participants) AddWithSubRole(role Role, subRole string, artists ...Artis
p.add(role, participants...)
}
// AddParticipants adds Participants directly (preserving CreditedAs and SubRole),
// ignoring duplicates.
func (p Participants) AddParticipants(role Role, participants ...Participant) {
p.add(role, participants...)
}
func (p Participants) Sort() {
for _, artists := range p {
slices.SortFunc(artists, func(a1, a2 Participant) int {
@ -133,16 +156,28 @@ func (p Participants) Merge(other Participants) {
}
func (p Participants) add(role Role, participants ...Participant) {
seen := make(map[string]struct{}, len(p[role]))
for _, artist := range p[role] {
seen[artist.ID+artist.SubRole] = struct{}{}
// Use a separator that can't appear in either field so e.g.
// (ID="12", SubRole="3") doesn't collide with (ID="1", SubRole="23").
const sep = "\x00"
seen := make(map[string]int, len(p[role]))
for i, artist := range p[role] {
seen[artist.ID+sep+artist.SubRole] = i
}
for _, participant := range participants {
key := participant.ID + participant.SubRole
if _, ok := seen[key]; !ok {
seen[key] = struct{}{}
p[role] = append(p[role], participant)
key := participant.ID + sep + participant.SubRole
if idx, ok := seen[key]; ok {
// Same artist/sub-role seen before. The merge (e.g. building
// album.Participants from per-track participants) is inherently
// lossy when tracks differ on CreditedAs, but silently dropping
// the later value can mean the wrong credit ends up on the album.
// Prefer the most recently observed non-empty CreditedAs.
if participant.CreditedAs != "" {
p[role][idx].CreditedAs = participant.CreditedAs
}
continue
}
seen[key] = len(p[role])
p[role] = append(p[role], participant)
}
}

View File

@ -116,6 +116,40 @@ var _ = Describe("Participants", func() {
RoleAlbumArtist: []Participant{_p("3", "AlbumArtist1"), _p("4", "AlbumArtist2"), _p("7", "AlbumArtist3"), _p("8", "AlbumArtist4")},
}))
})
It("upgrades CreditedAs from a later occurrence when the dedup key matches", func() {
// Same artist (same ID+SubRole) credited differently on two tracks.
// The merge is inherently lossy; the later non-empty credit wins so
// it isn't silently dropped just because of arrival order.
p1 := Participants{
RoleArtist: []Participant{
{Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit One"},
},
}
p2 := Participants{
RoleArtist: []Participant{
{Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit Two"},
},
}
p1.Merge(p2)
Expect(p1[RoleArtist]).To(HaveLen(1))
Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Credit Two"))
})
It("does not overwrite an existing CreditedAs with an empty one", func() {
p1 := Participants{
RoleArtist: []Participant{
{Artist: Artist{ID: "a1", Name: "Canonical"}, CreditedAs: "Credit One"},
},
}
p2 := Participants{
RoleArtist: []Participant{
{Artist: Artist{ID: "a1", Name: "Canonical"}}, // empty CreditedAs
},
}
p1.Merge(p2)
Expect(p1[RoleArtist][0].CreditedAs).To(Equal("Credit One"))
})
})
Describe("Hash", func() {
@ -182,6 +216,36 @@ var _ = Describe("Participants", func() {
})
})
var _ = Describe("Participant.CreditedAs", func() {
It("round-trips through JSON when set", func() {
p := Participant{
Artist: Artist{ID: "abc", Name: "Planetary Assault Systems"},
CreditedAs: "PAS",
}
data, err := json.Marshal(p)
Expect(err).ToNot(HaveOccurred())
Expect(string(data)).To(ContainSubstring(`"creditedAs":"PAS"`))
var back Participant
Expect(json.Unmarshal(data, &back)).To(Succeed())
Expect(back.CreditedAs).To(Equal("PAS"))
})
It("omits CreditedAs from JSON when empty", func() {
p := Participant{Artist: Artist{ID: "abc", Name: "Foo"}}
data, err := json.Marshal(p)
Expect(err).ToNot(HaveOccurred())
Expect(string(data)).NotTo(ContainSubstring("creditedAs"))
})
It("deserializes legacy JSON without the creditedAs key", func() {
legacy := `{"id":"abc","name":"Foo","subRole":""}`
var p Participant
Expect(json.Unmarshal([]byte(legacy), &p)).To(Succeed())
Expect(p.CreditedAs).To(Equal(""))
})
})
var _ = Describe("ParticipantList", func() {
Describe("Join", func() {
It("joins the participants with the given separator", func() {

View File

@ -205,27 +205,44 @@ const (
// Artists and roles
TagAlbumArtist TagName = "albumartist"
TagAlbumArtists TagName = "albumartists"
TagAlbumArtistSort TagName = "albumartistsort"
TagAlbumArtistsSort TagName = "albumartistssort"
TagTrackArtist TagName = "artist"
TagTrackArtists TagName = "artists"
TagTrackArtistSort TagName = "artistsort"
TagTrackArtistsSort TagName = "artistssort"
TagComposer TagName = "composer"
TagComposerSort TagName = "composersort"
TagLyricist TagName = "lyricist"
TagLyricistSort TagName = "lyricistsort"
TagDirector TagName = "director"
TagProducer TagName = "producer"
TagEngineer TagName = "engineer"
TagMixer TagName = "mixer"
TagRemixer TagName = "remixer"
TagDJMixer TagName = "djmixer"
TagConductor TagName = "conductor"
TagArranger TagName = "arranger"
TagPerformer TagName = "performer"
TagAlbumArtist TagName = "albumartist"
TagAlbumArtists TagName = "albumartists"
TagAlbumArtistSort TagName = "albumartistsort"
TagAlbumArtistsSort TagName = "albumartistssort"
TagAlbumArtistCredit TagName = "albumartistcredit"
TagAlbumArtistsCredit TagName = "albumartistscredit"
TagTrackArtist TagName = "artist"
TagTrackArtists TagName = "artists"
TagTrackArtistSort TagName = "artistsort"
TagTrackArtistsSort TagName = "artistssort"
TagTrackArtistCredit TagName = "artistcredit"
TagTrackArtistsCredit TagName = "artistscredit"
TagComposer TagName = "composer"
TagComposerSort TagName = "composersort"
TagComposerCredit TagName = "composercredit"
TagLyricist TagName = "lyricist"
TagLyricistSort TagName = "lyricistsort"
TagLyricistCredit TagName = "lyricistcredit"
TagDirector TagName = "director"
TagDirectorCredit TagName = "directorcredit"
TagProducer TagName = "producer"
TagProducerCredit TagName = "producercredit"
TagEngineer TagName = "engineer"
TagEngineerCredit TagName = "engineercredit"
TagMixer TagName = "mixer"
TagMixerCredit TagName = "mixercredit"
TagRemixer TagName = "remixer"
TagRemixerCredit TagName = "remixercredit"
TagDJMixer TagName = "djmixer"
TagDJMixerCredit TagName = "djmixercredit"
TagConductor TagName = "conductor"
TagConductorCredit TagName = "conductorcredit"
TagArranger TagName = "arranger"
TagArrangerCredit TagName = "arrangercredit"
TagPerformer TagName = "performer"
// Performer credits are not currently surfaced: performer tags are
// `type: pair` ('PERFORMER:instrument') and the matching credit
// representation hasn't been designed yet. Tracked separately.
// ReplayGain

View File

@ -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,

View File

@ -10,9 +10,10 @@ import (
)
type participant struct {
ID string `json:"id"`
Name string `json:"name"`
SubRole string `json:"subRole,omitempty"`
ID string `json:"id"`
Name string `json:"name"`
SubRole string `json:"subRole,omitempty"`
CreditedAs string `json:"creditedAs,omitempty"`
}
// flatParticipant represents a flattened participant structure for SQL processing
@ -26,7 +27,12 @@ func marshalParticipants(participants model.Participants) string {
dbParticipants := make(map[model.Role][]participant)
for role, artists := range participants {
for _, artist := range artists {
dbParticipants[role] = append(dbParticipants[role], participant{ID: artist.ID, SubRole: artist.SubRole, Name: artist.Name})
dbParticipants[role] = append(dbParticipants[role], participant{
ID: artist.ID,
Name: artist.Name,
SubRole: artist.SubRole,
CreditedAs: artist.CreditedAs,
})
}
}
res, _ := json.Marshal(dbParticipants)
@ -43,7 +49,11 @@ func unmarshalParticipants(data string) (model.Participants, error) {
participants := make(model.Participants, len(dbParticipants))
for role, participantList := range dbParticipants {
artists := slice.Map(participantList, func(p participant) model.Participant {
return model.Participant{Artist: model.Artist{ID: p.ID, Name: p.Name}, SubRole: p.SubRole}
return model.Participant{
Artist: model.Artist{ID: p.ID, Name: p.Name},
SubRole: p.SubRole,
CreditedAs: p.CreditedAs,
}
})
participants[role] = artists
}

View File

@ -0,0 +1,51 @@
package persistence
import (
"github.com/navidrome/navidrome/model"
. "github.com/onsi/ginkgo/v2"
. "github.com/onsi/gomega"
)
var _ = Describe("sql_participations", func() {
Describe("marshalParticipants/unmarshalParticipants", func() {
It("preserves CreditedAs through marshal/unmarshal round-trip", func() {
original := model.Participants{
model.RoleArtist: model.ParticipantList{
{Artist: model.Artist{ID: "canon-1", Name: "Planetary Assault Systems"}, CreditedAs: "PAS"},
},
model.RoleComposer: model.ParticipantList{
{Artist: model.Artist{ID: "canon-2", Name: "John Lennon"}, CreditedAs: "J. Lennon"},
},
}
serialized := marshalParticipants(original)
restored, err := unmarshalParticipants(serialized)
Expect(err).ToNot(HaveOccurred())
Expect(restored[model.RoleArtist]).To(HaveLen(1))
Expect(restored[model.RoleArtist][0].CreditedAs).To(Equal("PAS"))
Expect(restored[model.RoleComposer][0].CreditedAs).To(Equal("J. Lennon"))
})
It("preserves SubRole through marshal/unmarshal round-trip", func() {
original := model.Participants{
model.RolePerformer: model.ParticipantList{
{Artist: model.Artist{ID: "p-1", Name: "Performer"}, SubRole: "Guitar"},
},
}
serialized := marshalParticipants(original)
restored, err := unmarshalParticipants(serialized)
Expect(err).ToNot(HaveOccurred())
Expect(restored[model.RolePerformer]).To(HaveLen(1))
Expect(restored[model.RolePerformer][0].SubRole).To(Equal("Guitar"))
})
It("does not include CreditedAs in serialized JSON when empty (omitempty)", func() {
p := model.Participants{
model.RoleArtist: model.ParticipantList{
{Artist: model.Artist{ID: "canon-1", Name: "Foo"}},
},
}
serialized := marshalParticipants(p)
Expect(serialized).NotTo(ContainSubstring("creditedAs"))
})
})
})

View File

@ -38,8 +38,14 @@ main:
aliases: [ txxx:artists, artists, ----:com.apple.itunes:artists, wm/artists ]
artistssort:
aliases: [ artistssort ]
artistcredit:
aliases: [ artist_credit, artistcredit ]
artistscredit:
aliases: [ artists_credit, artistscredit ]
arranger:
aliases: [ tipl:arranger, ipls:arranger, arranger ]
arrangercredit:
aliases: [ arranger_credit, arrangercredit ]
composer:
aliases: [ tcom, composer, ©wrt, wm/composer, imus,
writer, txxx:writer, iwri,
@ -50,24 +56,42 @@ main:
# aliases: [ WRITER, TXXX:Writer, IWRI ]
composersort:
aliases: [ tsoc, txxx:composersort, composersort, soco, wm/composersortorder ]
composercredit:
aliases: [ composer_credit, composercredit ]
lyricist:
aliases: [ text, lyricist, ----:com.apple.itunes:lyricist, wm/writer ]
lyricistsort:
aliases: [ lyricistsort ]
lyricistcredit:
aliases: [ lyricist_credit, lyricistcredit ]
conductor:
aliases: [ tpe3, conductor, ----:com.apple.itunes:conductor, wm/conductor ]
conductorcredit:
aliases: [ conductor_credit, conductorcredit ]
director:
aliases: [ txxx:director, director, ©dir, wm/director ]
directorcredit:
aliases: [ director_credit, directorcredit ]
djmixer:
aliases: [ tipl:dj-mix, ipls:dj-mix, djmixer, ----:com.apple.itunes:djmixer, wm/djmixer ]
djmixercredit:
aliases: [ djmixer_credit, djmixercredit ]
mixer:
aliases: [ tipl:mix, ipls:mix, mixer, ----:com.apple.itunes:mixer, wm/mixer ]
mixercredit:
aliases: [ mixer_credit, mixercredit ]
engineer:
aliases: [ tipl:engineer, ipls:engineer, engineer, ----:com.apple.itunes:engineer, wm/engineer, ieng ]
engineercredit:
aliases: [ engineer_credit, engineercredit ]
producer:
aliases: [ tipl:producer, ipls:producer, producer, ----:com.apple.itunes:producer, wm/producer, ipro ]
producercredit:
aliases: [ producer_credit, producercredit ]
remixer:
aliases: [ tpe4, remixer, mixartist, ----:com.apple.itunes:remixer, wm/modifiedby ]
remixercredit:
aliases: [ remixer_credit, remixercredit ]
albumartist:
aliases: [ tpe2, albumartist, album artist, album_artist, aart, wm/albumartist ]
albumartistsort:
@ -76,6 +100,10 @@ main:
aliases: [ txxx:album artists, albumartists ]
albumartistssort:
aliases: [ albumartistssort ]
albumartistcredit:
aliases: [ albumartist_credit, albumartistcredit ]
albumartistscredit:
aliases: [ albumartists_credit, albumartistscredit ]
album:
aliases: [ talb, album, ©alb, wm/albumtitle, iprd ]
albumsort:

View File

@ -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
}

View File

@ -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,30 @@ 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.
//
// The map is keyed by the new artist ID so it stores ONE previous ID per
// new ID, even when many old IDs collapse to the same new one (e.g.
// switching from "name" to "musicbrainz_artistid|name" where multiple
// name-keyed IDs share an MBID). This is intentional: ReassignAnnotation
// is idempotent and the other prior IDs orphan their annotations into
// the unique-constraint collision case documented in the spec — a
// known limitation tracked for a separate follow-up that fixes
// ReassignAnnotation merge semantics at the SQL level.
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 +388,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 +489,36 @@ 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))
}
// 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
}
func (p *phaseFolders) logFolder(entry *folderEntry) (*folderEntry, error) {
logCall := log.Info
if entry.isEmpty() {

View File

@ -0,0 +1,67 @@
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())
})
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())
})
})
})

View File

@ -309,6 +309,11 @@ func (s *scannerImpl) runUpdateLibraries(ctx context.Context, state *scanState)
log.Error(ctx, "Scanner: Error updating album PID conf", err)
return fmt.Errorf("updating album PID conf: %w", err)
}
err = tx.Property(ctx).Put(consts.PIDArtistKey, conf.Server.PID.Artist)
if err != nil {
log.Error(ctx, "Scanner: Error updating artist PID conf", err)
return fmt.Errorf("updating artist PID conf: %w", err)
}
if state.changesDetected.Load() {
log.Debug(ctx, "Scanner: Refreshing library stats", "lib", lib.Name)
if err := tx.Library(ctx).RefreshStats(lib.ID); err != nil {

View File

@ -272,7 +272,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op
child.DisplayAlbumArtist = mf.AlbumArtist
child.AlbumArtists = artistRefs(mf.Participants[model.RoleAlbumArtist])
var contributors []responses.Contributor
child.DisplayComposer = mf.Participants[model.RoleComposer].Join(consts.ArtistJoiner)
child.DisplayComposer = mf.Participants[model.RoleComposer].JoinCredited(consts.ArtistJoiner)
for role, participants := range mf.Participants {
if role == model.RoleArtist || role == model.RoleAlbumArtist {
continue
@ -283,7 +283,7 @@ func osChildFromMediaFile(ctx context.Context, mf model.MediaFile) *responses.Op
SubRole: participant.SubRole,
Artist: responses.ArtistID3Ref{
Id: participant.ID,
Name: participant.Name,
Name: participant.DisplayName(),
},
})
}
@ -303,7 +303,7 @@ func artistRefs(participants model.ParticipantList) []responses.ArtistID3Ref {
return slice.Map(participants, func(p model.Participant) responses.ArtistID3Ref {
return responses.ArtistID3Ref{
Id: p.ID,
Name: p.Name,
Name: p.DisplayName(),
}
})
}

View File

@ -318,6 +318,45 @@ var _ = Describe("helpers", func() {
Expect(child.Title).To(Equal(""))
})
})
It("returns CreditedAs as the contributor name when present, with canonical artist ID", func() {
mf := model.MediaFile{
ID: "song-1",
Participants: model.Participants{
model.RoleArtist: model.ParticipantList{
{Artist: model.Artist{ID: "canon-1", Name: "Planetary Assault Systems"}, CreditedAs: "PAS"},
},
model.RoleComposer: model.ParticipantList{
{Artist: model.Artist{ID: "canon-2", Name: "Real Composer"}, CreditedAs: "R. Composer"},
},
},
}
child := childFromMediaFile(context.Background(), mf)
Expect(child.OpenSubsonicChild).NotTo(BeNil())
Expect(child.OpenSubsonicChild.Artists).To(HaveLen(1))
Expect(child.OpenSubsonicChild.Artists[0].Id).To(Equal("canon-1"))
Expect(child.OpenSubsonicChild.Artists[0].Name).To(Equal("PAS"))
Expect(child.OpenSubsonicChild.Contributors).To(HaveLen(1))
Expect(child.OpenSubsonicChild.Contributors[0].Artist.Id).To(Equal("canon-2"))
Expect(child.OpenSubsonicChild.Contributors[0].Artist.Name).To(Equal("R. Composer"))
Expect(child.OpenSubsonicChild.DisplayComposer).To(Equal("R. Composer"))
})
It("falls back to canonical Name when CreditedAs is empty (legacy participant rows)", func() {
mf := model.MediaFile{
ID: "song-2",
Participants: model.Participants{
model.RoleArtist: model.ParticipantList{
{Artist: model.Artist{ID: "canon-3", Name: "Some Artist"}}, // no CreditedAs
},
},
}
child := childFromMediaFile(context.Background(), mf)
Expect(child.OpenSubsonicChild).NotTo(BeNil())
Expect(child.OpenSubsonicChild.Artists[0].Name).To(Equal("Some Artist"))
})
})
Describe("osChildFromMediaFile", func() {

View File

@ -16,9 +16,11 @@ 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
CopyAttributesCalls map[string]string // fromID -> toID
}
func (m *MockArtistRepo) SetError(err bool) {
@ -157,4 +159,30 @@ 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
}
// 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)

View File

@ -12,6 +12,10 @@ const ALink = withWidth()((props) => {
const artistLink = useGetHandleArtistClick(width)
const dispatch = useDispatch()
const displayName = artist.creditedAs || artist.name
const showCanonicalTooltip =
artist.creditedAs && artist.creditedAs !== artist.name
return (
<Link
key={artist.id}
@ -20,9 +24,10 @@ const ALink = withWidth()((props) => {
e.stopPropagation()
dispatch(closeExtendedInfoDialog())
}}
title={showCanonicalTooltip ? artist.name : undefined}
{...rest}
>
{artist.name}
{displayName}
{artist.subroles?.length > 0 ? ` (${artist.subroles.join(', ')})` : ''}
</Link>
)
@ -37,7 +42,12 @@ const parseAndReplaceArtists = (
let lastIndex = 0
albumArtists?.forEach((artist) => {
const index = displayAlbumArtist.indexOf(artist.name, lastIndex)
// Match on the canonical name that's what appears in displayAlbumArtist
// (sourced from mf.Artist / mf.AlbumArtist, which are set from the canonical
// ARTIST tag at scan time). The ALink itself renders creditedAs || name,
// so the displayed link text still reflects the credit.
const matchName = artist.name
const index = displayAlbumArtist.indexOf(matchName, lastIndex)
if (index !== -1) {
// Add text before the artist name
if (index > lastIndex) {
@ -47,7 +57,7 @@ const parseAndReplaceArtists = (
result.push(
<ALink artist={artist} className={className} key={artist.id} />,
)
lastIndex = index + artist.name.length
lastIndex = index + matchName.length
}
})

View File

@ -213,6 +213,113 @@ describe('ArtistLinkField', () => {
})
})
describe('creditedAs', () => {
it('renders creditedAs as the link text when present', () => {
const record = {
artist: 'PAS',
participants: {
artist: [
{
id: 'canon-1',
name: 'Planetary Assault Systems',
creditedAs: 'PAS',
},
],
},
}
render(<ArtistLinkField record={record} source="artist" />)
expect(screen.getByText('PAS')).toBeInTheDocument()
expect(
screen.queryByText('Planetary Assault Systems'),
).not.toBeInTheDocument()
})
it('sets a title tooltip with the canonical name when creditedAs differs', () => {
const record = {
artist: 'PAS',
participants: {
artist: [
{
id: 'canon-1',
name: 'Planetary Assault Systems',
creditedAs: 'PAS',
},
],
},
}
render(<ArtistLinkField record={record} source="artist" />)
const link = screen.getByRole('link')
expect(link).toHaveAttribute('title', 'Planetary Assault Systems')
})
it('falls back to name when creditedAs is missing', () => {
const record = {
artist: 'Some Artist',
participants: {
artist: [{ id: 'canon-2', name: 'Some Artist' }],
},
}
render(<ArtistLinkField record={record} source="artist" />)
expect(screen.getByText('Some Artist')).toBeInTheDocument()
const link = screen.getByRole('link')
expect(link).not.toHaveAttribute('title')
})
it('does not set a tooltip when creditedAs equals name', () => {
const record = {
artist: 'Same Name',
participants: {
artist: [
{ id: 'canon-3', name: 'Same Name', creditedAs: 'Same Name' },
],
},
}
render(<ArtistLinkField record={record} source="artist" />)
const link = screen.getByRole('link')
expect(link).not.toHaveAttribute('title')
})
it('inline-linkifies when displayArtist holds the canonical name (Picard-default tagging)', () => {
// Picard "use standardized artist names" mode: ARTIST tag carries the
// canonical name, ARTIST_CREDIT carries the credit. The display string
// is the canonical name; parseAndReplaceArtists must match on `name`
// (not `creditedAs`) to embed the link inline. The link text itself
// still renders the credit via ALink.
const record = {
artist: 'Planetary Assault Systems',
participants: {
artist: [
{
id: 'canon-1',
name: 'Planetary Assault Systems',
creditedAs: 'PAS',
},
],
},
}
render(<ArtistLinkField record={record} source="artist" />)
const link = screen.getByRole('link')
// Link text is the credit, tooltip carries canonical
expect(link).toHaveTextContent('PAS')
expect(link).toHaveAttribute('title', 'Planetary Assault Systems')
// The original canonical string should not appear as raw plain text
// (it was replaced by the link)
expect(
screen.queryByText(/^Planetary Assault Systems$/),
).not.toBeInTheDocument()
})
})
describe('when limiting displayed artists', () => {
it('limits the number of artists displayed', () => {
const record = {