From 5c19a916bad03661d8a7b98f7f1a2b9df9a855cb Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 11:33:26 -0400 Subject: [PATCH 1/3] feat(library): store each album's MusicBrainz release-group id (M483 #5242) 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 --- internal/db/dbq/albums.sql.go | 84 ++++++++++++++----- internal/db/dbq/browse.sql.go | 6 +- internal/db/dbq/likes.sql.go | 3 +- internal/db/dbq/models.go | 1 + internal/db/dbq/recommendation.sql.go | 6 +- internal/db/dbq/you_might_like.sql.go | 6 +- .../0070_album_release_group_mbid.down.sql | 2 + .../0070_album_release_group_mbid.up.sql | 10 +++ internal/db/queries/albums.sql | 14 +++- internal/library/mbids.go | 10 +++ internal/library/mbids_test.go | 37 ++++++++ internal/library/scanner.go | 29 ++++++- internal/library/scanner_test.go | 77 +++++++++++++++++ 13 files changed, 250 insertions(+), 35 deletions(-) create mode 100644 internal/db/migrations/0070_album_release_group_mbid.down.sql create mode 100644 internal/db/migrations/0070_album_release_group_mbid.up.sql diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index 04ce65e6..ff5aa098 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -72,7 +72,7 @@ func (q *Queries) DeleteAlbumIfEmpty(ctx context.Context, id pgtype.UUID) (Delet } const getAlbumByArtistAndTitle = `-- name: GetAlbumByArtistAndTitle :one -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE artist_id = $1 AND title = $2 LIMIT 1 +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums WHERE artist_id = $1 AND title = $2 LIMIT 1 ` type GetAlbumByArtistAndTitleParams struct { @@ -96,12 +96,13 @@ func (q *Queries) GetAlbumByArtistAndTitle(ctx context.Context, arg GetAlbumByAr &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ) return i, err } const getAlbumByID = `-- name: GetAlbumByID :one -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE id = $1 +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums WHERE id = $1 ` func (q *Queries) GetAlbumByID(ctx context.Context, id pgtype.UUID) (Album, error) { @@ -119,6 +120,7 @@ func (q *Queries) GetAlbumByID(ctx context.Context, id pgtype.UUID) (Album, erro &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ) return i, err } @@ -172,7 +174,7 @@ func (q *Queries) GetAlbumCoverageRollup(ctx context.Context) (GetAlbumCoverageR } const getAlbumWithArtist = `-- name: GetAlbumWithArtist :one -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM albums JOIN artists ON artists.id = albums.artist_id WHERE albums.id = $1 @@ -201,13 +203,14 @@ func (q *Queries) GetAlbumWithArtist(ctx context.Context, id pgtype.UUID) (GetAl &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ) return i, err } const getAlbumsByIDs = `-- name: GetAlbumsByIDs :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE id = ANY($1::uuid[]) +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums WHERE id = ANY($1::uuid[]) ` // Batched lookup used by /api/library/sync to hydrate upsert payloads @@ -233,6 +236,7 @@ func (q *Queries) GetAlbumsByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -245,7 +249,7 @@ func (q *Queries) GetAlbumsByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ } const listAlbumsAlphaByArtist = `-- name: ListAlbumsAlphaByArtist :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.sort_name AS artist_sort_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.sort_name AS artist_sort_name FROM albums JOIN artists ON artists.id = albums.artist_id ORDER BY artists.sort_name, albums.sort_title @@ -285,6 +289,7 @@ func (q *Queries) ListAlbumsAlphaByArtist(ctx context.Context, arg ListAlbumsAlp &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistSortName, ); err != nil { return nil, err @@ -298,7 +303,7 @@ func (q *Queries) ListAlbumsAlphaByArtist(ctx context.Context, arg ListAlbumsAlp } const listAlbumsAlphaByName = `-- name: ListAlbumsAlphaByName :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums ORDER BY sort_title LIMIT $1 OFFSET $2 +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums ORDER BY sort_title LIMIT $1 OFFSET $2 ` type ListAlbumsAlphaByNameParams struct { @@ -327,6 +332,7 @@ func (q *Queries) ListAlbumsAlphaByName(ctx context.Context, arg ListAlbumsAlpha &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -339,7 +345,7 @@ func (q *Queries) ListAlbumsAlphaByName(ctx context.Context, arg ListAlbumsAlpha } const listAlbumsAlphaWithArtist = `-- name: ListAlbumsAlphaWithArtist :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM albums JOIN artists ON artists.id = albums.artist_id ORDER BY albums.sort_title, albums.id @@ -379,6 +385,7 @@ func (q *Queries) ListAlbumsAlphaWithArtist(ctx context.Context, arg ListAlbumsA &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err @@ -392,7 +399,7 @@ func (q *Queries) ListAlbumsAlphaWithArtist(ctx context.Context, arg ListAlbumsA } const listAlbumsByArtist = `-- name: ListAlbumsByArtist :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums WHERE artist_id = $1 ORDER BY release_date NULLS LAST, sort_title +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums WHERE artist_id = $1 ORDER BY release_date NULLS LAST, sort_title ` func (q *Queries) ListAlbumsByArtist(ctx context.Context, artistID pgtype.UUID) ([]Album, error) { @@ -416,6 +423,7 @@ func (q *Queries) ListAlbumsByArtist(ctx context.Context, artistID pgtype.UUID) &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -428,7 +436,7 @@ func (q *Queries) ListAlbumsByArtist(ctx context.Context, artistID pgtype.UUID) } const listAlbumsByArtistWithTrackCount = `-- name: ListAlbumsByArtistWithTrackCount :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, (SELECT count(*) FROM tracks t WHERE t.album_id = albums.id)::bigint AS track_count FROM albums @@ -465,6 +473,7 @@ func (q *Queries) ListAlbumsByArtistWithTrackCount(ctx context.Context, artistID &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.TrackCount, ); err != nil { return nil, err @@ -478,7 +487,7 @@ func (q *Queries) ListAlbumsByArtistWithTrackCount(ctx context.Context, artistID } const listAlbumsByGenre = `-- name: ListAlbumsByGenre :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid FROM albums WHERE EXISTS ( SELECT 1 @@ -531,6 +540,7 @@ func (q *Queries) ListAlbumsByGenre(ctx context.Context, arg ListAlbumsByGenrePa &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -599,7 +609,7 @@ func (q *Queries) ListAlbumsMissingMbidWithTrack(ctx context.Context, limit int3 } const listAlbumsNewest = `-- name: ListAlbumsNewest :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums ORDER BY created_at DESC LIMIT $1 OFFSET $2 +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums ORDER BY created_at DESC LIMIT $1 OFFSET $2 ` type ListAlbumsNewestParams struct { @@ -628,6 +638,7 @@ func (q *Queries) ListAlbumsNewest(ctx context.Context, arg ListAlbumsNewestPara &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -640,7 +651,7 @@ func (q *Queries) ListAlbumsNewest(ctx context.Context, arg ListAlbumsNewestPara } const listAlbumsRandom = `-- name: ListAlbumsRandom :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums ORDER BY random() LIMIT $1 +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums ORDER BY random() LIMIT $1 ` func (q *Queries) ListAlbumsRandom(ctx context.Context, limit int32) ([]Album, error) { @@ -664,6 +675,7 @@ func (q *Queries) ListAlbumsRandom(ctx context.Context, limit int32) ([]Album, e &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -676,7 +688,7 @@ func (q *Queries) ListAlbumsRandom(ctx context.Context, limit int32) ([]Album, e } const listRecentlyAddedAlbumsWithArtist = `-- name: ListRecentlyAddedAlbumsWithArtist :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM albums JOIN artists ON artists.id = albums.artist_id ORDER BY albums.created_at DESC, albums.id @@ -713,6 +725,7 @@ func (q *Queries) ListRecentlyAddedAlbumsWithArtist(ctx context.Context, limit i &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err @@ -726,7 +739,7 @@ func (q *Queries) ListRecentlyAddedAlbumsWithArtist(ctx context.Context, limit i } const searchAlbums = `-- name: SearchAlbums :many -SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version FROM albums +SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums WHERE title ILIKE '%' || $1 || '%' ORDER BY sort_title LIMIT $2 OFFSET $3 @@ -759,6 +772,7 @@ func (q *Queries) SearchAlbums(ctx context.Context, arg SearchAlbumsParams) ([]A &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } @@ -818,9 +832,29 @@ func (q *Queries) SetAlbumMbidIfNull(ctx context.Context, arg SetAlbumMbidIfNull return err } +const setAlbumReleaseGroupMbidIfNull = `-- name: SetAlbumReleaseGroupMbidIfNull :exec +UPDATE albums + SET release_group_mbid = $2, + updated_at = now() + WHERE id = $1 AND release_group_mbid IS NULL +` + +type SetAlbumReleaseGroupMbidIfNullParams struct { + ID pgtype.UUID + ReleaseGroupMbid *string +} + +// #5241: heal the release-group id on an existing album row, from a rescan's +// tags or from the re-acquisition sweeper's MusicBrainz lookup. Only fills a +// NULL, so a tag never gets overwritten by a later guess or vice versa. +func (q *Queries) SetAlbumReleaseGroupMbidIfNull(ctx context.Context, arg SetAlbumReleaseGroupMbidIfNullParams) error { + _, err := q.db.Exec(ctx, setAlbumReleaseGroupMbidIfNull, arg.ID, arg.ReleaseGroupMbid) + return err +} + const upsertAlbum = `-- name: UpsertAlbum :one -INSERT INTO albums (title, sort_title, artist_id, release_date, mbid, cover_art_path) -VALUES ($1, $2, $3, $4, $5, $6) +INSERT INTO albums (title, sort_title, artist_id, release_date, mbid, cover_art_path, release_group_mbid) +VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (mbid) WHERE mbid IS NOT NULL DO UPDATE SET title = EXCLUDED.title, @@ -828,17 +862,19 @@ DO UPDATE SET artist_id = EXCLUDED.artist_id, release_date = EXCLUDED.release_date, cover_art_path = EXCLUDED.cover_art_path, + release_group_mbid = COALESCE(EXCLUDED.release_group_mbid, albums.release_group_mbid), updated_at = now() -RETURNING id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version +RETURNING id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid ` type UpsertAlbumParams struct { - Title string - SortTitle string - ArtistID pgtype.UUID - ReleaseDate pgtype.Date - Mbid *string - CoverArtPath *string + Title string + SortTitle string + ArtistID pgtype.UUID + ReleaseDate pgtype.Date + Mbid *string + CoverArtPath *string + ReleaseGroupMbid *string } func (q *Queries) UpsertAlbum(ctx context.Context, arg UpsertAlbumParams) (Album, error) { @@ -849,6 +885,7 @@ func (q *Queries) UpsertAlbum(ctx context.Context, arg UpsertAlbumParams) (Album arg.ReleaseDate, arg.Mbid, arg.CoverArtPath, + arg.ReleaseGroupMbid, ) var i Album err := row.Scan( @@ -863,6 +900,7 @@ func (q *Queries) UpsertAlbum(ctx context.Context, arg UpsertAlbumParams) (Album &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ) return i, err } diff --git a/internal/db/dbq/browse.sql.go b/internal/db/dbq/browse.sql.go index 3b22be31..0477431f 100644 --- a/internal/db/dbq/browse.sql.go +++ b/internal/db/dbq/browse.sql.go @@ -100,7 +100,7 @@ func (q *Queries) ListAlbumYearsWithCount(ctx context.Context) ([]ListAlbumYears } const listAlbumsByGenreWithArtist = `-- name: ListAlbumsByGenreWithArtist :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM albums JOIN artists ON artists.id = albums.artist_id WHERE EXISTS ( @@ -152,6 +152,7 @@ func (q *Queries) ListAlbumsByGenreWithArtist(ctx context.Context, arg ListAlbum &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err @@ -165,7 +166,7 @@ func (q *Queries) ListAlbumsByGenreWithArtist(ctx context.Context, arg ListAlbum } const listAlbumsByYearRangeWithArtist = `-- name: ListAlbumsByYearRangeWithArtist :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM albums JOIN artists ON artists.id = albums.artist_id WHERE albums.release_date IS NOT NULL @@ -219,6 +220,7 @@ func (q *Queries) ListAlbumsByYearRangeWithArtist(ctx context.Context, arg ListA &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err diff --git a/internal/db/dbq/likes.sql.go b/internal/db/dbq/likes.sql.go index 047d9c50..b85e1d49 100644 --- a/internal/db/dbq/likes.sql.go +++ b/internal/db/dbq/likes.sql.go @@ -120,7 +120,7 @@ func (q *Queries) ListLikedAlbumIDs(ctx context.Context, userID pgtype.UUID) ([] } const listLikedAlbumRows = `-- name: ListLikedAlbumRows :many -SELECT a.id, a.title, a.sort_title, a.artist_id, a.release_date, a.mbid, a.cover_art_path, a.created_at, a.updated_at, a.cover_art_source, a.cover_art_sources_version FROM albums a +SELECT a.id, a.title, a.sort_title, a.artist_id, a.release_date, a.mbid, a.cover_art_path, a.created_at, a.updated_at, a.cover_art_source, a.cover_art_sources_version, a.release_group_mbid FROM albums a JOIN general_likes_albums l ON l.album_id = a.id WHERE l.user_id = $1 ORDER BY l.liked_at DESC @@ -154,6 +154,7 @@ func (q *Queries) ListLikedAlbumRows(ctx context.Context, arg ListLikedAlbumRows &i.UpdatedAt, &i.CoverArtSource, &i.CoverArtSourcesVersion, + &i.ReleaseGroupMbid, ); err != nil { return nil, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 4034fb2c..c14ff1a3 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -207,6 +207,7 @@ type Album struct { UpdatedAt pgtype.Timestamptz CoverArtSource *string CoverArtSourcesVersion int32 + ReleaseGroupMbid *string } type AlbumLoudness struct { diff --git a/internal/db/dbq/recommendation.sql.go b/internal/db/dbq/recommendation.sql.go index a1e9a59e..9ddd88b2 100644 --- a/internal/db/dbq/recommendation.sql.go +++ b/internal/db/dbq/recommendation.sql.go @@ -370,7 +370,7 @@ func (q *Queries) ListMostPlayedTracksForUser(ctx context.Context, arg ListMostP } const listRediscoverAlbumsFallbackForUser = `-- name: ListRediscoverAlbumsFallbackForUser :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM general_likes_albums gla JOIN albums ON albums.id = gla.album_id JOIN artists ON artists.id = albums.artist_id @@ -418,6 +418,7 @@ func (q *Queries) ListRediscoverAlbumsFallbackForUser(ctx context.Context, arg L &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err @@ -459,7 +460,7 @@ eligible AS ( HAVING COALESCE(max(pe.started_at), '1970-01-01'::timestamptz) < now() - interval '14 days' ) -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM eligible e JOIN albums ON albums.id = e.album_id JOIN artists ON artists.id = albums.artist_id @@ -510,6 +511,7 @@ func (q *Queries) ListRediscoverAlbumsForUser(ctx context.Context, arg ListRedis &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err diff --git a/internal/db/dbq/you_might_like.sql.go b/internal/db/dbq/you_might_like.sql.go index ed4027f1..066e01f6 100644 --- a/internal/db/dbq/you_might_like.sql.go +++ b/internal/db/dbq/you_might_like.sql.go @@ -102,7 +102,7 @@ WITH liked_album_ids AS ( JOIN tracks trk ON trk.id = likt.track_id WHERE likt.user_id = $1 ) -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM liked_album_ids lai JOIN albums ON albums.id = lai.album_id JOIN artists ON artists.id = albums.artist_id @@ -143,6 +143,7 @@ func (q *Queries) ListYouMightLikeAlbumFallbackForUser(ctx context.Context, arg &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err @@ -156,7 +157,7 @@ func (q *Queries) ListYouMightLikeAlbumFallbackForUser(ctx context.Context, arg } const listYouMightLikeAlbumsForUser = `-- name: ListYouMightLikeAlbumsForUser :many -SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, artists.name AS artist_name +SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.name AS artist_name FROM you_might_like_albums yml JOIN albums ON albums.id = yml.album_id JOIN artists ON artists.id = albums.artist_id @@ -201,6 +202,7 @@ func (q *Queries) ListYouMightLikeAlbumsForUser(ctx context.Context, arg ListYou &i.Album.UpdatedAt, &i.Album.CoverArtSource, &i.Album.CoverArtSourcesVersion, + &i.Album.ReleaseGroupMbid, &i.ArtistName, ); err != nil { return nil, err diff --git a/internal/db/migrations/0070_album_release_group_mbid.down.sql b/internal/db/migrations/0070_album_release_group_mbid.down.sql new file mode 100644 index 00000000..1deb2957 --- /dev/null +++ b/internal/db/migrations/0070_album_release_group_mbid.down.sql @@ -0,0 +1,2 @@ +DROP INDEX IF EXISTS albums_release_group_mbid_idx; +ALTER TABLE albums DROP COLUMN IF EXISTS release_group_mbid; diff --git a/internal/db/migrations/0070_album_release_group_mbid.up.sql b/internal/db/migrations/0070_album_release_group_mbid.up.sql new file mode 100644 index 00000000..08a85f2e --- /dev/null +++ b/internal/db/migrations/0070_album_release_group_mbid.up.sql @@ -0,0 +1,10 @@ +-- MusicBrainz release-group id for an album (#5241). albums.mbid holds the +-- *release* id (Picard's musicbrainz_albumid: one edition), but Lidarr names +-- albums by release group, so re-acquisition and request completion need +-- this one. Not unique: several releases of one group can each be an album +-- row. Filled by the scanner from musicbrainz_releasegroupid, or by the +-- re-acquisition sweeper resolving the release through MusicBrainz. +ALTER TABLE albums ADD COLUMN release_group_mbid text; + +CREATE INDEX albums_release_group_mbid_idx ON albums (release_group_mbid) + WHERE release_group_mbid IS NOT NULL; diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index c9b88fa9..1a7ec259 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -1,6 +1,6 @@ -- name: UpsertAlbum :one -INSERT INTO albums (title, sort_title, artist_id, release_date, mbid, cover_art_path) -VALUES ($1, $2, $3, $4, $5, $6) +INSERT INTO albums (title, sort_title, artist_id, release_date, mbid, cover_art_path, release_group_mbid) +VALUES ($1, $2, $3, $4, $5, $6, $7) ON CONFLICT (mbid) WHERE mbid IS NOT NULL DO UPDATE SET title = EXCLUDED.title, @@ -8,6 +8,7 @@ DO UPDATE SET artist_id = EXCLUDED.artist_id, release_date = EXCLUDED.release_date, cover_art_path = EXCLUDED.cover_art_path, + release_group_mbid = COALESCE(EXCLUDED.release_group_mbid, albums.release_group_mbid), updated_at = now() RETURNING *; @@ -142,6 +143,15 @@ UPDATE albums updated_at = now() WHERE id = $1 AND mbid IS NULL; +-- name: SetAlbumReleaseGroupMbidIfNull :exec +-- #5241: heal the release-group id on an existing album row, from a rescan's +-- tags or from the re-acquisition sweeper's MusicBrainz lookup. Only fills a +-- NULL, so a tag never gets overwritten by a later guess or vice versa. +UPDATE albums + SET release_group_mbid = $2, + updated_at = now() + WHERE id = $1 AND release_group_mbid IS NULL; + -- name: ListAlbumsMissingMbidWithTrack :many -- One-shot MBID backfill: returns each album where mbid IS NULL alongside -- one of its tracks' file_path so the worker can re-read tags. LIMIT diff --git a/internal/library/mbids.go b/internal/library/mbids.go index 3287b012..e63550b7 100644 --- a/internal/library/mbids.go +++ b/internal/library/mbids.go @@ -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 "" diff --git a/internal/library/mbids_test.go b/internal/library/mbids_test.go index ccafeb32..db5225dc 100644 --- a/internal/library/mbids_test.go +++ b/internal/library/mbids_test.go @@ -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) + } +} diff --git a/internal/library/scanner.go b/internal/library/scanner.go index 496142eb..32fb0b26 100644 --- a/internal/library/scanner.go +++ b/internal/library/scanner.go @@ -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 diff --git a/internal/library/scanner_test.go b/internal/library/scanner_test.go index b17741a9..b5404c63 100644 --- a/internal/library/scanner_test.go +++ b/internal/library/scanner_test.go @@ -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) + } +} From 2b4e274b127edee85ccf30d39718999a5055b266 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 11:34:04 -0400 Subject: [PATCH 2/3] feat(tags): resolve a MusicBrainz release to its release group (M483 #5243) ReleaseGroupForRelease asks /ws/2/release/?inc=release-groups through the registered MusicBrainz provider, so it shares that provider's client and 1 req/s limiter with tag enrichment and respects its on/off switch. ErrNotFound when switched off or MusicBrainz has no such release (an id that is already a release group included); ErrTransient to retry. Co-Authored-By: Claude Opus 5.5 --- internal/tags/provider_musicbrainz.go | 48 ++++++++++++++++++++++ internal/tags/provider_musicbrainz_test.go | 34 +++++++++++++++ 2 files changed, 82 insertions(+) diff --git a/internal/tags/provider_musicbrainz.go b/internal/tags/provider_musicbrainz.go index 5a0362c5..b652a917 100644 --- a/internal/tags/provider_musicbrainz.go +++ b/internal/tags/provider_musicbrainz.go @@ -155,6 +155,54 @@ func (p *musicbrainzProvider) TestConnection(ctx context.Context) error { return err } +// ReleaseGroupForRelease names the MusicBrainz release group a release +// belongs to (#5241). Minstrel's albums.mbid is the release id from the tags, +// while Lidarr knows albums only by release group, so re-acquiring a lost +// album needs this translation when its files (and their release-group tag) +// are already gone. +// +// It goes through the registered MusicBrainz provider, so the lookup shares +// that provider's client and 1 req/s limiter with tag enrichment rather than +// doubling Minstrel's request rate, and it respects the provider's on/off +// switch. ErrNotFound: MusicBrainz is switched off, or has no release under +// that id (an id that is already a release group lands here too). +// ErrTransient: retry later. +func ReleaseGroupForRelease(ctx context.Context, releaseMBID string) (string, error) { + p, err := ProviderByID("musicbrainz") + if err != nil { + return "", ErrNotFound + } + mb, ok := p.(*musicbrainzProvider) + if !ok { + return "", ErrNotFound + } + return mb.releaseGroupForRelease(ctx, releaseMBID) +} + +// mbReleaseResponse models the subset of /ws/2/release/{mbid}?inc=release-groups +// we read. +type mbReleaseResponse struct { + ReleaseGroup struct { + ID string `json:"id"` + } `json:"release-group"` +} + +func (p *musicbrainzProvider) releaseGroupForRelease(ctx context.Context, releaseMBID string) (string, error) { + if !p.enabled.Load() || releaseMBID == "" { + return "", ErrNotFound + } + q := url.Values{"inc": {"release-groups"}, "fmt": {"json"}} + full := mbBaseURL + "/release/" + url.PathEscape(releaseMBID) + "?" + q.Encode() + var resp mbReleaseResponse + if err := p.client.getJSON(ctx, full, &resp); err != nil { + return "", err + } + if resp.ReleaseGroup.ID == "" { + return "", ErrNotFound + } + return resp.ReleaseGroup.ID, nil +} + // normalizeMBTags drops non-positive-vote tags (net-downvoted or zero) and // scales the remaining vote counts into [0,1] relative to the strongest tag // on this recording, so one recording's raw counts don't dominate another's. diff --git a/internal/tags/provider_musicbrainz_test.go b/internal/tags/provider_musicbrainz_test.go index 8af8774c..9de04629 100644 --- a/internal/tags/provider_musicbrainz_test.go +++ b/internal/tags/provider_musicbrainz_test.go @@ -176,3 +176,37 @@ func TestMusicBrainzFetch_EmptyTags(t *testing.T) { t.Errorf("empty tags: err = %v, want ErrNotFound", err) } } + +// #5241: a release id resolves to its release group through +// /release/?inc=release-groups; an unknown id (or one that is already a +// group) and a switched-off source both say ErrNotFound. +func TestMusicBrainzReleaseGroupForRelease(t *testing.T) { + const release, group = "rel-1", "group-1" + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if r.URL.Path != "/release/"+release { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"Not Found"}`)) + return + } + if got := r.URL.Query().Get("inc"); got != "release-groups" { + t.Errorf("inc = %q, want release-groups", got) + } + _, _ = w.Write([]byte(`{"id":"` + release + `","title":"X","release-group":{"id":"` + group + `","title":"X"}}`)) + })) + defer srv.Close() + old := mbBaseURL + mbBaseURL = srv.URL + defer func() { mbBaseURL = old }() + + ctx := context.Background() + got, err := newMBProvider(true).releaseGroupForRelease(ctx, release) + if err != nil || got != group { + t.Fatalf("resolve = (%q, %v), want (%q, nil)", got, err, group) + } + if _, err := newMBProvider(true).releaseGroupForRelease(ctx, "group-1"); !errors.Is(err, ErrNotFound) { + t.Errorf("unknown release: err = %v, want ErrNotFound", err) + } + if _, err := newMBProvider(false).releaseGroupForRelease(ctx, release); !errors.Is(err, ErrNotFound) { + t.Errorf("switched off: err = %v, want ErrNotFound", err) + } +} From e509d7d5a9f20d7de071d5689d9e3e66788c4b36 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 7 Oct 2026 11:38:45 -0400 Subject: [PATCH 3/3] feat(lidarr): ask Lidarr for release groups; repair stored release-id requests (M483 #5244) Lidarr's metadata is keyed by MusicBrainz release group, but re-acquisition requested albums by their release id, so every add came back "not found". - Sweeper requests an album by its tag-supplied release group, else the one MusicBrainz names (cached onto the album). An album MusicBrainz cannot name is skipped and counted, with no attempt spent. - Reconciler: an add refused as not found re-reads the request's album id as a release (library first, then MusicBrainz), rewrites the request to the group and adds again. This repairs the requests already stored. - Completion matches an album by release id or release group, and only once a track of it is on disk, so a re-acquisition request no longer completes against the row of the album it is trying to bring back. Closes #5241. Co-Authored-By: Claude Opus 5.5 --- internal/db/dbq/albums.sql.go | 35 +++ internal/db/dbq/lidarr_requests.sql.go | 20 ++ internal/db/dbq/reacquisition.sql.go | 21 +- internal/db/queries/albums.sql | 16 ++ internal/db/queries/lidarr_requests.sql | 9 + internal/db/queries/reacquisition.sql | 3 +- internal/lidarrrequests/reconciler.go | 94 +++++++- .../reconciler_integration_test.go | 1 + .../reconciler_release_group_test.go | 203 ++++++++++++++++++ internal/reacquisition/sweeper.go | 58 ++++- internal/reacquisition/sweeper_test.go | 163 ++++++++++++++ 11 files changed, 596 insertions(+), 27 deletions(-) create mode 100644 internal/lidarrrequests/reconciler_release_group_test.go create mode 100644 internal/reacquisition/sweeper_test.go diff --git a/internal/db/dbq/albums.sql.go b/internal/db/dbq/albums.sql.go index ff5aa098..baaf1f56 100644 --- a/internal/db/dbq/albums.sql.go +++ b/internal/db/dbq/albums.sql.go @@ -248,6 +248,22 @@ func (q *Queries) GetAlbumsByIDs(ctx context.Context, dollar_1 []pgtype.UUID) ([ return items, nil } +const getReleaseGroupForReleaseMbid = `-- name: GetReleaseGroupForReleaseMbid :one +SELECT release_group_mbid::text + FROM albums + WHERE mbid = $1 AND release_group_mbid IS NOT NULL + LIMIT 1 +` + +// #5241: the release group the library already knows for a release id, so a +// request carrying a release id can be repaired without asking MusicBrainz. +func (q *Queries) GetReleaseGroupForReleaseMbid(ctx context.Context, mbid *string) (string, error) { + row := q.db.QueryRow(ctx, getReleaseGroupForReleaseMbid, mbid) + var release_group_mbid string + err := row.Scan(&release_group_mbid) + return release_group_mbid, err +} + const listAlbumsAlphaByArtist = `-- name: ListAlbumsAlphaByArtist :many SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid, artists.sort_name AS artist_sort_name FROM albums @@ -852,6 +868,25 @@ func (q *Queries) SetAlbumReleaseGroupMbidIfNull(ctx context.Context, arg SetAlb return err } +const setReleaseGroupForReleaseMbidIfNull = `-- name: SetReleaseGroupForReleaseMbidIfNull :exec +UPDATE albums + SET release_group_mbid = $1::text, + updated_at = now() + WHERE mbid = $2::text AND release_group_mbid IS NULL +` + +type SetReleaseGroupForReleaseMbidIfNullParams struct { + ReleaseGroupMbid string + ReleaseMbid string +} + +// #5241: cache a MusicBrainz-resolved release group onto the album row that +// holds the release, filling only a NULL. +func (q *Queries) SetReleaseGroupForReleaseMbidIfNull(ctx context.Context, arg SetReleaseGroupForReleaseMbidIfNullParams) error { + _, err := q.db.Exec(ctx, setReleaseGroupForReleaseMbidIfNull, arg.ReleaseGroupMbid, arg.ReleaseMbid) + return err +} + const upsertAlbum = `-- name: UpsertAlbum :one INSERT INTO albums (title, sort_title, artist_id, release_date, mbid, cover_art_path, release_group_mbid) VALUES ($1, $2, $3, $4, $5, $6, $7) diff --git a/internal/db/dbq/lidarr_requests.sql.go b/internal/db/dbq/lidarr_requests.sql.go index 73f2c716..ecaac112 100644 --- a/internal/db/dbq/lidarr_requests.sql.go +++ b/internal/db/dbq/lidarr_requests.sql.go @@ -560,3 +560,23 @@ func (q *Queries) RejectLidarrRequest(ctx context.Context, arg RejectLidarrReque ) return i, err } + +const setLidarrRequestAlbumMbid = `-- name: SetLidarrRequestAlbumMbid :exec +UPDATE lidarr_requests + SET lidarr_album_mbid = $2, + updated_at = now() + WHERE id = $1 +` + +type SetLidarrRequestAlbumMbidParams struct { + ID pgtype.UUID + LidarrAlbumMbid *string +} + +// #5241: repoint a request at the album's release group. Requests made before +// the release-group read named albums by MusicBrainz release id, which Lidarr +// does not index; the reconciler rewrites them once it has resolved the group. +func (q *Queries) SetLidarrRequestAlbumMbid(ctx context.Context, arg SetLidarrRequestAlbumMbidParams) error { + _, err := q.db.Exec(ctx, setLidarrRequestAlbumMbid, arg.ID, arg.LidarrAlbumMbid) + return err +} diff --git a/internal/db/dbq/reacquisition.sql.go b/internal/db/dbq/reacquisition.sql.go index 335e2771..0a4e9da0 100644 --- a/internal/db/dbq/reacquisition.sql.go +++ b/internal/db/dbq/reacquisition.sql.go @@ -114,6 +114,7 @@ const listAlbumsDueReacquisition = `-- name: ListAlbumsDueReacquisition :many SELECT albums.id AS album_id, albums.title AS album_title, albums.mbid AS album_mbid, + albums.release_group_mbid AS album_release_group_mbid, artists.id AS artist_id, artists.name AS artist_name, artists.mbid AS artist_mbid, @@ -135,7 +136,7 @@ SELECT albums.id AS album_id, * POWER(2, GREATEST(COALESCE(r.attempts, 0) - 1, 0)))::int, $3::int)) ) - GROUP BY albums.id, albums.title, albums.mbid, + GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid, artists.id, artists.name, artists.mbid, r.attempts, r.last_attempt_at ORDER BY r.last_attempt_at NULLS FIRST, albums.sort_title LIMIT $4 @@ -149,14 +150,15 @@ type ListAlbumsDueReacquisitionParams struct { } type ListAlbumsDueReacquisitionRow struct { - AlbumID pgtype.UUID - AlbumTitle string - AlbumMbid *string - ArtistID pgtype.UUID - ArtistName string - ArtistMbid *string - MissingTrackCount int64 - Attempts int32 + AlbumID pgtype.UUID + AlbumTitle string + AlbumMbid *string + AlbumReleaseGroupMbid *string + ArtistID pgtype.UUID + ArtistName string + ArtistMbid *string + MissingTrackCount int64 + Attempts int32 } // The sweeper's selection. An album qualifies when: @@ -194,6 +196,7 @@ func (q *Queries) ListAlbumsDueReacquisition(ctx context.Context, arg ListAlbums &i.AlbumID, &i.AlbumTitle, &i.AlbumMbid, + &i.AlbumReleaseGroupMbid, &i.ArtistID, &i.ArtistName, &i.ArtistMbid, diff --git a/internal/db/queries/albums.sql b/internal/db/queries/albums.sql index 1a7ec259..4ce5452f 100644 --- a/internal/db/queries/albums.sql +++ b/internal/db/queries/albums.sql @@ -152,6 +152,22 @@ UPDATE albums updated_at = now() WHERE id = $1 AND release_group_mbid IS NULL; +-- name: GetReleaseGroupForReleaseMbid :one +-- #5241: the release group the library already knows for a release id, so a +-- request carrying a release id can be repaired without asking MusicBrainz. +SELECT release_group_mbid::text + FROM albums + WHERE mbid = $1 AND release_group_mbid IS NOT NULL + LIMIT 1; + +-- name: SetReleaseGroupForReleaseMbidIfNull :exec +-- #5241: cache a MusicBrainz-resolved release group onto the album row that +-- holds the release, filling only a NULL. +UPDATE albums + SET release_group_mbid = sqlc.arg(release_group_mbid)::text, + updated_at = now() + WHERE mbid = sqlc.arg(release_mbid)::text AND release_group_mbid IS NULL; + -- name: ListAlbumsMissingMbidWithTrack :many -- One-shot MBID backfill: returns each album where mbid IS NULL alongside -- one of its tracks' file_path so the worker can re-read tags. LIMIT diff --git a/internal/db/queries/lidarr_requests.sql b/internal/db/queries/lidarr_requests.sql index 7561acb0..ec96568c 100644 --- a/internal/db/queries/lidarr_requests.sql +++ b/internal/db/queries/lidarr_requests.sql @@ -119,3 +119,12 @@ UPDATE lidarr_requests WHERE id = $1 AND status = 'approved' AND lidarr_add_confirmed_at IS NULL; + +-- name: SetLidarrRequestAlbumMbid :exec +-- #5241: repoint a request at the album's release group. Requests made before +-- the release-group read named albums by MusicBrainz release id, which Lidarr +-- does not index; the reconciler rewrites them once it has resolved the group. +UPDATE lidarr_requests + SET lidarr_album_mbid = $2, + updated_at = now() + WHERE id = $1; diff --git a/internal/db/queries/reacquisition.sql b/internal/db/queries/reacquisition.sql index 5f638e3e..85f3de86 100644 --- a/internal/db/queries/reacquisition.sql +++ b/internal/db/queries/reacquisition.sql @@ -40,6 +40,7 @@ RETURNING *; SELECT albums.id AS album_id, albums.title AS album_title, albums.mbid AS album_mbid, + albums.release_group_mbid AS album_release_group_mbid, artists.id AS artist_id, artists.name AS artist_name, artists.mbid AS artist_mbid, @@ -61,7 +62,7 @@ SELECT albums.id AS album_id, * POWER(2, GREATEST(COALESCE(r.attempts, 0) - 1, 0)))::int, sqlc.arg(backoff_max_hours)::int)) ) - GROUP BY albums.id, albums.title, albums.mbid, + GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid, artists.id, artists.name, artists.mbid, r.attempts, r.last_attempt_at ORDER BY r.last_attempt_at NULLS FIRST, albums.sort_title LIMIT sqlc.arg(page_limit); diff --git a/internal/lidarrrequests/reconciler.go b/internal/lidarrrequests/reconciler.go index 22b3c5cb..589f5264 100644 --- a/internal/lidarrrequests/reconciler.go +++ b/internal/lidarrrequests/reconciler.go @@ -2,6 +2,7 @@ package lidarrrequests import ( "context" + "errors" "fmt" "log/slog" "time" @@ -14,6 +15,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/eventbus" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" ) // Reconciler is a background worker that periodically scans approved @@ -30,6 +32,10 @@ type Reconciler struct { bus *eventbus.Bus tick time.Duration batch int32 + // releaseGroup names the MusicBrainz release group of a release id, to + // repair a request stored under a release id Lidarr does not index + // (#5241). A field so tests can stand in for MusicBrainz. + releaseGroup func(ctx context.Context, releaseMBID string) (string, error) } // NewReconciler constructs a Reconciler with production defaults: @@ -50,6 +56,8 @@ func NewReconciler(pool *pgxpool.Pool, cfg *lidarrconfig.Service, clientFn func( bus: bus, tick: 5 * time.Minute, batch: 50, + + releaseGroup: tags.ReleaseGroupForRelease, } } @@ -185,12 +193,84 @@ func (r *Reconciler) ensureLidarrAdd(ctx context.Context, q *dbq.Queries, cfg li // cfg.Enabled, so this is just defensive — nothing to do. return nil } - if err := sendLidarrAdd(ctx, client, cfg, row); err != nil { + err := sendLidarrAdd(ctx, client, cfg, row) + if errors.Is(err, lidarr.ErrNotFound) { + // Lidarr's metadata has no album under this id. Requests made before + // albums carried a release group (#5241) name the album by its + // MusicBrainz release id, which Lidarr does not index: repoint the + // request at the release group and try once more. + if repaired, ok := r.repointAtReleaseGroup(ctx, q, row); ok { + row = repaired + err = sendLidarrAdd(ctx, client, cfg, row) + } + } + if err != nil { return fmt.Errorf("lidarr add retry: %w", err) } return q.MarkLidarrRequestAddConfirmed(ctx, row.ID) } +// repointAtReleaseGroup reads an album or track request's album id as a +// MusicBrainz release and, when its release group can be named, rewrites the +// request to it. The library's own albums answer first; MusicBrainz is asked +// only for a release no album row knows the group of, and its answer is cached +// onto that album. Reports false when there is nothing to repoint: not an +// album request, the group is unknown, or the id already is the group. +func (r *Reconciler) repointAtReleaseGroup(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) (dbq.LidarrRequest, bool) { + if row.Kind == dbq.LidarrRequestKindArtist || row.LidarrAlbumMbid == nil || *row.LidarrAlbumMbid == "" { + return row, false + } + release := *row.LidarrAlbumMbid + group, err := q.GetReleaseGroupForReleaseMbid(ctx, &release) + if err != nil { + if !isNoRows(err) { + r.logger.Warn("lidarrrequests: release group lookup failed", "request_id", row.ID, "err", err) + return row, false + } + group, err = r.releaseGroup(ctx, release) + if err != nil { + if !errors.Is(err, tags.ErrNotFound) { + r.logger.Warn("lidarrrequests: MusicBrainz release group lookup failed", + "request_id", row.ID, "release_mbid", release, "err", err) + } + return row, false + } + if cerr := q.SetReleaseGroupForReleaseMbidIfNull(ctx, dbq.SetReleaseGroupForReleaseMbidIfNullParams{ + ReleaseGroupMbid: group, + ReleaseMbid: release, + }); cerr != nil { + r.logger.Warn("lidarrrequests: cache release group failed", "release_mbid", release, "err", cerr) + } + } + if group == "" || group == release { + return row, false + } + if err := q.SetLidarrRequestAlbumMbid(ctx, dbq.SetLidarrRequestAlbumMbidParams{ + ID: row.ID, + LidarrAlbumMbid: &group, + }); err != nil { + r.logger.Warn("lidarrrequests: repoint request failed", "request_id", row.ID, "err", err) + return row, false + } + r.logger.Info("lidarrrequests: request repointed from release to release group", + "request_id", row.ID, "release_mbid", release, "release_group_mbid", group) + row.LidarrAlbumMbid = &group + return row, true +} + +// albumForRequest finds the library album a request's album id names, by +// release id or release group, and only once it has a track back on disk. A +// re-acquisition request names an album whose row never went away; matching +// the row alone would complete the request before Lidarr delivered anything. +// An exact release match is preferred over another release of the group. +const albumForRequest = ` +SELECT a.id + FROM albums a + WHERE (a.mbid = $1 OR a.release_group_mbid = $1) + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id AND t.missing_since IS NULL) + ORDER BY (a.mbid = $1) DESC, a.id + LIMIT 1` + func (r *Reconciler) reconcileArtist(ctx context.Context, q *dbq.Queries, row dbq.LidarrRequest) error { var artistID pgtype.UUID err := r.pool.QueryRow(ctx, @@ -221,10 +301,7 @@ func (r *Reconciler) reconcileAlbum(ctx context.Context, q *dbq.Queries, row dbq return nil } var albumID pgtype.UUID - err := r.pool.QueryRow(ctx, - "SELECT id FROM albums WHERE mbid = $1", - *row.LidarrAlbumMbid, - ).Scan(&albumID) + err := r.pool.QueryRow(ctx, albumForRequest, *row.LidarrAlbumMbid).Scan(&albumID) if err != nil { if isNoRows(err) { return nil @@ -250,10 +327,7 @@ func (r *Reconciler) reconcileTrack(ctx context.Context, q *dbq.Queries, row dbq } // Track-kind requests match via their parent album's MBID, not track.mbid. var albumID pgtype.UUID - err := r.pool.QueryRow(ctx, - "SELECT id FROM albums WHERE mbid = $1", - *row.LidarrAlbumMbid, - ).Scan(&albumID) + err := r.pool.QueryRow(ctx, albumForRequest, *row.LidarrAlbumMbid).Scan(&albumID) if err != nil { if isNoRows(err) { return nil @@ -264,7 +338,7 @@ func (r *Reconciler) reconcileTrack(ctx context.Context, q *dbq.Queries, row dbq // Load any track from that album to set matched_track_id. var trackID pgtype.UUID err = r.pool.QueryRow(ctx, - "SELECT id FROM tracks WHERE album_id = $1 ORDER BY id LIMIT 1", + "SELECT id FROM tracks WHERE album_id = $1 AND missing_since IS NULL ORDER BY id LIMIT 1", albumID, ).Scan(&trackID) if err != nil { diff --git a/internal/lidarrrequests/reconciler_integration_test.go b/internal/lidarrrequests/reconciler_integration_test.go index 959b6067..6458f83f 100644 --- a/internal/lidarrrequests/reconciler_integration_test.go +++ b/internal/lidarrrequests/reconciler_integration_test.go @@ -152,6 +152,7 @@ func TestReconciler_MatchesAlbumByMBID(t *testing.T) { artist := seedArtist(t, q, "Test Artist", artistMBID) album := seedAlbum(t, q, artist.ID, "Test Album", albumMBID) + _ = seedTrack(t, q, album.ID, artist.ID, "Album Track", "/music/album-test/01.flac") req := seedApprovedRequestDirect(t, q, user, CreateParams{ Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "Test Artist", diff --git a/internal/lidarrrequests/reconciler_release_group_test.go b/internal/lidarrrequests/reconciler_release_group_test.go new file mode 100644 index 00000000..f8cd3a22 --- /dev/null +++ b/internal/lidarrrequests/reconciler_release_group_test.go @@ -0,0 +1,203 @@ +package lidarrrequests + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "sync" + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarr" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" +) + +// #5241: Lidarr names albums by MusicBrainz release group. A request carrying +// a group id completes against the album row whose tags named that group, +// once a track of it is on disk. +func TestReconciler_AlbumMatchesByReleaseGroup(t *testing.T) { + pool := newPool(t) + q := dbq.New(pool) + ctx := context.Background() + enableLidarrForPool(t, pool) + user := seedUser(t, pool) + + const artistMBID, release, group = "rg-artist-match", "rg-release-match", "rg-group-match" + artist := seedArtist(t, q, "RG Artist", artistMBID) + album := seedAlbum(t, q, artist.ID, "RG Album", release) + if err := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{ + ID: album.ID, ReleaseGroupMbid: nilableStr(group), + }); err != nil { + t.Fatal(err) + } + _ = seedTrack(t, q, album.ID, artist.ID, "RG One", "/music/rg-match/01.flac") + + req := seedApprovedRequestDirect(t, q, user, CreateParams{ + Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "RG Artist", + LidarrAlbumMBID: group, AlbumTitle: "RG Album", + }) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) + if err := rec.tickOnce(ctx); err != nil { + t.Fatalf("tickOnce: %v", err) + } + got, err := q.GetLidarrRequestByID(ctx, req.ID) + if err != nil { + t.Fatal(err) + } + if got.Status != dbq.LidarrRequestStatusCompleted || got.MatchedAlbumID != album.ID { + t.Errorf("status %v matched %v, want completed on %v", got.Status, got.MatchedAlbumID, album.ID) + } +} + +// A re-acquisition request names an album whose row never went away. The row +// alone must not complete it: nothing has come back until a track is on disk. +func TestReconciler_AlbumWithEveryTrackMissingStaysApproved(t *testing.T) { + pool := newPool(t) + q := dbq.New(pool) + ctx := context.Background() + enableLidarrForPool(t, pool) + user := seedUser(t, pool) + + const artistMBID, release = "rg-artist-missing", "rg-release-missing" + artist := seedArtist(t, q, "Missing Artist", artistMBID) + album := seedAlbum(t, q, artist.ID, "Missing Album", release) + tr := seedTrack(t, q, album.ID, artist.ID, "Gone", "/music/rg-missing/01.flac") + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", tr.ID); err != nil { + t.Fatal(err) + } + + req := seedApprovedRequestDirect(t, q, user, CreateParams{ + Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "Missing Artist", + LidarrAlbumMBID: release, AlbumTitle: "Missing Album", + }) + rec := NewReconciler(pool, lidarrconfig.New(pool), nil, newTestLogger(), nil) + if err := rec.tickOnce(ctx); err != nil { + t.Fatalf("tickOnce: %v", err) + } + got, err := q.GetLidarrRequestByID(ctx, req.ID) + if err != nil { + t.Fatal(err) + } + if got.Status != dbq.LidarrRequestStatusApproved { + t.Errorf("status = %v, want approved while every track is missing", got.Status) + } +} + +// fakeAlbumLidarr answers album lookups only for knownGroup (Lidarr's metadata +// is keyed by release group) and records every add. +type fakeAlbumLidarr struct { + knownGroup string + mu sync.Mutex + added []string +} + +func (f *fakeAlbumLidarr) handler(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + switch { + case strings.HasSuffix(r.URL.Path, "/metadataprofile"): + _, _ = w.Write([]byte(`[{"id":1,"name":"Standard"}]`)) + case strings.HasSuffix(r.URL.Path, "/album/lookup"): + if r.URL.Query().Get("term") == "lidarr:"+f.knownGroup { + _, _ = w.Write([]byte(`[{"foreignAlbumId":"` + f.knownGroup + `","artist":{"id":42}}]`)) + return + } + _, _ = w.Write([]byte(`[]`)) + case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/album"): + var body map[string]any + _ = json.NewDecoder(r.Body).Decode(&body) + f.mu.Lock() + f.added = append(f.added, body["foreignAlbumId"].(string)) + f.mu.Unlock() + w.WriteHeader(http.StatusCreated) + _, _ = w.Write([]byte(`{"id":9}`)) + default: + w.WriteHeader(http.StatusNotFound) + } +} + +// The ~49 requests on the deploy were stored under release ids. The reconciler +// resolves the group (MusicBrainz here, since no album row knows it), rewrites +// the request, adds it, and caches the group onto the album holding the +// release. +func TestReconciler_RepointsAReleaseIdRequestAtItsReleaseGroup(t *testing.T) { + pool := newPool(t) + q := dbq.New(pool) + ctx := context.Background() + user := seedUser(t, pool) + + const artistMBID, release, group = "rg-artist-repoint", "rg-release-repoint", "rg-group-repoint" + fake := &fakeAlbumLidarr{knownGroup: group} + srv := httptest.NewServer(http.HandlerFunc(fake.handler)) + t.Cleanup(srv.Close) + if err := lidarrconfig.New(pool).Save(ctx, lidarrconfig.Config{ + Enabled: true, BaseURL: srv.URL, APIKey: "k", + DefaultQualityProfileID: 7, DefaultRootFolderPath: "/music", + }); err != nil { + t.Fatal(err) + } + + artist := seedArtist(t, q, "Repoint Artist", artistMBID) + album := seedAlbum(t, q, artist.ID, "Repoint Album", release) + tr := seedTrack(t, q, album.ID, artist.ID, "Lost", "/music/rg-repoint/01.flac") + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() WHERE id = $1", tr.ID); err != nil { + t.Fatal(err) + } + req := seedApprovedRequestDirect(t, q, user, CreateParams{ + Kind: "album", LidarrArtistMBID: artistMBID, ArtistName: "Repoint Artist", + LidarrAlbumMBID: release, AlbumTitle: "Repoint Album", + }) + + rec := NewReconciler(pool, lidarrconfig.New(pool), + func() *lidarr.Client { return lidarr.NewClient(srv.URL, "k") }, newTestLogger(), nil) + asked := 0 + rec.releaseGroup = func(_ context.Context, id string) (string, error) { + asked++ + if id == release { + return group, nil + } + return "", tags.ErrNotFound + } + if err := rec.tickOnce(ctx); err != nil { + t.Fatalf("tickOnce: %v", err) + } + + got, err := q.GetLidarrRequestByID(ctx, req.ID) + if err != nil { + t.Fatal(err) + } + if got.LidarrAlbumMbid == nil || *got.LidarrAlbumMbid != group { + t.Errorf("request album mbid = %v, want the release group %s", got.LidarrAlbumMbid, group) + } + if !got.LidarrAddConfirmedAt.Valid { + t.Error("add not confirmed after the repoint") + } + fake.mu.Lock() + added := append([]string(nil), fake.added...) + fake.mu.Unlock() + if len(added) != 1 || added[0] != group { + t.Errorf("Lidarr adds = %v, want [%s]", added, group) + } + var cached *string + if err := pool.QueryRow(ctx, "SELECT release_group_mbid FROM albums WHERE id = $1", album.ID).Scan(&cached); err != nil { + t.Fatal(err) + } + if cached == nil || *cached != group { + t.Errorf("album release group = %v, want %s cached", cached, group) + } + if got.Status != dbq.LidarrRequestStatusApproved { + t.Errorf("status = %v, want approved until a track is back", got.Status) + } + + // The next tick finds the group on the request and the album: no second + // MusicBrainz question. + before := asked + if err := rec.tickOnce(ctx); err != nil { + t.Fatalf("second tick: %v", err) + } + if asked != before { + t.Errorf("MusicBrainz asked again on the next tick (%d -> %d)", before, asked) + } +} diff --git a/internal/reacquisition/sweeper.go b/internal/reacquisition/sweeper.go index e14d1820..16875b40 100644 --- a/internal/reacquisition/sweeper.go +++ b/internal/reacquisition/sweeper.go @@ -13,6 +13,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" ) // requestCreator is the slice of lidarrrequests.Service the sweeper needs, @@ -37,6 +38,10 @@ type Sweeper struct { requests requestCreator logger *slog.Logger tick time.Duration + // releaseGroup names the MusicBrainz release group of a release id, for an + // album whose tags never carried one (#5241). Lidarr knows albums only by + // release group. A field so tests can stand in for MusicBrainz. + releaseGroup func(ctx context.Context, releaseMBID string) (string, error) } // NewSweeper constructs a Sweeper. The tick is deliberately coarse: the @@ -49,11 +54,12 @@ func NewSweeper( logger *slog.Logger, ) *Sweeper { return &Sweeper{ - pool: pool, - settings: settings, - requests: requests, - logger: logger, - tick: 1 * time.Hour, + pool: pool, + settings: settings, + requests: requests, + logger: logger, + tick: 1 * time.Hour, + releaseGroup: tags.ReleaseGroupForRelease, } } @@ -80,6 +86,10 @@ type PassResult struct { Approved int // of those, sent on to Lidarr GaveUp int // albums that spent their attempt budget Unnameable int64 // albums with missing files but no MBID to ask for + // Unresolved: albums skipped this pass because MusicBrainz could not name + // their release group (switched off, or no such release). Lidarr cannot be + // asked for them, and no attempt is spent; the next pass tries again. + Unresolved int } // SweepOnce runs one pass. Exported so the admin surface can offer a "run @@ -160,11 +170,23 @@ func (s *Sweeper) attempt( return nil } + // Lidarr names albums by release group; albums.mbid is the release. + group, err := s.albumReleaseGroup(ctx, q, album) + if err != nil { + if errors.Is(err, tags.ErrNotFound) { + res.Unresolved++ + s.logger.Info("reacquisition: no MusicBrainz release group for album; skipped", + "album", album.AlbumTitle, "release_mbid", *album.AlbumMbid) + return nil + } + return fmt.Errorf("release group: %w", err) + } + req, err := s.requests.Create(ctx, adminID, lidarrrequests.CreateParams{ Kind: "album", LidarrArtistMBID: *album.ArtistMbid, ArtistName: album.ArtistName, - LidarrAlbumMBID: *album.AlbumMbid, + LidarrAlbumMBID: group, AlbumTitle: album.AlbumTitle, }) if err != nil { @@ -212,10 +234,31 @@ func (s *Sweeper) attempt( return nil } +// albumReleaseGroup is the album's release group: the one its tags carried, +// else MusicBrainz's answer for its release id, cached onto the album so the +// next pass (and request completion) need not ask again. Returns +// tags.ErrNotFound when MusicBrainz cannot name it. +func (s *Sweeper) albumReleaseGroup(ctx context.Context, q *dbq.Queries, album dbq.ListAlbumsDueReacquisitionRow) (string, error) { + if album.AlbumReleaseGroupMbid != nil && *album.AlbumReleaseGroupMbid != "" { + return *album.AlbumReleaseGroupMbid, nil + } + group, err := s.releaseGroup(ctx, *album.AlbumMbid) + if err != nil { + return "", err + } + if cerr := q.SetAlbumReleaseGroupMbidIfNull(ctx, dbq.SetAlbumReleaseGroupMbidIfNullParams{ + ID: album.AlbumID, + ReleaseGroupMbid: &group, + }); cerr != nil { + s.logger.Warn("reacquisition: cache release group failed", "album", album.AlbumTitle, "err", cerr) + } + return group, nil +} + func (s *Sweeper) logSummary(res PassResult) { // Silence when a pass did nothing at all — this runs hourly forever, and // an unconditional line would bury the passes that mattered. - if res.Requested == 0 && res.Cleared == 0 && res.GaveUp == 0 { + if res.Requested == 0 && res.Cleared == 0 && res.GaveUp == 0 && res.Unresolved == 0 { return } s.logger.Info("reacquisition: sweep", @@ -224,5 +267,6 @@ func (s *Sweeper) logSummary(res PassResult) { "gave_up", res.GaveUp, "cleared", res.Cleared, "unnameable", res.Unnameable, + "unresolved", res.Unresolved, ) } diff --git a/internal/reacquisition/sweeper_test.go b/internal/reacquisition/sweeper_test.go new file mode 100644 index 00000000..d8833a69 --- /dev/null +++ b/internal/reacquisition/sweeper_test.go @@ -0,0 +1,163 @@ +package reacquisition + +import ( + "context" + "io" + "log/slog" + "os" + "testing" + + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig" + "git.fabledsword.com/bvandeusen/minstrel/internal/lidarrrequests" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" +) + +func newSweepPool(t *testing.T) *pgxpool.Pool { + t.Helper() + if testing.Short() { + t.Skip("skipping integration test in -short mode") + } + dsn := os.Getenv("MINSTREL_TEST_DATABASE_URL") + if dsn == "" { + t.Skip("MINSTREL_TEST_DATABASE_URL not set") + } + if err := db.Migrate(dsn, slog.New(slog.NewTextHandler(io.Discard, nil))); err != nil { + t.Fatalf("migrate: %v", err) + } + pool, err := pgxpool.New(context.Background(), dsn) + if err != nil { + t.Fatalf("pool: %v", err) + } + t.Cleanup(pool.Close) + dbtest.ResetDB(t, pool) + if _, err := pool.Exec(context.Background(), "DELETE FROM lidarr_requests"); err != nil { + t.Fatalf("reset requests: %v", err) + } + return pool +} + +// lostAlbum seeds an album whose only track has been missing for two days, +// named by MusicBrainz release id and (when group is set) release group. +func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) pgtype.UUID { + t.Helper() + ctx := context.Background() + q := dbq.New(pool) + artistMBID := "artist-" + release + ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: title + " Artist", SortName: title, Mbid: &artistMBID}) + if err != nil { + t.Fatal(err) + } + params := dbq.UpsertAlbumParams{Title: title, SortTitle: title, ArtistID: ar.ID, Mbid: &release} + if group != "" { + params.ReleaseGroupMbid = &group + } + al, err := q.UpsertAlbum(ctx, params) + if err != nil { + t.Fatal(err) + } + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: "Lost", AlbumID: al.ID, ArtistID: ar.ID, DurationMs: 180000, + FilePath: "/music/" + release + "/01.flac", FileSize: 1, FileFormat: "flac", + }) + if err != nil { + t.Fatal(err) + } + if _, err := pool.Exec(ctx, "UPDATE tracks SET missing_since = now() - interval '2 days' WHERE id = $1", tr.ID); err != nil { + t.Fatal(err) + } + return al.ID +} + +// #5241: re-acquisition asks Lidarr for the album's release group, never its +// release id. A tag-supplied group is used as is; otherwise MusicBrainz names +// it and the answer is cached onto the album; an album MusicBrainz cannot +// name is skipped without spending an attempt. +func TestSweep_RequestsAlbumsByReleaseGroup_Integration(t *testing.T) { + pool := newSweepPool(t) + ctx := context.Background() + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + q := dbq.New(pool) + + if _, err := q.CreateUser(ctx, dbq.CreateUserParams{ + Username: dbtest.TestUserPrefix + "sweepadmin", PasswordHash: "x", ApiTokenHash: "x", IsAdmin: true, + }); err != nil { + t.Fatal(err) + } + settings, err := NewSettingsService(ctx, pool, logger) + if err != nil { + t.Fatal(err) + } + cfg := Defaults + cfg.AutoApprove = false + if _, err := settings.Set(ctx, cfg); err != nil { + t.Fatal(err) + } + + tagged := lostAlbum(t, pool, "Tagged", "release-tagged", "group-tagged") + resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "") + unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "") + + requests := lidarrrequests.NewService(pool, lidarrconfig.New(pool), nil, nil) + s := NewSweeper(pool, settings, requests, logger) + asked := map[string]int{} + s.releaseGroup = func(_ context.Context, release string) (string, error) { + asked[release]++ + if release == "release-resolved" { + return "group-resolved", nil + } + return "", tags.ErrNotFound + } + if err := s.SweepOnce(ctx); err != nil { + t.Fatalf("sweep: %v", err) + } + + requested := func(album pgtype.UUID) (string, bool) { + t.Helper() + var mbid *string + err := pool.QueryRow(ctx, ` +SELECT lr.lidarr_album_mbid + FROM missing_reacquisitions r + JOIN lidarr_requests lr ON lr.id = r.last_request_id + WHERE r.album_id = $1`, album).Scan(&mbid) + if err != nil || mbid == nil { + return "", false + } + return *mbid, true + } + if got, ok := requested(tagged); !ok || got != "group-tagged" { + t.Errorf("tagged album requested as %q (ok=%v), want group-tagged", got, ok) + } + if asked["release-tagged"] != 0 { + t.Error("MusicBrainz asked about an album whose tags named its group") + } + if got, ok := requested(resolved); !ok || got != "group-resolved" { + t.Errorf("resolved album requested as %q (ok=%v), want group-resolved", got, ok) + } + var cached *string + if err := pool.QueryRow(ctx, "SELECT release_group_mbid FROM albums WHERE id = $1", resolved).Scan(&cached); err != nil { + t.Fatal(err) + } + if cached == nil || *cached != "group-resolved" { + t.Errorf("resolved group not cached on the album: %v", cached) + } + var attempts int + if err := pool.QueryRow(ctx, "SELECT count(*) FROM missing_reacquisitions WHERE album_id = $1", unknown).Scan(&attempts); err != nil { + t.Fatal(err) + } + if attempts != 0 { + t.Errorf("unresolvable album spent an attempt (%d state rows)", attempts) + } + var stray int + if err := pool.QueryRow(ctx, "SELECT count(*) FROM lidarr_requests WHERE lidarr_album_mbid LIKE 'release-%'").Scan(&stray); err != nil { + t.Fatal(err) + } + if stray != 0 { + t.Errorf("%d request(s) named an album by its release id", stray) + } +}