diff --git a/internal/library/mbids.go b/internal/library/mbids.go index fcaf78ad..608ed0a0 100644 --- a/internal/library/mbids.go +++ b/internal/library/mbids.go @@ -1,6 +1,8 @@ package library import ( + "strings" + "github.com/dhowden/tag" "github.com/dhowden/tag/mbz" ) @@ -13,11 +15,26 @@ import ( // a *tag.Comm value whose Description holds the Picard tag name. // - MP4 (M4A) freeform iTunes atoms are stored under their bare sub-name. // -// We previously hand-rolled this lookup against guessed keys -// ("MUSICBRAINZ_ALBUMID", "TXXX:MusicBrainz Album Id", ...), none of which -// dhowden actually produces — so every extraction returned ("",""), which -// fed an empty MBID into both the scanner and the boot-time backfill. +// Then sanitizes: dhowden's readTextWithDescrFrame (used for ID3v2 TXXX) +// does NOT strip the trailing/embedded NUL bytes that ID3v2.4 uses as +// frame terminator and multi-value separator — so the Text often comes +// out as "uuid\x00" or "uuid1\x00uuid2". Postgres rejects NULs in text +// columns (SQLSTATE 22021). We split on NUL and take the first non-empty +// segment — Minstrel uses the primary release artist for MBCAA lookup, +// so first-wins matches the original semantics. func extractMBIDs(m tag.Metadata) (albumMBID, artistMBID string) { info := mbz.Extract(m) - return info.Get(mbz.Album), info.Get(mbz.Artist) + return cleanMBID(info.Get(mbz.Album)), cleanMBID(info.Get(mbz.Artist)) +} + +func cleanMBID(s string) string { + if s == "" { + return "" + } + for _, part := range strings.Split(s, "\x00") { + if t := strings.TrimSpace(part); t != "" { + return t + } + } + return "" } diff --git a/internal/library/mbids_test.go b/internal/library/mbids_test.go index 376257be..33494bf7 100644 --- a/internal/library/mbids_test.go +++ b/internal/library/mbids_test.go @@ -109,3 +109,57 @@ func TestExtractMBIDs_VorbisUppercaseKeysMiss(t *testing.T) { t.Errorf("uppercase Vorbis keys must not match dhowden output; got (%q, %q)", a, ar) } } + +func TestExtractMBIDs_ID3v2StripsTrailingNUL(t *testing.T) { + // dhowden's readTextWithDescrFrame leaves the ID3v2.4 frame- + // terminator NUL on TXXX values. Postgres rejects NULs in text + // columns (SQLSTATE 22021), so we must strip them. + m := stubMeta{ + format: tag.ID3v2_4, + raw: map[string]interface{}{ + "TXXX": &tag.Comm{ + Description: "MusicBrainz Album Id", + Text: "abc-123\x00", + }, + "TXXX_0": &tag.Comm{ + Description: "MusicBrainz Artist Id", + Text: "def-456\x00", + }, + }, + } + a, ar := extractMBIDs(m) + if a != "abc-123" || ar != "def-456" { + t.Errorf("got (%q, %q), want trailing NUL stripped", a, ar) + } +} + +func TestExtractMBIDs_ID3v2MultiValueTakesFirst(t *testing.T) { + // Some Picard configs emit multi-value TXXX:MusicBrainz Artist Id + // for collaborations, NUL-separated. Minstrel uses the primary + // release artist for MBCAA, so first wins. + m := stubMeta{ + format: tag.ID3v2_4, + raw: map[string]interface{}{ + "TXXX": &tag.Comm{ + Description: "MusicBrainz Artist Id", + Text: "primary-mbid\x00collab-mbid", + }, + }, + } + _, ar := extractMBIDs(m) + if ar != "primary-mbid" { + t.Errorf("got %q, want %q", ar, "primary-mbid") + } +} + +func TestCleanMBID_OnlyNULReturnsEmpty(t *testing.T) { + // Defensive: a TXXX value that is purely NULs (or empty after + // trimming) must round-trip to empty so the caller skips the row + // instead of writing a NUL-only string to Postgres. + if got := cleanMBID("\x00\x00"); got != "" { + t.Errorf("got %q, want empty", got) + } + if got := cleanMBID(""); got != "" { + t.Errorf("got %q, want empty", got) + } +}