Compare commits

...

26 commits

Author SHA1 Message Date
Deluan
3ce7fc32bd docs(scanner): tighten map_participants comments
Drop unverifiable claims and shrink verbose explanations to one or
two lines, matching the file's existing comment style.
2026-05-24 23:37:32 -03:00
Deluan
a4116e01c2 docs(scanner): correct unsubstantiated comment about UnknownArtist taggers
The previous comment claimed 'some rippers emit the literal [Unknown
Artist] string' — no such tagger is actually known. The defensive
second clause in the missing-albumartist check was preserving
accidental behavior from the prior parseArtists implementation, not
guarding against a documented real-world case. Rewrite the comment
to be honest about provenance: keep the clause for behavioral parity
and the cheap round-trip defense, but stop asserting facts I can't
back up.
2026-05-24 23:35:11 -03:00
Deluan
4e7c2128f3 docs(scanner): document artistIDMap many-to-one collapse behavior
Per Copilot review on PR #5527: the 'first prevID wins' check looked
like a bug. It is intentional but the rationale wasn't obvious from
context. Annotate the code with the merge-collision relationship to
ReassignAnnotation's missing merge semantics — the follow-up that
fixes the SQL side will benefit both album and artist re-PID paths.
2026-05-24 22:24:50 -03:00
Deluan
c413d6510e fix(metadata): drop unused performer/plural-credit tag mappings
Per Copilot & Codex review on PR #5527:

- performercredit was registered as a plain string mapping, but PERFORMER
  is type:pair ('PERFORMER:instrument'). The credit form needs the same
  pair structure to be meaningful, and processPerformers doesn't read it
  either way. Better to remove the misleading mapping than ship dead
  config that suggests support we don't have.

- TagComposersCredit and TagLyricistsCredit (plural variants) were never
  consumed by any code path — only the singular TagComposerCredit and
  TagLyricistCredit are wired into roleMappings.

Both can be added back when the underlying feature is actually
implemented.
2026-05-24 22:24:06 -03:00
Deluan
2ff6d91057 fix(participants): add separator to dedup key to avoid prefix collisions
Per Gemini review on PR #5527: the dedup key was 'ID+SubRole' with no
separator, so (ID='12', SubRole='3') and (ID='1', SubRole='23') would
collide. Real-world risk is negligible (artist IDs are fixed-length
MD5 hashes), but the fix is trivial — use a NUL byte that cannot
appear in either field.
2026-05-24 22:21:53 -03:00
Deluan
627d6161cc fix(scanner): split parallel tag lists with same separators as names
Per Gemini, Copilot, and Codex review on PR #5527: credits, sorts, and
MBIDs were read via md.Strings() (raw), while names use getRoleValues /
getArtistValues which apply the role/artist split separators. A tag
combination like COMPOSER='A;B' + COMPOSER_CREDIT='AA;BB' produced 2
names but 1 credit, silently dropping all credits via the length-
mismatch fallback. Same hazard for MBIDs.

Route MBIDs/sorts/credits through the same splitter as names so
positional alignment holds for libraries using ';' or '/' delimiters.
2026-05-24 22:21:24 -03:00
Deluan
abd9d6bdba test(participants): use neutral names to avoid gosec false positive
The G101 'potential hardcoded credentials' lint flagged a struct field
named CreditedAs paired with a string literal. The artist names in the
dedup-merge test are arbitrary — rename to plain placeholders.
2026-05-24 21:59:20 -03:00
Deluan
87cd3f1037 fix(scanner): treat explicit '[Unknown Artist]' ALBUMARTIST as missing
The old parseArtists function (removed in this branch) substituted
consts.UnknownArtist when albumartist tags were empty, so the
downstream compilation/fallback check matched both cases. The
inlined replacement only checked len(albumNames)==0, narrowing the
condition: files explicitly tagged ALBUMARTIST='[Unknown Artist]'
(emitted by some rippers, and matching what Navidrome itself stores
for untagged files) stopped routing to Various Artists for
compilations.

Restore the original semantics by treating len==1 with the literal
UnknownArtist string as equivalent to no tag.
2026-05-24 21:37:01 -03:00
Deluan
363b93ef25 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.
2026-05-24 21:34:47 -03:00
Deluan
c898f0e2a9 fix(participants): preserve later non-empty CreditedAs on dedup merge
Participants.add deduplicates by ID+SubRole. The same artist appearing
on multiple tracks of one album would silently keep only the
first-seen CreditedAs, so the album-level participants JSON could
end up with a credit from one track applied to all tracks of that
album.

Change the dedup to update the existing entry's CreditedAs when a
later occurrence has a non-empty value. Per-track media_file
participants are unaffected (each track's JSON is built
independently). The album-level merge result is inherently lossy
when tracks differ, but no longer silently drops a meaningful credit
based on arrival order.
2026-05-24 21:29:47 -03:00
Deluan
1cf5a9f215 fix(ui): match canonical name in parseAndReplaceArtists
The display string (record.artist/albumArtist) is sourced from
mf.Artist/mf.AlbumArtist, which carry the canonical-tag value.
Matching on creditedAs failed for Picard-default tagging (ARTIST
tag = canonical name, ARTIST_CREDIT = credit) because indexOf
could not find the credit inside the canonical display string,
silently degrading from inline linkification to bullet-list
fallback.

Match on artist.name (always equal to displayString slice) and
let ALink continue rendering creditedAs || name for visible text.
2026-05-24 21:28:05 -03:00
Deluan
86cb5fee93 fix(scanner): backfill PIDArtist property on upgrade
A user upgrading from a pre-PID.Artist version with a non-default
PID.Artist already configured would have hit a silent annotation
loss: empty stored PIDArtistKey was normalized to the current
config, pidHashChanged returned false, no full rescan was forced,
and prevArtistPIDConf stayed empty during the next normal scan so
the artistIDMap guard skipped annotation migration entirely.

Backfill via migration so the stored value reflects the historical
default ('name', byte-identical to the legacy hardcoded artistID).
Drop the empty-string fallback in pidHashChanged — with the
migration in place, an empty stored value would be anomalous, and
the fallback was the exact dead branch that hid this hole.
2026-05-24 21:13:17 -03:00
Deluan
7d601029c9 refactor(participants): use DisplayName in JoinCredited 2026-05-24 20:09:04 -03:00
Deluan
57785255bf fix(persistence): preserve CreditedAs through participants JSON round-trip 2026-05-24 20:05:36 -03:00
Deluan
03b8b15f6e feat(participants): add DisplayName method to prioritize CreditedAs over Name
Signed-off-by: Deluan <deluan@navidrome.org>
2026-05-24 20:01:45 -03:00
Deluan
a859804fd4 feat(ui): render creditedAs as link text with canonical tooltip 2026-05-24 19:50:25 -03:00
Deluan
e2fd0959bd feat(subsonic): serve CreditedAs as name in contributor lists 2026-05-24 19:47:59 -03:00
Deluan
8b5e50a1aa 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.
2026-05-24 19:42:04 -03:00
Deluan
46cb9f8d58 feat(scanner): trigger rescan when PID.Artist changes 2026-05-24 19:35:03 -03:00
Deluan
12980e8d88 feat(scanner): populate CreditedAs from paired *_credit tags 2026-05-24 19:30:53 -03:00
Deluan
d374150a5a feat(metadata): register *_credit tag aliases in mappings.yaml 2026-05-24 19:30:53 -03:00
Deluan
c2712e91fc feat(model): add TagXxxCredit constants for each role 2026-05-24 19:16:02 -03:00
Deluan
437d60a0df refactor(scanner): wire buildArtists through computeArtistPID 2026-05-24 19:13:14 -03:00
Deluan
398efa04a3 feat(model): add CreditedAs to Participant 2026-05-24 19:09:36 -03:00
Deluan
3358a8bc91 feat(metadata): add computeArtistPID with name-attr normalization 2026-05-24 19:07:25 -03:00
Deluan
273c8d23f1 feat(conf): add PID.Artist config option with default 'name' 2026-05-24 18:57:30 -03:00
25 changed files with 1007 additions and 119 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

@ -242,8 +242,9 @@ type backupOptions struct {
}
type pidOptions struct {
Track string
Album string
Track string
Album string
Artist string
}
type inspectOptions struct {
@ -439,6 +440,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)
@ -851,6 +853,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

@ -122,10 +122,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

@ -201,27 +201,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

@ -271,7 +271,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
@ -282,7 +282,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(),
},
})
}
@ -296,7 +296,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

@ -317,6 +317,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 = {