Files
bvandeusen 4481874482 fix(server,web/m7-382): handle duplicate-MBID conflicts during backfill
Forward-fix from running the prior two commits in production. Backfill
on a real library exposed two further issues:

(1) gofmt -s flagged the column-aligned stubMeta methods in
    mbids_test.go. Switched to the gofmt-canonical single-space form.

(2) Many albums failed to heal with SQLSTATE 23505 (unique constraint
    albums_mbid_unique). Cause: the operator's library has duplicate
    album rows in the DB — same MusicBrainz release, split into
    multiple rows by the pre-MBID scanner because of subtle title or
    artist disagreements between files. When backfill now correctly
    extracts the MBID, two rows want to claim the same one and the
    partial unique index rejects the second.

    The conflict is correct DB behavior — we shouldn't have two rows
    with the same MBID — but it's not a write failure to alarm the
    operator about. Detect 23505 specifically:
      - downgrade the log line from Warn to Info
      - track these in a new BackfillMBIDsResult.Duplicates counter
        (separate from Skipped, which retains its "no MBID in tag"
        meaning)
      - leave the duplicate row's mbid NULL; merging duplicates is a
        separate (future) operator workflow

    Same handling threaded through the scanner heal path so a regular
    rescan doesn't generate the noise either.

    JSON tally + admin UI gain a "Duplicates" line so the operator
    can see how many duplicate rows their library carries.

Follow-up scope (separate task): a duplicate-album merge UX that
reparents the duplicate's tracks to the canonical row and deletes
the orphaned album row.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-04 21:45:15 -04:00

166 lines
5.1 KiB
Go

package library
import (
"testing"
"github.com/dhowden/tag"
)
// stubMeta is a minimal tag.Metadata used to feed the dhowden mbz extractor
// the same Raw()/Format() shape its real parsers produce. The other interface
// methods return zero values — the extractor only branches on Format and
// iterates Raw().
type stubMeta struct {
format tag.Format
raw map[string]interface{}
}
func (s stubMeta) Format() tag.Format { return s.format }
func (s stubMeta) FileType() tag.FileType { return tag.UnknownFileType }
func (s stubMeta) Title() string { return "" }
func (s stubMeta) Album() string { return "" }
func (s stubMeta) Artist() string { return "" }
func (s stubMeta) AlbumArtist() string { return "" }
func (s stubMeta) Composer() string { return "" }
func (s stubMeta) Year() int { return 0 }
func (s stubMeta) Genre() string { return "" }
func (s stubMeta) Track() (int, int) { return 0, 0 }
func (s stubMeta) Disc() (int, int) { return 0, 0 }
func (s stubMeta) Picture() *tag.Picture { return nil }
func (s stubMeta) Lyrics() string { return "" }
func (s stubMeta) Comment() string { return "" }
func (s stubMeta) Raw() map[string]interface{} { return s.raw }
func TestExtractMBIDs_Vorbis(t *testing.T) {
// dhowden lowercases all Vorbis comment keys at parse time.
m := stubMeta{
format: tag.VORBIS,
raw: map[string]interface{}{
"musicbrainz_albumid": "abc",
"musicbrainz_artistid": "def",
},
}
a, ar := extractMBIDs(m)
if a != "abc" || ar != "def" {
t.Errorf("got (%q, %q), want (\"abc\", \"def\")", a, ar)
}
}
func TestExtractMBIDs_ID3v2(t *testing.T) {
// dhowden stores TXXX frames under bare "TXXX"/"TXXX_N" keys with
// *tag.Comm values whose Description holds the Picard tag name.
m := stubMeta{
format: tag.ID3v2_4,
raw: map[string]interface{}{
"TXXX": &tag.Comm{
Description: "MusicBrainz Album Id",
Text: "abc-123",
},
"TXXX_0": &tag.Comm{
Description: "MusicBrainz Artist Id",
Text: "def-456",
},
},
}
a, ar := extractMBIDs(m)
if a != "abc-123" || ar != "def-456" {
t.Errorf("got (%q, %q), want (\"abc-123\", \"def-456\")", a, ar)
}
}
func TestExtractMBIDs_MP4(t *testing.T) {
// dhowden stores freeform iTunes atoms under their bare sub-name.
// mbz.Extract maps the human-readable form back to its lowercase key.
m := stubMeta{
format: tag.MP4,
raw: map[string]interface{}{
"MusicBrainz Album Id": "abc",
"MusicBrainz Artist Id": "def",
},
}
a, ar := extractMBIDs(m)
if a != "abc" || ar != "def" {
t.Errorf("got (%q, %q), want (\"abc\", \"def\")", a, ar)
}
}
func TestExtractMBIDs_MissingTagsReturnEmpty(t *testing.T) {
m := stubMeta{format: tag.VORBIS, raw: map[string]interface{}{}}
a, ar := extractMBIDs(m)
if a != "" || ar != "" {
t.Errorf("got (%q, %q), want empty", a, ar)
}
}
func TestExtractMBIDs_VorbisUppercaseKeysMiss(t *testing.T) {
// Regression guard: real dhowden never produces uppercase Vorbis
// keys. If a future change starts looking up "MUSICBRAINZ_ALBUMID"
// directly, this test must keep returning empty so we don't
// silently re-introduce the v0 bug where every album skipped.
m := stubMeta{
format: tag.VORBIS,
raw: map[string]interface{}{
"MUSICBRAINZ_ALBUMID": "wrong-case",
"MUSICBRAINZ_ARTISTID": "wrong-case",
},
}
a, ar := extractMBIDs(m)
if a != "" || ar != "" {
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)
}
}