diff --git a/internal/api/admin_wanted.go b/internal/api/admin_wanted.go index 8dec9100..b6e015a1 100644 --- a/internal/api/admin_wanted.go +++ b/internal/api/admin_wanted.go @@ -86,6 +86,16 @@ type wantsPage struct { Limit int `json:"limit"` Offset int `json:"offset"` Counts wantCounts `json:"counts"` + // Library is the one-time listing of the library's albums (#5567). + Library libraryListingView `json:"library"` +} + +// libraryListingView: whether the library's albums are all on the list +// yet, and how many could not be listed (nothing names them to +// MusicBrainz). +type libraryListingView struct { + ListedAt *string `json:"listed_at"` + Unlisted int32 `json:"unlisted"` } const wantsPageSize = 50 @@ -116,11 +126,21 @@ func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { writeErrWithLog(w, h.logger, "admin wanted: count", apierror.Internal(err)) return } + listing, err := q.GetLibraryListing(r.Context()) + if err != nil { + writeErrWithLog(w, h.logger, "admin wanted: library listing", apierror.Internal(err)) + return + } out := wantsPage{ - Items: make([]wantView, 0, len(rows)), Limit: wantsPageSize, Offset: offset, + Library: libraryListingView{Unlisted: listing.LibraryUnlisted}, + Items: make([]wantView, 0, len(rows)), Limit: wantsPageSize, Offset: offset, Counts: wantCounts{Wanted: counts.Wanted, Missing: counts.Missing, Held: counts.Held, Following: counts.Following, Activity: counts.Activity, Attention: counts.Attention, Blocked: counts.Blocked}, } + if listing.LibraryListedAt.Valid { + at := formatTimestamp(listing.LibraryListedAt) + out.Library.ListedAt = &at + } switch state { case "missing": out.Total = counts.Missing diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index d656ee35..d094ae0a 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -218,17 +218,20 @@ type AcquisitionService struct { } type AcquisitionSetting struct { - ID bool - AutoGrab bool - SearchIntervalHours int32 - AllowedQualities []string - LastPassAt pgtype.Timestamptz - LastPass []byte - UpdatedAt pgtype.Timestamptz - ImportRoot string - SeedRatio float32 - SeedDays int32 - StallHours int32 + ID bool + AutoGrab bool + SearchIntervalHours int32 + AllowedQualities []string + LastPassAt pgtype.Timestamptz + LastPass []byte + UpdatedAt pgtype.Timestamptz + ImportRoot string + SeedRatio float32 + SeedDays int32 + StallHours int32 + LibraryListingStartedAt pgtype.Timestamptz + LibraryListedAt pgtype.Timestamptz + LibraryUnlisted int32 } type Album struct { diff --git a/internal/db/dbq/reacquisition.sql.go b/internal/db/dbq/reacquisition.sql.go index c4ea7208..870f9bad 100644 --- a/internal/db/dbq/reacquisition.sql.go +++ b/internal/db/dbq/reacquisition.sql.go @@ -109,7 +109,10 @@ SELECT albums.id AS album_id, AND artists.mbid IS NOT NULL GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid, artists.name, artists.mbid -HAVING NOT EXISTS ( +HAVING EXISTS ( + SELECT 1 FROM wants w + WHERE w.album_id = albums.id OR w.release_group_mbid = albums.release_group_mbid) + AND NOT EXISTS ( SELECT 1 FROM wants w WHERE (w.album_id = albums.id OR w.release_group_mbid = albums.release_group_mbid) AND (w.fulfilled_at IS NULL OR w.fulfilled_at > MAX(tracks.missing_since))) @@ -140,6 +143,8 @@ type ListAlbumsDueReacquisitionRow struct { // re-stamps (#2523), so it is a genuine "gone since" clock; // - MusicBrainz can name it: the album and its artist have MBIDs. Albums // failing this are counted separately (CountAlbumsMissingWithoutMbid); +// - it is on the wanted list (#5567): an album taken off the list, or +// dropped into the library by hand, is not fetched back; // - no want already covers it: none open, and none held since its files // went missing. A want held after the loss means the library has a full // copy despite the missing rows (#5245), so asking again would loop. diff --git a/internal/db/dbq/search.sql.go b/internal/db/dbq/search.sql.go index aa30dc94..3b2a75e4 100644 --- a/internal/db/dbq/search.sql.go +++ b/internal/db/dbq/search.sql.go @@ -96,7 +96,7 @@ func (q *Queries) EndGrab(ctx context.Context, arg EndGrabParams) error { const getAcquisitionSettings = `-- name: GetAcquisitionSettings :one -SELECT id, auto_grab, search_interval_hours, allowed_qualities, last_pass_at, last_pass, updated_at, import_root, seed_ratio, seed_days, stall_hours FROM acquisition_settings WHERE id = true +SELECT id, auto_grab, search_interval_hours, allowed_qualities, last_pass_at, last_pass, updated_at, import_root, seed_ratio, seed_days, stall_hours, library_listing_started_at, library_listed_at, library_unlisted FROM acquisition_settings WHERE id = true ` // Searching for wants and grabbing releases (M502 #5521). @@ -115,6 +115,9 @@ func (q *Queries) GetAcquisitionSettings(ctx context.Context) (AcquisitionSettin &i.SeedRatio, &i.SeedDays, &i.StallHours, + &i.LibraryListingStartedAt, + &i.LibraryListedAt, + &i.LibraryUnlisted, ) return i, err } @@ -608,7 +611,7 @@ UPDATE acquisition_settings stall_hours = $7, updated_at = now() WHERE id = true -RETURNING id, auto_grab, search_interval_hours, allowed_qualities, last_pass_at, last_pass, updated_at, import_root, seed_ratio, seed_days, stall_hours +RETURNING id, auto_grab, search_interval_hours, allowed_qualities, last_pass_at, last_pass, updated_at, import_root, seed_ratio, seed_days, stall_hours, library_listing_started_at, library_listed_at, library_unlisted ` type UpdateAcquisitionSettingsParams struct { @@ -644,6 +647,9 @@ func (q *Queries) UpdateAcquisitionSettings(ctx context.Context, arg UpdateAcqui &i.SeedRatio, &i.SeedDays, &i.StallHours, + &i.LibraryListingStartedAt, + &i.LibraryListedAt, + &i.LibraryUnlisted, ) return i, err } diff --git a/internal/db/dbq/wanted.sql.go b/internal/db/dbq/wanted.sql.go index 7e7df749..dacf5e07 100644 --- a/internal/db/dbq/wanted.sql.go +++ b/internal/db/dbq/wanted.sql.go @@ -124,6 +124,33 @@ func (q *Queries) DeleteWant(ctx context.Context, id pgtype.UUID) (int64, error) return result.RowsAffected(), nil } +const finishLibraryListing = `-- name: FinishLibraryListing :one +UPDATE acquisition_settings s + SET library_listed_at = now(), + library_unlisted = ( + SELECT COUNT(*) FROM albums a + WHERE a.created_at <= s.library_listing_started_at + AND NOT EXISTS (SELECT 1 FROM wants w + WHERE w.album_id = a.id + OR (a.release_group_mbid <> '' AND w.release_group_mbid = a.release_group_mbid))) + WHERE s.id = true AND s.library_listed_at IS NULL +RETURNING library_listed_at, library_unlisted +` + +type FinishLibraryListingRow struct { + LibraryListedAt pgtype.Timestamptz + LibraryUnlisted int32 +} + +// The listing is done: count the albums from before the cutoff it could not +// list, because nothing names them to MusicBrainz. +func (q *Queries) FinishLibraryListing(ctx context.Context) (FinishLibraryListingRow, error) { + row := q.db.QueryRow(ctx, finishLibraryListing) + var i FinishLibraryListingRow + err := row.Scan(&i.LibraryListedAt, &i.LibraryUnlisted) + return i, err +} + const fulfillWant = `-- name: FulfillWant :exec UPDATE wants SET fulfilled_at = now(), @@ -199,6 +226,26 @@ func (q *Queries) GetFollow(ctx context.Context, artistMbid string) (GetFollowRo return i, err } +const getLibraryListing = `-- name: GetLibraryListing :one + +SELECT library_listing_started_at, library_listed_at, library_unlisted + FROM acquisition_settings WHERE id = true +` + +type GetLibraryListingRow struct { + LibraryListingStartedAt pgtype.Timestamptz + LibraryListedAt pgtype.Timestamptz + LibraryUnlisted int32 +} + +// The one-time listing of the library (#5567). --------------------------- +func (q *Queries) GetLibraryListing(ctx context.Context) (GetLibraryListingRow, error) { + row := q.db.QueryRow(ctx, getLibraryListing) + var i GetLibraryListingRow + err := row.Scan(&i.LibraryListingStartedAt, &i.LibraryListedAt, &i.LibraryUnlisted) + return i, err +} + const getReleaseTypeProfile = `-- name: GetReleaseTypeProfile :one SELECT id, name, primary_types, secondary_types, is_default, created_at, updated_at FROM release_type_profiles WHERE id = $1 ` @@ -424,6 +471,83 @@ func (q *Queries) ListFollowsDue(ctx context.Context, arg ListFollowsDueParams) return items, nil } +const listLibraryAlbums = `-- name: ListLibraryAlbums :execrows +INSERT INTO wants (release_group_mbid, artist_mbid, artist_name, title, reason, album_id, wanted_at, fulfilled_at) +SELECT DISTINCT ON (a.release_group_mbid) + a.release_group_mbid, ar.mbid, ar.name, a.title, 'library', a.id, now(), + COALESCE((SELECT MIN(t.missing_since) FROM tracks t WHERE t.album_id = a.id) - interval '1 second', now()) + FROM albums a + JOIN artists ar ON ar.id = a.artist_id + WHERE a.release_group_mbid IS NOT NULL AND a.release_group_mbid <> '' + AND ar.mbid IS NOT NULL AND ar.mbid <> '' + AND a.created_at <= $1::timestamptz + AND ($2::uuid IS NULL OR a.id = $2::uuid) + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) + ORDER BY a.release_group_mbid, + (SELECT COUNT(*) FROM tracks t WHERE t.album_id = a.id AND t.missing_since IS NULL) DESC, a.id +ON CONFLICT (release_group_mbid) DO NOTHING +` + +type ListLibraryAlbumsParams struct { + Cutoff pgtype.Timestamptz + AlbumID pgtype.UUID +} + +// Puts library albums on the list as held: each release group once, by the +// album holding the most tracks. Only albums the library had at the cutoff, +// and with album_id given only that album. A group already on the list is +// left as it is. An album with tracks already missing is held from just +// before the first loss, so re-acquisition still fetches it back. +func (q *Queries) ListLibraryAlbums(ctx context.Context, arg ListLibraryAlbumsParams) (int64, error) { + result, err := q.db.Exec(ctx, listLibraryAlbums, arg.Cutoff, arg.AlbumID) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + +const listLibraryAlbumsToName = `-- name: ListLibraryAlbumsToName :many +SELECT a.id, a.title, a.mbid FROM albums a + WHERE a.release_group_mbid IS NULL AND a.mbid IS NOT NULL AND a.mbid <> '' + AND a.created_at <= $1::timestamptz + ORDER BY a.created_at, a.id + LIMIT $2 +` + +type ListLibraryAlbumsToNameParams struct { + Cutoff pgtype.Timestamptz + MaxRows int32 +} + +type ListLibraryAlbumsToNameRow struct { + ID pgtype.UUID + Title string + Mbid *string +} + +// Library albums from before the cutoff that carry a release id but no +// release group yet: the listing asks MusicBrainz for each. ” marks one +// MusicBrainz could not name, so it is asked once. +func (q *Queries) ListLibraryAlbumsToName(ctx context.Context, arg ListLibraryAlbumsToNameParams) ([]ListLibraryAlbumsToNameRow, error) { + rows, err := q.db.Query(ctx, listLibraryAlbumsToName, arg.Cutoff, arg.MaxRows) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListLibraryAlbumsToNameRow + for rows.Next() { + var i ListLibraryAlbumsToNameRow + if err := rows.Scan(&i.ID, &i.Title, &i.Mbid); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listOpenWants = `-- name: ListOpenWants :many SELECT id, release_group_mbid, artist_mbid, artist_name, title, primary_type, first_release_date, reason, album_id, created_by, wanted_at, fulfilled_at, created_at, updated_at, searched_at, search_outcome, search_considered, best_release, best_verdict, search_error FROM wants WHERE fulfilled_at IS NULL @@ -697,6 +821,21 @@ func (q *Queries) RepointWant(ctx context.Context, arg RepointWantParams) error return err } +const setAlbumReleaseGroup = `-- name: SetAlbumReleaseGroup :exec +UPDATE albums SET release_group_mbid = $1 + WHERE id = $2 AND release_group_mbid IS NULL +` + +type SetAlbumReleaseGroupParams struct { + ReleaseGroupMbid *string + ID pgtype.UUID +} + +func (q *Queries) SetAlbumReleaseGroup(ctx context.Context, arg SetAlbumReleaseGroupParams) error { + _, err := q.db.Exec(ctx, setAlbumReleaseGroup, arg.ReleaseGroupMbid, arg.ID) + return err +} + const setDefaultReleaseTypeProfile = `-- name: SetDefaultReleaseTypeProfile :execrows UPDATE release_type_profiles SET is_default = true, updated_at = now() WHERE id = $1 ` @@ -709,6 +848,22 @@ func (q *Queries) SetDefaultReleaseTypeProfile(ctx context.Context, id pgtype.UU return result.RowsAffected(), nil } +const startLibraryListing = `-- name: StartLibraryListing :one +UPDATE acquisition_settings + SET library_listing_started_at = COALESCE(library_listing_started_at, now()) + WHERE id = true +RETURNING library_listing_started_at +` + +// Marks the listing begun, once; its time is the cutoff for which albums +// count as the library's. +func (q *Queries) StartLibraryListing(ctx context.Context) (pgtype.Timestamptz, error) { + row := q.db.QueryRow(ctx, startLibraryListing) + var library_listing_started_at pgtype.Timestamptz + err := row.Scan(&library_listing_started_at) + return library_listing_started_at, err +} + const updateReleaseTypeProfile = `-- name: UpdateReleaseTypeProfile :one UPDATE release_type_profiles SET name = $2, diff --git a/internal/db/migrations/0085_library_wants.down.sql b/internal/db/migrations/0085_library_wants.down.sql new file mode 100644 index 00000000..3fe34736 --- /dev/null +++ b/internal/db/migrations/0085_library_wants.down.sql @@ -0,0 +1,8 @@ +ALTER TABLE acquisition_settings + DROP COLUMN library_listing_started_at, + DROP COLUMN library_listed_at, + DROP COLUMN library_unlisted; + +DELETE FROM wants WHERE reason = 'library'; +ALTER TABLE wants DROP CONSTRAINT wants_reason_check; +ALTER TABLE wants ADD CONSTRAINT wants_reason_check CHECK (reason IN ('request', 'follow', 'missing')); diff --git a/internal/db/migrations/0085_library_wants.up.sql b/internal/db/migrations/0085_library_wants.up.sql new file mode 100644 index 00000000..9cf69406 --- /dev/null +++ b/internal/db/migrations/0085_library_wants.up.sql @@ -0,0 +1,23 @@ +-- The library's albums go on the wanted list as held, once (M502 #5567, +-- operator 2026-10-10). After that the list is the truth: an album taken +-- off it stays off, and one dropped into the library folder by hand is not +-- added. Re-acquisition fetches back only albums on the list. +-- +-- wants.reason library: on the list because the library +-- held it when the list began. +-- acquisition_settings when the one-time listing began (its cutoff: +-- albums added later are not listed), when it +-- finished, and how many albums it could not +-- list because nothing names them to +-- MusicBrainz. +-- +-- The listing itself runs in the wanted worker after boot, not here: it may +-- have to ask MusicBrainz for each album's release group. +ALTER TABLE wants DROP CONSTRAINT wants_reason_check; +ALTER TABLE wants ADD CONSTRAINT wants_reason_check + CHECK (reason IN ('request', 'follow', 'missing', 'library')); + +ALTER TABLE acquisition_settings + ADD COLUMN library_listing_started_at timestamptz, + ADD COLUMN library_listed_at timestamptz, + ADD COLUMN library_unlisted integer NOT NULL DEFAULT 0; diff --git a/internal/db/queries/reacquisition.sql b/internal/db/queries/reacquisition.sql index e03f7169..fbae4a29 100644 --- a/internal/db/queries/reacquisition.sql +++ b/internal/db/queries/reacquisition.sql @@ -23,6 +23,8 @@ RETURNING *; -- re-stamps (#2523), so it is a genuine "gone since" clock; -- * MusicBrainz can name it: the album and its artist have MBIDs. Albums -- failing this are counted separately (CountAlbumsMissingWithoutMbid); +-- * it is on the wanted list (#5567): an album taken off the list, or +-- dropped into the library by hand, is not fetched back; -- * no want already covers it: none open, and none held since its files -- went missing. A want held after the loss means the library has a full -- copy despite the missing rows (#5245), so asking again would loop. @@ -43,7 +45,10 @@ SELECT albums.id AS album_id, AND artists.mbid IS NOT NULL GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid, artists.name, artists.mbid -HAVING NOT EXISTS ( +HAVING EXISTS ( + SELECT 1 FROM wants w + WHERE w.album_id = albums.id OR w.release_group_mbid = albums.release_group_mbid) + AND NOT EXISTS ( SELECT 1 FROM wants w WHERE (w.album_id = albums.id OR w.release_group_mbid = albums.release_group_mbid) AND (w.fulfilled_at IS NULL OR w.fulfilled_at > MAX(tracks.missing_since))) diff --git a/internal/db/queries/wanted.sql b/internal/db/queries/wanted.sql index 5da52d33..394b6d76 100644 --- a/internal/db/queries/wanted.sql +++ b/internal/db/queries/wanted.sql @@ -222,3 +222,66 @@ SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::big (SELECT COUNT(*) FROM release_blocks b WHERE b.expires_at IS NULL OR b.expires_at > now())::bigint AS blocked FROM wants; + +-- The one-time listing of the library (#5567). --------------------------- + +-- name: GetLibraryListing :one +SELECT library_listing_started_at, library_listed_at, library_unlisted + FROM acquisition_settings WHERE id = true; + +-- name: StartLibraryListing :one +-- Marks the listing begun, once; its time is the cutoff for which albums +-- count as the library's. +UPDATE acquisition_settings + SET library_listing_started_at = COALESCE(library_listing_started_at, now()) + WHERE id = true +RETURNING library_listing_started_at; + +-- name: ListLibraryAlbums :execrows +-- Puts library albums on the list as held: each release group once, by the +-- album holding the most tracks. Only albums the library had at the cutoff, +-- and with album_id given only that album. A group already on the list is +-- left as it is. An album with tracks already missing is held from just +-- before the first loss, so re-acquisition still fetches it back. +INSERT INTO wants (release_group_mbid, artist_mbid, artist_name, title, reason, album_id, wanted_at, fulfilled_at) +SELECT DISTINCT ON (a.release_group_mbid) + a.release_group_mbid, ar.mbid, ar.name, a.title, 'library', a.id, now(), + COALESCE((SELECT MIN(t.missing_since) FROM tracks t WHERE t.album_id = a.id) - interval '1 second', now()) + FROM albums a + JOIN artists ar ON ar.id = a.artist_id + WHERE a.release_group_mbid IS NOT NULL AND a.release_group_mbid <> '' + AND ar.mbid IS NOT NULL AND ar.mbid <> '' + AND a.created_at <= sqlc.arg(cutoff)::timestamptz + AND (sqlc.narg(album_id)::uuid IS NULL OR a.id = sqlc.narg(album_id)::uuid) + AND EXISTS (SELECT 1 FROM tracks t WHERE t.album_id = a.id) + ORDER BY a.release_group_mbid, + (SELECT COUNT(*) FROM tracks t WHERE t.album_id = a.id AND t.missing_since IS NULL) DESC, a.id +ON CONFLICT (release_group_mbid) DO NOTHING; + +-- name: ListLibraryAlbumsToName :many +-- Library albums from before the cutoff that carry a release id but no +-- release group yet: the listing asks MusicBrainz for each. '' marks one +-- MusicBrainz could not name, so it is asked once. +SELECT a.id, a.title, a.mbid FROM albums a + WHERE a.release_group_mbid IS NULL AND a.mbid IS NOT NULL AND a.mbid <> '' + AND a.created_at <= sqlc.arg(cutoff)::timestamptz + ORDER BY a.created_at, a.id + LIMIT sqlc.arg(max_rows); + +-- name: SetAlbumReleaseGroup :exec +UPDATE albums SET release_group_mbid = sqlc.arg(release_group_mbid) + WHERE id = sqlc.arg(id) AND release_group_mbid IS NULL; + +-- name: FinishLibraryListing :one +-- The listing is done: count the albums from before the cutoff it could not +-- list, because nothing names them to MusicBrainz. +UPDATE acquisition_settings s + SET library_listed_at = now(), + library_unlisted = ( + SELECT COUNT(*) FROM albums a + WHERE a.created_at <= s.library_listing_started_at + AND NOT EXISTS (SELECT 1 FROM wants w + WHERE w.album_id = a.id + OR (a.release_group_mbid <> '' AND w.release_group_mbid = a.release_group_mbid))) + WHERE s.id = true AND s.library_listed_at IS NULL +RETURNING library_listed_at, library_unlisted; diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index 5da0e417..0a6cf757 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -185,10 +185,13 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) { t.Fatalf("dbtest.ResetDB reset release-type profiles: %v", err) } // Search and grab settings (M502 #5521), a singleton reset the same way. + // The library listing (#5567) reads as done, so a test's albums are not + // put on the wanted list behind its back; its own tests clear it. if _, err := pool.Exec(ctx, ` UPDATE acquisition_settings SET auto_grab = DEFAULT, search_interval_hours = DEFAULT, allowed_qualities = DEFAULT, import_root = DEFAULT, seed_ratio = DEFAULT, seed_days = DEFAULT, stall_hours = DEFAULT, + library_listing_started_at = now(), library_listed_at = now(), library_unlisted = 0, last_pass_at = NULL, last_pass = DEFAULT, updated_at = DEFAULT`, ); err != nil { t.Fatalf("dbtest.ResetDB reset acquisition settings: %v", err) diff --git a/internal/reacquisition/sweeper_test.go b/internal/reacquisition/sweeper_test.go index bb8adaaa..51a974f6 100644 --- a/internal/reacquisition/sweeper_test.go +++ b/internal/reacquisition/sweeper_test.go @@ -69,11 +69,23 @@ func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) p return al.ID } -// #5241: an album is wanted by its 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 and tried again next pass. -func TestSweep_WantsAlbumsByReleaseGroup_Integration(t *testing.T) { +// heldWant puts an album on the wanted list, held since ago. +func heldWant(t *testing.T, pool *pgxpool.Pool, group string, album pgtype.UUID, ago string) { + t.Helper() + if _, err := pool.Exec(context.Background(), ` + INSERT INTO wants (release_group_mbid, artist_mbid, artist_name, title, reason, album_id, fulfilled_at) + VALUES ($1, 'a', 'A', 'T', 'library', $2, now() - $3::interval)`, group, album, ago); err != nil { + t.Fatal(err) + } +} + +// Only an album on the wanted list is fetched back (#5567): one taken off +// it, or dropped into the library by hand, is not. An album on the list is +// wanted by its release group, never its release id (#5241): 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 and +// tried again next pass. +func TestSweep_WantsBackOnlyAlbumsOnTheList_Integration(t *testing.T) { pool := newSweepPool(t) ctx := context.Background() logger := slog.New(slog.NewTextHandler(io.Discard, nil)) @@ -85,7 +97,11 @@ func TestSweep_WantsAlbumsByReleaseGroup_Integration(t *testing.T) { } tagged := lostAlbum(t, pool, "Tagged", "release-tagged", "group-tagged") resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "") - _ = lostAlbum(t, pool, "Unknown", "release-unknown", "") + unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "") + _ = lostAlbum(t, pool, "Off The List", "release-off", "group-off") + heldWant(t, pool, "group-tagged", tagged, "3 days") + heldWant(t, pool, "group-resolved", resolved, "3 days") + heldWant(t, pool, "group-unknown", unknown, "3 days") s := NewSweeper(pool, settings, logger) asked := map[string]int{} @@ -112,8 +128,14 @@ func TestSweep_WantsAlbumsByReleaseGroup_Integration(t *testing.T) { } wantFor("group-tagged", tagged) wantFor("group-resolved", resolved) - if asked["release-tagged"] != 0 { - t.Error("MusicBrainz asked about an album whose tags named its group") + if w, _ := q.GetWantByReleaseGroup(ctx, "group-unknown"); !w.FulfilledAt.Valid { + t.Error("an album MusicBrainz cannot name was reopened under the wrong group") + } + if _, err := q.GetWantByReleaseGroup(ctx, "group-off"); err == nil { + t.Error("an album not on the list was wanted back") + } + if asked["release-tagged"] != 0 || asked["release-off"] != 0 { + t.Errorf("MusicBrainz asked %v: not about a tagged album, nor one off the list", asked) } var cached *string if err := pool.QueryRow(ctx, "SELECT release_group_mbid FROM albums WHERE id = $1", resolved).Scan(&cached); err != nil { @@ -122,13 +144,6 @@ func TestSweep_WantsAlbumsByReleaseGroup_Integration(t *testing.T) { if cached == nil || *cached != "group-resolved" { t.Errorf("resolved group not cached on the album: %v", cached) } - var n int - if err := pool.QueryRow(ctx, "SELECT count(*) FROM wants").Scan(&n); err != nil { - t.Fatal(err) - } - if n != 2 { - t.Errorf("%d wants, want 2: the unresolvable album is skipped", n) - } // The next pass finds both already wanted and asks nothing new. asked = map[string]int{} diff --git a/internal/tags/provider_musicbrainz.go b/internal/tags/provider_musicbrainz.go index c60b6e0f..14aa424b 100644 --- a/internal/tags/provider_musicbrainz.go +++ b/internal/tags/provider_musicbrainz.go @@ -3,6 +3,7 @@ package tags import ( "context" "errors" + "fmt" "net/http" "net/url" "sync/atomic" @@ -172,15 +173,20 @@ func (p *musicbrainzProvider) TestConnection(ctx context.Context) error { func ReleaseGroupForRelease(ctx context.Context, releaseMBID string) (string, error) { p, err := ProviderByID("musicbrainz") if err != nil { - return "", ErrNotFound + return "", ErrSwitchedOff } mb, ok := p.(*musicbrainzProvider) if !ok { - return "", ErrNotFound + return "", ErrSwitchedOff } return mb.releaseGroupForRelease(ctx, releaseMBID) } +// ErrSwitchedOff is ReleaseGroupForRelease's answer while MusicBrainz is +// switched off. It is an ErrNotFound, so callers that only skip still do; a +// caller that would record "MusicBrainz has none" can tell it apart. +var ErrSwitchedOff = fmt.Errorf("%w: MusicBrainz is switched off", ErrNotFound) + // mbReleaseResponse models the subset of /ws/2/release/{mbid}?inc=release-groups // we read. type mbReleaseResponse struct { @@ -190,7 +196,10 @@ type mbReleaseResponse struct { } func (p *musicbrainzProvider) releaseGroupForRelease(ctx context.Context, releaseMBID string) (string, error) { - if !p.enabled.Load() || releaseMBID == "" { + if !p.enabled.Load() { + return "", ErrSwitchedOff + } + if releaseMBID == "" { return "", ErrNotFound } q := url.Values{"inc": {"release-groups"}, "fmt": {"json"}} diff --git a/internal/wanted/library.go b/internal/wanted/library.go new file mode 100644 index 00000000..cefadbc0 --- /dev/null +++ b/internal/wanted/library.go @@ -0,0 +1,117 @@ +package wanted + +import ( + "context" + "errors" + "fmt" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/tags" +) + +// ReasonLibrary: on the list because the library held it when the list +// began (#5567). +const ReasonLibrary = "library" + +// libraryNamesPerPass bounds the MusicBrainz lookups one pass makes for +// albums whose tags named a release but not its group: about two minutes at +// MusicBrainz's one request a second, so the pass still settles its wants. +const libraryNamesPerPass = 120 + +// listLibrary puts the library's albums on the wanted list as held, once +// (operator, 2026-10-10: "make a record saying that the import has +// happened so later changes like unwanting hold and dont auto import from +// the file directory"). It runs a step each pass until done: +// +// - the first step stamps when the listing began, which is its cutoff: +// an album the library gains afterwards, dropped in by hand, is never +// listed; +// - every album whose release group is known goes on the list at once; +// - albums that name a release but not its group are looked up on +// MusicBrainz, a bounded number a pass, and listed as each is named; +// - once none is left to name, the listing is stamped done, with a count +// of the albums nothing names to MusicBrainz. +// +// After that it never runs again, so an album taken off the list stays off. +func (s *Service) listLibrary(ctx context.Context, res *PassResult) error { + q := dbq.New(s.pool) + state, err := q.GetLibraryListing(ctx) + if err != nil { + return fmt.Errorf("wanted: read the library listing: %w", err) + } + if state.LibraryListedAt.Valid { + return nil + } + cutoff := state.LibraryListingStartedAt + if !cutoff.Valid { + if cutoff, err = q.StartLibraryListing(ctx); err != nil { + return fmt.Errorf("wanted: start the library listing: %w", err) + } + n, err := q.ListLibraryAlbums(ctx, dbq.ListLibraryAlbumsParams{Cutoff: cutoff}) + if err != nil { + return fmt.Errorf("wanted: list the library: %w", err) + } + res.Listed += int(n) + s.logger.Info("wanted: listing the library's albums as held", "listed", n) + } + + for { + albums, err := q.ListLibraryAlbumsToName(ctx, dbq.ListLibraryAlbumsToNameParams{Cutoff: cutoff, MaxRows: libraryNamesPerPass}) + if err != nil { + return fmt.Errorf("wanted: albums to name: %w", err) + } + if len(albums) == 0 { + break + } + for _, a := range albums { + if ctx.Err() != nil { + return ctx.Err() + } + group, err := s.releaseGroup(ctx, deref(a.Mbid)) + switch { + case errors.Is(err, tags.ErrSwitchedOff): + // Nothing can be named while MusicBrainz is off: the rest are + // counted as unlisted rather than left waiting for ever. + return s.finishLibrary(ctx, q, res) + case errors.Is(err, tags.ErrNotFound): + group = "" // MusicBrainz has none: asked once + case err != nil: + // Unreachable or refusing for now: the next pass goes on. + s.logger.Info("wanted: naming the library's albums paused", "err", err) + return nil + } + if err := q.SetAlbumReleaseGroup(ctx, dbq.SetAlbumReleaseGroupParams{ID: a.ID, ReleaseGroupMbid: &group}); err != nil { + return fmt.Errorf("wanted: name album %s: %w", a.Title, err) + } + if group == "" { + continue + } + n, err := q.ListLibraryAlbums(ctx, dbq.ListLibraryAlbumsParams{Cutoff: cutoff, AlbumID: a.ID}) + if err != nil { + return fmt.Errorf("wanted: list album %s: %w", a.Title, err) + } + res.Listed += int(n) + } + if len(albums) == libraryNamesPerPass { + return nil // the rest next pass + } + } + return s.finishLibrary(ctx, q, res) +} + +func (s *Service) finishLibrary(ctx context.Context, q *dbq.Queries, res *PassResult) error { + done, err := q.FinishLibraryListing(ctx) + if err != nil { + return fmt.Errorf("wanted: finish the library listing: %w", err) + } + s.logger.Info("wanted: the library's albums are on the list", "listed_this_pass", res.Listed, + "unlisted", done.LibraryUnlisted) + return nil +} + +func deref(p *string) string { + if p == nil { + return "" + } + return *p +} diff --git a/internal/wanted/wanted.go b/internal/wanted/wanted.go index 0d430b52..6f21ec0c 100644 --- a/internal/wanted/wanted.go +++ b/internal/wanted/wanted.go @@ -2,7 +2,9 @@ // artists, each on a release-type profile, and wanted release groups. A want // comes from an approved request, a followed artist's new release, or an // album whose files went missing; it is held once the library has a full -// copy. Searching for open wants is #5521's. +// copy. The library's own albums were put on the list as held once (#5567), +// so the list is the record of what the library keeps. Searching for open +// wants is #5521's. package wanted import ( @@ -172,9 +174,9 @@ func (s *Service) Run(ctx context.Context) { switch { case err != nil && ctx.Err() == nil: s.logger.Warn("wanted: pass stopped", "err", err, "follows", res.Follows, "wanted", res.Wanted, "held", res.Held) - case res.Wanted > 0 || res.Held > 0 || res.Baselined > 0: + case res.Wanted > 0 || res.Held > 0 || res.Baselined > 0 || res.Listed > 0: s.logger.Info("wanted: pass complete", "follows", res.Follows, "baselined", res.Baselined, - "wanted", res.Wanted, "held", res.Held) + "wanted", res.Wanted, "held", res.Held, "listed", res.Listed) } select { case <-ctx.Done(): @@ -190,6 +192,7 @@ type PassResult struct { Baselined int // of those, first reads Wanted int // wants recorded from follows Held int // wants the library now answers + Listed int // library albums put on the list as held (#5567) } // Pass applies the follows due and settles open wants. An unreachable @@ -201,6 +204,11 @@ func (s *Service) Pass(ctx context.Context) (PassResult, error) { if ctx.Err() != nil { return res, ctx.Err() } + // The one-time listing of the library (#5567); a failure waits for the + // next pass and does not hold back settling. + if err := s.listLibrary(ctx, &res); err != nil && ctx.Err() == nil { + s.logger.Warn("wanted: list the library", "err", err) + } if err := s.settleWants(ctx, &res); err != nil { return res, err } diff --git a/internal/wanted/wanted_test.go b/internal/wanted/wanted_test.go index 7ede0c7c..0b7ecd09 100644 --- a/internal/wanted/wanted_test.go +++ b/internal/wanted/wanted_test.go @@ -455,3 +455,135 @@ func seedUser(t *testing.T, pool *pgxpool.Pool) pgtype.UUID { } return u.ID } + +// startListing clears the reset's "already listed", as a fresh install has it. +func startListing(t *testing.T, pool *pgxpool.Pool) { + t.Helper() + if _, err := pool.Exec(context.Background(), ` + UPDATE acquisition_settings SET library_listing_started_at = NULL, library_listed_at = NULL, library_unlisted = 0`); err != nil { + t.Fatal(err) + } +} + +// The library's albums go on the list as held, once (#5567). An album the +// tags name by release only is named through MusicBrainz first; one nothing +// names is counted, not listed. Taken off the list, an album stays off, and +// one added to the library afterwards is never listed. +func TestListLibrary_OnceAndThenTheListIsTheTruth_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + s := newService(t, pool, newFakeMB(t)) + s.releaseGroup = func(_ context.Context, release string) (string, error) { + if release == "rel-parklife" { + return "rg-parklife", nil + } + return "", tags.ErrNotFound + } + blurAlbum := seedHeld(t, q, "Blur", "rel-blur", "rg-blur", 3) + parklife := seedHeld(t, q, "Parklife", "rel-parklife", "", 2) + _ = seedHeld(t, q, "Bootleg", "rel-bootleg", "", 1) + // An album with a track gone: held from before the loss, so + // re-acquisition still fetches it back. + lost := seedHeld(t, q, "13", "rel-13", "rg-13", 2) + if _, err := pool.Exec(ctx, `UPDATE tracks SET missing_since = now() - interval '1 day' + WHERE album_id = $1 AND track_number = 1`, lost.ID); err != nil { + t.Fatal(err) + } + startListing(t, pool) + + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + for group, album := range map[string]pgtype.UUID{"rg-blur": blurAlbum.ID, "rg-parklife": parklife.ID, "rg-13": lost.ID} { + w, err := q.GetWantByReleaseGroup(ctx, group) + if err != nil { + t.Fatalf("%s not listed: %v", group, err) + } + if w.Reason != ReasonLibrary || !w.FulfilledAt.Valid || w.AlbumID != album { + t.Errorf("%s = %+v: a held library want about the album", group, w) + } + } + if w, _ := q.GetWantByReleaseGroup(ctx, "rg-13"); !w.FulfilledAt.Time.Before(time.Now().Add(-23 * time.Hour)) { + t.Errorf("an album missing a track is held from %v, not from before the loss", w.FulfilledAt.Time) + } + state, err := q.GetLibraryListing(ctx) + if err != nil || !state.LibraryListedAt.Valid || state.LibraryUnlisted != 1 { + t.Fatalf("listing = %+v %v: done, with the bootleg unlisted", state, err) + } + + // Off the list stays off; an album added later is never listed. + w, _ := q.GetWantByReleaseGroup(ctx, "rg-blur") + if _, err := s.RemoveWant(ctx, w.ID); err != nil { + t.Fatal(err) + } + _ = seedHeld(t, q, "Think Tank", "rel-think", "rg-think", 2) + if _, err := s.Pass(ctx); err != nil { + t.Fatal(err) + } + for _, group := range []string{"rg-blur", "rg-think"} { + if _, err := q.GetWantByReleaseGroup(ctx, group); err == nil { + t.Errorf("%s is on the list", group) + } + } +} + +// While MusicBrainz is switched off nothing can be named, so the listing +// finishes with what it has rather than waiting for ever. +func TestListLibrary_MusicBrainzOffFinishesWithWhatIsNamed_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + s := newService(t, pool, newFakeMB(t)) + s.releaseGroup = func(context.Context, string) (string, error) { return "", tags.ErrSwitchedOff } + _ = seedHeld(t, q, "Blur", "rel-blur", "rg-blur", 1) + _ = seedHeld(t, q, "Parklife", "rel-parklife", "", 1) + startListing(t, pool) + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + state, _ := q.GetLibraryListing(ctx) + if !state.LibraryListedAt.Valid || state.LibraryUnlisted != 1 { + t.Errorf("listing = %+v", state) + } + var named *string + if err := pool.QueryRow(ctx, `SELECT release_group_mbid FROM albums WHERE title = 'Parklife'`).Scan(&named); err != nil { + t.Fatal(err) + } + if named != nil { + t.Errorf("an album was marked unnameable while MusicBrainz was off: %q", *named) + } +} + +// A MusicBrainz that does not answer pauses the naming; the next pass goes on. +func TestListLibrary_UnreachablePausesTheNaming_Integration(t *testing.T) { + pool := newPool(t) + ctx := context.Background() + q := dbq.New(pool) + s := newService(t, pool, newFakeMB(t)) + down := true + s.releaseGroup = func(context.Context, string) (string, error) { + if down { + return "", tags.ErrTransient + } + return "rg-parklife", nil + } + _ = seedHeld(t, q, "Parklife", "rel-parklife", "", 1) + startListing(t, pool) + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + if state, _ := q.GetLibraryListing(ctx); state.LibraryListedAt.Valid { + t.Fatal("finished while an album was still to be named") + } + down = false + if err := s.listLibrary(ctx, &PassResult{}); err != nil { + t.Fatal(err) + } + if _, err := q.GetWantByReleaseGroup(ctx, "rg-parklife"); err != nil { + t.Errorf("not listed once named: %v", err) + } + if state, _ := q.GetLibraryListing(ctx); !state.LibraryListedAt.Valid { + t.Error("not finished") + } +} diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 738e16e6..fe1e4953 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -569,7 +569,8 @@ export type ResolvedDuplicatesResponse = { // Wanted list (M502 #5520) ------------------------------------------------ -export type WantReason = 'request' | 'follow' | 'missing'; +// library: in the library when the list began (#5567). +export type WantReason = 'request' | 'follow' | 'missing' | 'library'; export type WantState = 'wanted' | 'missing' | 'held'; // One release group on the wanted list. held_at is set once the library @@ -811,7 +812,12 @@ export type ReleaseBlock = { export type ReleaseBlocksPage = { items: ReleaseBlock[]; limit: number; offset: number }; -export type WantsPage = Page & { counts: WantCounts }; +// The one-time listing of the library's albums as held (#5567): when it +// finished (null while it runs), and how many albums nothing names to +// MusicBrainz, so they could not be listed. +export type LibraryListing = { listed_at: string | null; unlisted: number }; + +export type WantsPage = Page & { counts: WantCounts; library: LibraryListing }; export type Follow = { artist_mbid: string; diff --git a/web/src/lib/components/wanted/WantList.svelte b/web/src/lib/components/wanted/WantList.svelte index e996ff48..ee953bb5 100644 --- a/web/src/lib/components/wanted/WantList.svelte +++ b/web/src/lib/components/wanted/WantList.svelte @@ -13,7 +13,9 @@ // One tab of the Wanted view (M502 #5520–#5522): the albums still wanted, // the missing albums wanted back, or the ones held. Each open want says // what its last search found and how its download is getting on; Releases - // lists every candidate with its verdict, and Search runs one now. + // lists every candidate with its verdict, and Search runs one now. The + // library's albums were put on the list as held once (#5567); taking one + // off leaves its files and stops it being fetched back if they go. let { list }: { list: WantState } = $props(); @@ -21,6 +23,7 @@ const wantsStore = $derived(createWantsQuery(list)); const wants = $derived($wantsStore); const items = $derived((wants.data?.pages ?? []).flatMap((p) => p.items) as Want[]); + const library = $derived(wants.data?.pages[0]?.library); let busy = $state(null); let searching = $state(null); @@ -30,7 +33,11 @@ busy = w.id; try { await removeWant(w.id); - pushToast(`${w.title} is off the wanted list.`); + pushToast( + w.held_at + ? `${w.title} is off the list. Its files stay, and it won't be fetched back if they go missing.` + : `${w.title} is off the wanted list.` + ); await client.invalidateQueries({ queryKey: ['adminWants'] }); } catch (e) { pushToast(errMessage(e), 'error'); @@ -137,6 +144,7 @@ function why(w: Want): string { if (w.reason === 'request') return w.requested_by ? `Requested by ${w.requested_by}` : 'Requested'; if (w.reason === 'follow') return 'New from a followed artist'; + if (w.reason === 'library') return 'In your library when the list began'; return 'Its files went missing'; } @@ -151,6 +159,22 @@ } +{#if list === 'held' && library} + {#if !library.listed_at} +

+ Your library's albums are being added to this list as held. Albums whose tags name only a + release are looked up on MusicBrainz first, about one a second. +

+ {:else if library.unlisted > 0} +

+ {library.unlisted === 1 ? '1 album' : `${library.unlisted} albums`} in your library + {library.unlisted === 1 ? "isn't" : "aren't"} on this list: nothing in {library.unlisted === 1 ? 'its' : 'their'} + tags names {library.unlisted === 1 ? 'it' : 'them'} to MusicBrainz. Those are never fetched back if + their files go missing. +

+ {/if} +{/if} + {#if wants.isPending}

Reading the wanted list…

{:else if wants.isError && !wants.isFetchNextPageError} @@ -216,6 +240,17 @@
{arrived(w)}
{/if} + {#if w.held_at} + + {/if} {#if !w.held_at}