feat(library): store each album's MusicBrainz release-group id (M483 #5242)
release / govulncheck (push) Successful in 22s
release / web (push) Successful in 1m14s
release / go (push) Successful in 1m33s
release / integration (push) Successful in 4m36s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m24s
release / Build signed APK (releases and dev) (push) Canceled after 5m29s
release / govulncheck (push) Successful in 22s
release / web (push) Successful in 1m14s
release / go (push) Successful in 1m33s
release / integration (push) Successful in 4m36s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m24s
release / Build signed APK (releases and dev) (push) Canceled after 5m29s
albums.mbid is the release id (Picard's musicbrainz_albumid, one edition). Lidarr names albums by release group, so re-acquisition and request completion need that id too (#5241). Migration 0070 adds albums.release_group_mbid (nullable, non-unique index: several releases share a group). The scanner reads musicbrainz_releasegroupid through extractReleaseGroupMBID, writes it on insert and heals it onto existing rows when NULL. tagReadVersion goes to 3 so the next scan fills it for the library already indexed, bound by tag reads (no ffprobe, no decode). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -41,6 +41,16 @@ func extractRecordingMBID(m tag.Metadata) string {
|
||||
return cleanMBID(mbz.Extract(m).Get(mbz.Recording))
|
||||
}
|
||||
|
||||
// extractReleaseGroupMBID reads the MusicBrainz *release-group* ID —
|
||||
// Picard's musicbrainz_releasegroupid, surfaced by dhowden/tag as
|
||||
// mbz.ReleaseGroup. mbz.Album (what albums.mbid stores) is the release: one
|
||||
// edition. Lidarr names albums by release group, so re-acquisition and
|
||||
// request completion key on this one (#5241). Separate from extractMBIDs for
|
||||
// the same reason extractRecordingMBID is.
|
||||
func extractReleaseGroupMBID(m tag.Metadata) string {
|
||||
return cleanMBID(mbz.Extract(m).Get(mbz.ReleaseGroup))
|
||||
}
|
||||
|
||||
func cleanMBID(s string) string {
|
||||
if s == "" {
|
||||
return ""
|
||||
|
||||
@@ -163,3 +163,40 @@ func TestCleanMBID_OnlyNULReturnsEmpty(t *testing.T) {
|
||||
t.Errorf("got %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
// #5241: the release-group id is read beside the release id, from each
|
||||
// format's own key, and the two are never confused.
|
||||
func TestExtractReleaseGroupMBID(t *testing.T) {
|
||||
cases := []struct {
|
||||
name string
|
||||
meta stubMeta
|
||||
}{
|
||||
{"vorbis", stubMeta{format: tag.VORBIS, raw: map[string]interface{}{
|
||||
"musicbrainz_albumid": "release-1",
|
||||
"musicbrainz_releasegroupid": "group-1",
|
||||
}}},
|
||||
{"id3v2", stubMeta{format: tag.ID3v2_4, raw: map[string]interface{}{
|
||||
"TXXX": &tag.Comm{Description: "MusicBrainz Album Id", Text: "release-1"},
|
||||
"TXXX_0": &tag.Comm{Description: "MusicBrainz Release Group Id", Text: "group-1\x00"},
|
||||
}}},
|
||||
{"mp4", stubMeta{format: tag.MP4, raw: map[string]interface{}{
|
||||
"MusicBrainz Album Id": "release-1",
|
||||
"MusicBrainz Release Group Id": "group-1",
|
||||
}}},
|
||||
}
|
||||
for _, c := range cases {
|
||||
t.Run(c.name, func(t *testing.T) {
|
||||
if got := extractReleaseGroupMBID(c.meta); got != "group-1" {
|
||||
t.Errorf("release group = %q, want group-1", got)
|
||||
}
|
||||
if album, _ := extractMBIDs(c.meta); album != "release-1" {
|
||||
t.Errorf("album (release) = %q, want release-1", album)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
none := stubMeta{format: tag.VORBIS, raw: map[string]interface{}{"musicbrainz_albumid": "release-1"}}
|
||||
if got := extractReleaseGroupMBID(none); got != "" {
|
||||
t.Errorf("untagged release group = %q, want empty", got)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -56,7 +56,11 @@ var audioExtensions = map[string]bool{
|
||||
// "EDM", "Children'S Music" -> "Children's Music". Bumped rather than left
|
||||
// to new files only because taste_profile.sql reads tracks.genre directly,
|
||||
// so a half-repaired library would carry both spellings as separate tags.
|
||||
const tagReadVersion int16 = 2
|
||||
// 3: the MusicBrainz release-group id is read into albums.release_group_mbid
|
||||
// (#5241). Lidarr names albums by release group, not by the release id
|
||||
// albums.mbid holds, so re-acquisition and request completion need it for
|
||||
// the albums already in the library, not only for files added from now on.
|
||||
const tagReadVersion int16 = 3
|
||||
|
||||
type Stats struct {
|
||||
Scanned int `json:"scanned"`
|
||||
@@ -258,6 +262,7 @@ func (s *Scanner) scanFile(
|
||||
}
|
||||
albumMBID, artistMBID := extractMBIDs(meta)
|
||||
recordingMBID := extractRecordingMBID(meta)
|
||||
releaseGroupMBID := extractReleaseGroupMBID(meta)
|
||||
|
||||
artistName := meta.Artist()
|
||||
if artistName == "" {
|
||||
@@ -276,7 +281,7 @@ func (s *Scanner) scanFile(
|
||||
if err != nil {
|
||||
return pgtype.UUID{}, false, fmt.Errorf("artist: %w", err)
|
||||
}
|
||||
album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID)
|
||||
album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID, releaseGroupMBID)
|
||||
if err != nil {
|
||||
return pgtype.UUID{}, false, fmt.Errorf("album: %w", err)
|
||||
}
|
||||
@@ -518,9 +523,23 @@ func (s *Scanner) resolveArtist(ctx context.Context, q *dbq.Queries, name, mbid
|
||||
return artist, nil
|
||||
}
|
||||
|
||||
func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid string) (dbq.Album, error) {
|
||||
func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid, releaseGroupMBID string) (dbq.Album, error) {
|
||||
existing, err := q.GetAlbumByArtistAndTitle(ctx, dbq.GetAlbumByArtistAndTitleParams{ArtistID: artistID, Title: title})
|
||||
if err == nil {
|
||||
// Heal the release group the same way. Not unique, so no conflict to
|
||||
// handle: every release of a group may carry it.
|
||||
if releaseGroupMBID != "" && existing.ReleaseGroupMbid == nil {
|
||||
rg := releaseGroupMBID
|
||||
if uerr := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{
|
||||
ID: existing.ID,
|
||||
ReleaseGroupMbid: &rg,
|
||||
}); uerr != nil {
|
||||
s.logger.Warn("library scan: heal album release group failed",
|
||||
"album_id", existing.ID, "err", uerr)
|
||||
} else {
|
||||
existing.ReleaseGroupMbid = &rg
|
||||
}
|
||||
}
|
||||
// Heal: backfill mbid on a previously-imported row if we have one now.
|
||||
if mbid != "" && (existing.Mbid == nil || *existing.Mbid == "") {
|
||||
m := mbid
|
||||
@@ -564,6 +583,10 @@ func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgt
|
||||
m := mbid
|
||||
params.Mbid = &m
|
||||
}
|
||||
if releaseGroupMBID != "" {
|
||||
rg := releaseGroupMBID
|
||||
params.ReleaseGroupMbid = &rg
|
||||
}
|
||||
album, err := q.UpsertAlbum(ctx, params)
|
||||
if err != nil {
|
||||
return dbq.Album{}, err
|
||||
|
||||
@@ -308,3 +308,80 @@ func TestScanner_AdoptsMovedFile_Integration(t *testing.T) {
|
||||
t.Errorf("tracks = %d, want 8 — a rename must not add a row", total)
|
||||
}
|
||||
}
|
||||
|
||||
// TestScanner_ReleaseGroupMBID_Integration is #5241's scan half: a new album
|
||||
// takes its release-group id from the tag, and an album indexed before this
|
||||
// read existed gets it on the next scan, through the tagReadVersion bump,
|
||||
// without its file changing.
|
||||
func TestScanner_ReleaseGroupMBID_Integration(t *testing.T) {
|
||||
if testing.Short() {
|
||||
t.Skip("skipping scanner integration in -short mode")
|
||||
}
|
||||
dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL")
|
||||
if dsn == "" {
|
||||
t.Skip("MINSTREL_TEST_DATABASE_URL not set")
|
||||
}
|
||||
ctx := context.Background()
|
||||
logger := slog.New(slog.NewTextHandler(io.Discard, nil))
|
||||
|
||||
if err := db.Migrate(dsn, logger); err != nil {
|
||||
t.Fatalf("migrate: %v", err)
|
||||
}
|
||||
pool, err := pgxpool.New(ctx, dsn)
|
||||
if err != nil {
|
||||
t.Fatalf("pool: %v", err)
|
||||
}
|
||||
t.Cleanup(pool.Close)
|
||||
if _, err := pool.Exec(ctx, "TRUNCATE tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil {
|
||||
t.Fatalf("truncate: %v", err)
|
||||
}
|
||||
|
||||
const (
|
||||
groupA = "aaaaaaaa-1111-2222-3333-444444444444"
|
||||
groupB = "bbbbbbbb-1111-2222-3333-444444444444"
|
||||
)
|
||||
root := t.TempDir()
|
||||
writeTestMP3(t, filepath.Join(root, "rg/A/01.mp3"), map[string]string{
|
||||
"TIT2": "One", "TPE1": "Artist RG", "TALB": "Album A",
|
||||
"TXXX": "MusicBrainz Release Group Id\x00" + groupA,
|
||||
})
|
||||
pathB := filepath.Join(root, "rg/B/01.mp3")
|
||||
writeTestMP3(t, pathB, map[string]string{
|
||||
"TIT2": "Two", "TPE1": "Artist RG", "TALB": "Album B",
|
||||
"TXXX": "MusicBrainz Release Group Id\x00" + groupB,
|
||||
})
|
||||
|
||||
scanner := New(pool, logger, []string{root}, nil)
|
||||
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||
t.Fatalf("first scan: %v", err)
|
||||
}
|
||||
groupOf := func(title string) *string {
|
||||
t.Helper()
|
||||
var rg *string
|
||||
if err := pool.QueryRow(ctx, "SELECT release_group_mbid FROM albums WHERE title = $1", title).Scan(&rg); err != nil {
|
||||
t.Fatalf("album %q: %v", title, err)
|
||||
}
|
||||
return rg
|
||||
}
|
||||
if rg := groupOf("Album A"); rg == nil || *rg != groupA {
|
||||
t.Fatalf("new album release group = %v, want %s", rg, groupA)
|
||||
}
|
||||
|
||||
// Album B as a library indexed under tag-read version 2 left it: no
|
||||
// release group, file untouched since.
|
||||
if _, err := pool.Exec(ctx, "UPDATE albums SET release_group_mbid = NULL WHERE title = 'Album B'"); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, "UPDATE tracks SET tag_read_version = 2 WHERE file_path = $1", pathB); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||
t.Fatalf("second scan: %v", err)
|
||||
}
|
||||
if rg := groupOf("Album B"); rg == nil || *rg != groupB {
|
||||
t.Errorf("existing album release group after rescan = %v, want %s", rg, groupB)
|
||||
}
|
||||
if rg := groupOf("Album A"); rg == nil || *rg != groupA {
|
||||
t.Errorf("album A release group changed to %v", rg)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user