From c54792c51e5d99d0b92c3de87f092a22a365ae6b Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 11 Oct 2026 12:24:59 -0400 Subject: [PATCH] feat: a held album below the quality you settle for, or holding a video rip, stays wanted for a better copy, and the import replaces the old tracks with their likes and plays carried over (M502 #5524) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - acquisition_settings.upgrade_until (default High quality lossy, Lidarr's cutoff, note #5527; "" is off), an "Upgrade until" row under Searching. - The wanted worker judges held albums hourly: tier per format from size per second (the scanner records no bitrate), the album's worst. Below the cutoff it is wanted for quality; a video rip among its tracks always. - The schedule searches upgrades weekly, after open wants; a release no better than the copy held is refused ("not_better"). - The import matches each file to the track it would replace (recording, then disc/track) and replaces it when it is a rip or the file is a better tier: over the same path in one rename, or filed beside it and the old track folded in (library.ReplaceTrack: file first, then foldTrackInto). Nothing better: the release is set aside. - Collection › Upgrades (Lidarr's Cutoff Unmet); Suspect sources says when a clean copy was looked for instead of Lidarr's search. Co-Authored-By: Claude Opus 5.5 --- cmd/minstrel/main.go | 4 +- internal/acquire/search.go | 24 +- internal/acquire/search_test.go | 24 +- internal/api/admin_search.go | 12 +- internal/api/admin_suspect_sources.go | 8 +- internal/api/admin_wanted.go | 19 +- internal/db/dbq/imports.sql.go | 89 ++++- internal/db/dbq/models.go | 5 + internal/db/dbq/reacquisition.sql.go | 6 +- internal/db/dbq/search.sql.go | 43 ++- internal/db/dbq/tracks.sql.go | 15 +- internal/db/dbq/wanted.sql.go | 140 +++++++- internal/db/migrations/0090_upgrades.down.sql | 7 + internal/db/migrations/0090_upgrades.up.sql | 29 ++ internal/db/queries/imports.sql | 23 ++ internal/db/queries/search.sql | 21 +- internal/db/queries/tracks.sql | 13 +- internal/db/queries/wanted.sql | 46 +++ internal/dbtest/reset.go | 2 +- internal/importer/importer.go | 14 +- internal/importer/place.go | 16 +- internal/importer/upgrade.go | 304 ++++++++++++++++++ internal/importer/upgrade_test.go | 53 +++ internal/importer/verify.go | 7 + internal/library/replace.go | 83 +++++ internal/release/quality.go | 102 ++++++ internal/release/release_test.go | 58 ++++ internal/wanted/search.go | 44 ++- internal/wanted/upgrade.go | 125 +++++++ internal/wanted/upgrade_test.go | 137 ++++++++ internal/wanted/wanted.go | 13 +- web/src/lib/api/admin.suspect-sources.test.ts | 1 + web/src/lib/api/types.ts | 18 +- web/src/lib/api/wanted.ts | 1 + .../lib/components/SearchSettingsCard.svelte | 28 +- .../lib/components/SearchSettingsCard.test.ts | 22 +- .../components/wanted/CatalogueGrid.test.ts | 2 +- .../components/wanted/CollectionRail.svelte | 4 +- web/src/lib/components/wanted/WantList.svelte | 27 +- .../lib/components/wanted/WantList.test.ts | 30 +- web/src/lib/styles/error-copy.json | 5 +- .../routes/admin/suspect-sources/+page.svelte | 26 +- .../suspect-sources/suspect-sources.test.ts | 12 +- web/src/routes/collection/collection.test.ts | 3 +- .../routes/collection/upgrades/+page.svelte | 8 + 45 files changed, 1574 insertions(+), 99 deletions(-) create mode 100644 internal/db/migrations/0090_upgrades.down.sql create mode 100644 internal/db/migrations/0090_upgrades.up.sql create mode 100644 internal/importer/upgrade.go create mode 100644 internal/importer/upgrade_test.go create mode 100644 internal/library/replace.go create mode 100644 internal/wanted/upgrade.go create mode 100644 internal/wanted/upgrade_test.go create mode 100644 web/src/routes/collection/upgrades/+page.svelte diff --git a/cmd/minstrel/main.go b/cmd/minstrel/main.go index b8add60c..e8b4e4d7 100644 --- a/cmd/minstrel/main.go +++ b/cmd/minstrel/main.go @@ -340,7 +340,9 @@ func run() error { if acoustIDSettings != nil { printer = &importer.AcoustIDPrinter{Settings: acoustIDSettings, Client: acoustIDClient} } - go importer.New(pool, scanner, cat, printer, cfg.Library.ScanPaths, logger.With("component", "import")).Run(ctx) + imp := importer.New(pool, scanner, cat, printer, cfg.Library.ScanPaths, logger.With("component", "import")) + imp.DataDir = cfg.Storage.DataDir + go imp.Run(ctx) // Download lifecycle (M502 #5523): every ten minutes, a download that // failed or that nobody shares is retried, then given up and its album diff --git a/internal/acquire/search.go b/internal/acquire/search.go index d3edf564..0610b04d 100644 --- a/internal/acquire/search.go +++ b/internal/acquire/search.go @@ -50,6 +50,9 @@ const ( CodeProfile = "profile" CodeNoSeeders = "no_seeders" CodeDoubt = "doubt" + // CodeNotBetter: the album, but no better than the copy the library + // holds, for a want that is an upgrade (#5524). + CodeNotBetter = "not_better" ) // Reason is one thing a verdict rests on. @@ -64,6 +67,12 @@ type Reason struct { type Want struct { ID pgtype.UUID release.Want + // Floor is the tier of the copy the library holds, for an album wanted + // for a better one (#5524): a release must rank above it. "" for an + // album not held, and for a video rip, which any clean copy beats. + Floor release.Tier + // Held says the floor in words: "MP3, about 192 kbps". + Held string } // Candidate is one result with everything decided about it. @@ -188,11 +197,11 @@ func (s *Service) For(ctx context.Context, want Want) Result { blocked := s.Memory.blocked(ctx) for i := range cands { - decide(&cands[i], want.Want, s.Profile, blocked) + decide(&cands[i], want, s.Profile, blocked) } sortCandidates(cands) for _, i := range s.peek(ctx, cands) { - decide(&cands[i], want.Want, s.Profile, blocked) + decide(&cands[i], want, s.Profile, blocked) r.Looked++ } sortCandidates(cands) @@ -209,7 +218,8 @@ func (s *Service) For(ctx context.Context, want Want) Result { // whether it is the album, then whether the profile takes its quality, then // whether anything leaves it to a person. A quality refusal said about the // wrong album would be noise, so identity comes first. -func decide(c *Candidate, w release.Want, profile release.Profile, blocked map[string]string) { +func decide(c *Candidate, want Want, profile release.Profile, blocked map[string]string) { + w := want.Want c.Verdict, c.Reason, c.Reasons = "", "", nil if why, ok := blocked[c.InfoHash]; ok && c.InfoHash != "" { set(c, Blocked, Hard, CodeBlocked, why) @@ -240,6 +250,14 @@ func decide(c *Candidate, w release.Want, profile release.Profile, blocked map[s set(c, Refused, Soft, CodeProfile, "Your quality settings don’t take "+strings.ToLower(release.Label(c.Tier))+".") return } + if want.Floor != "" && release.Rank(c.Tier) <= release.Rank(want.Floor) { + held := want.Held + if held == "" { + held = strings.ToLower(release.Label(want.Floor)) + } + set(c, Refused, Soft, CodeNotBetter, "No better than the copy you have ("+held+").") + return + } if doubt != "" { set(c, Review, Soft, CodeDoubt, doubt) return diff --git a/internal/acquire/search_test.go b/internal/acquire/search_test.go index f158b345..b074e694 100644 --- a/internal/acquire/search_test.go +++ b/internal/acquire/search_test.go @@ -95,7 +95,7 @@ func TestFor_ProfileSeedersAndDoubt(t *testing.T) { func TestFor_BlockedReleasesAreSetAside(t *testing.T) { c := Candidate{Release: release.Parse("Daft Punk - Random Access Memories [FLAC]"), InfoHash: "aaaa", Seeders: 3} - decide(&c, ram.Want, anyQuality, map[string]string{"aaaa": "You set this release aside."}) + decide(&c, ram, anyQuality, map[string]string{"aaaa": "You set this release aside."}) if c.Verdict != Blocked || c.Reason != "You set this release aside." { t.Errorf("verdict %s %q", c.Verdict, c.Reason) } @@ -239,3 +239,25 @@ func TestLandingFor_OneFolderPerTorrent(t *testing.T) { t.Error(got) } } + +func TestDecide_AnUpgradeTakesOnlyABetterCopy(t *testing.T) { + up := ram + up.Floor, up.Held = release.TierLossyLow, "MP3, about 192 kbps" + same := Candidate{Release: release.Parse("Daft Punk - Random Access Memories [MP3 192]"), InfoHash: "aaaa", Seeders: 3} + decide(&same, up, anyQuality, nil) + if same.Verdict != Refused || same.Reasons[0].Code != CodeNotBetter || + same.Reason != "No better than the copy you have (MP3, about 192 kbps)." { + t.Errorf("same quality: %s %q", same.Verdict, same.Reason) + } + flac := Candidate{Release: release.Parse("Daft Punk - Random Access Memories [FLAC]"), InfoHash: "bbbb", Seeders: 3} + decide(&flac, up, anyQuality, nil) + if flac.Verdict != Grab { + t.Errorf("a FLAC over a 192 MP3: %s %q", flac.Verdict, flac.Reason) + } + // A video rip has no floor: any clean copy is better. + rip := ram + decide(&same, rip, anyQuality, nil) + if same.Verdict != Grab { + t.Errorf("no floor: %s %q", same.Verdict, same.Reason) + } +} diff --git a/internal/api/admin_search.go b/internal/api/admin_search.go index 35540523..cc843ddd 100644 --- a/internal/api/admin_search.go +++ b/internal/api/admin_search.go @@ -43,6 +43,9 @@ type searchSettingsView struct { SeedRatio float32 `json:"seed_ratio"` SeedDays int32 `json:"seed_days"` StallHours int32 `json:"stall_hours"` + // UpgradeUntil is the tier a held album is upgraded to (#5524); "" is + // no quality upgrades. + UpgradeUntil string `json:"upgrade_until"` } func searchSettings(s dbq.AcquisitionSetting, roots []string) searchSettingsView { @@ -51,6 +54,7 @@ func searchSettings(s dbq.AcquisitionSetting, roots []string) searchSettingsView AllowedQualities: s.AllowedQualities, Tiers: release.Tiers, LastPass: s.LastPass, ImportRoot: s.ImportRoot, ScanPaths: roots, SeedRatio: s.SeedRatio, SeedDays: s.SeedDays, StallHours: s.StallHours, + UpgradeUntil: s.UpgradeUntil, } if v.ScanPaths == nil { v.ScanPaths = []string{} @@ -86,6 +90,7 @@ type putSearchSettingsBody struct { SeedRatio float32 `json:"seed_ratio"` SeedDays int32 `json:"seed_days"` StallHours int32 `json:"stall_hours"` + UpgradeUntil string `json:"upgrade_until"` } // handlePutSearchSettings implements PUT /api/admin/acquisition/settings. @@ -108,6 +113,10 @@ func (h *handlers) handlePutSearchSettings(w http.ResponseWriter, r *http.Reques return } } + if body.UpgradeUntil != "" && !release.KnownTier(body.UpgradeUntil) { + writeErr(w, apierror.BadRequest("bad_upgrade_until", "unknown quality "+body.UpgradeUntil)) + return + } if body.ImportRoot != "" && !slices.Contains(h.importer.Roots, body.ImportRoot) { writeErr(w, apierror.BadRequest("bad_import_root", "file new music into one of the library's folders")) return @@ -121,6 +130,7 @@ func (h *handlers) handlePutSearchSettings(w http.ResponseWriter, r *http.Reques s, err := dbq.New(h.pool).UpdateAcquisitionSettings(r.Context(), dbq.UpdateAcquisitionSettingsParams{ AutoGrab: body.AutoGrab, SearchIntervalHours: body.SearchIntervalHours, AllowedQualities: body.AllowedQualities, ImportRoot: body.ImportRoot, SeedRatio: body.SeedRatio, SeedDays: body.SeedDays, StallHours: body.StallHours, + UpgradeUntil: body.UpgradeUntil, }) if err != nil { writeErrWithLog(w, h.logger, "admin search settings: put", apierror.Internal(err)) @@ -322,7 +332,7 @@ func (h *handlers) writeSearchErr(w http.ResponseWriter, err error) bool { case errors.Is(err, pgx.ErrNoRows): writeErr(w, apierror.NotFound("want")) case errors.Is(err, wanted.ErrWantHeld): - writeErr(w, apierror.Conflict("want_held", "the library already holds this album")) + writeErr(w, apierror.Conflict("want_held", "the library already holds this album, and it isn't wanted for a better copy")) default: writeErrWithLog(w, h.logger, "admin wanted: search", apierror.Internal(err)) } diff --git a/internal/api/admin_suspect_sources.go b/internal/api/admin_suspect_sources.go index 6161d0c4..30125395 100644 --- a/internal/api/admin_suspect_sources.go +++ b/internal/api/admin_suspect_sources.go @@ -31,8 +31,9 @@ type suspectTrackView struct { // first pass (M498 #5439). VerdictAt is when it was last set. Verdict *string `json:"verdict"` VerdictAt *string `json:"verdict_at"` - // SearchedAt is when the resolver last asked Lidarr to search the album for - // a better copy (#5447): null when it has not. + // Replacing: the album is on the wanted list for a clean copy (#5524); + // SearchedAt is when one was last searched for, null before the first. + Replacing bool `json:"replacing"` SearchedAt *string `json:"searched_at"` } @@ -115,7 +116,8 @@ func groupSuspectByDirectory(rows []dbq.ListSuspectSourceTracksRow) []suspectGro at := row.SourceVerdictAt.Time.UTC().Format(time.RFC3339) t.VerdictAt = &at } - if row.SearchedAt.Valid { + t.Replacing = row.WantUpgrade == "rip" + if t.Replacing && row.SearchedAt.Valid { at := row.SearchedAt.Time.UTC().Format(time.RFC3339) t.SearchedAt = &at } diff --git a/internal/api/admin_wanted.go b/internal/api/admin_wanted.go index 3ff14ad7..7dab1834 100644 --- a/internal/api/admin_wanted.go +++ b/internal/api/admin_wanted.go @@ -39,6 +39,10 @@ type wantView struct { WantedAt string `json:"wanted_at"` HeldAt *string `json:"held_at"` ImageURL string `json:"image_url"` + // Upgrade: why a held album is wanted again, quality or rip, and + // HeldQuality what the library's copy is (#5524). + Upgrade string `json:"upgrade,omitempty"` + HeldQuality string `json:"held_quality,omitempty"` // Search is the schedule's last look; Grab the download running (#5521). Search *wantSearchView `json:"search"` Grab *wantGrabView `json:"grab"` @@ -80,6 +84,8 @@ type wantCounts struct { Wanted int64 `json:"wanted"` Missing int64 `json:"missing"` Held int64 `json:"held"` + // Upgrades: held albums wanted for a better copy (#5524). + Upgrades int64 `json:"upgrades"` Following int64 `json:"following"` // Activity: downloads running; Attention: of those, the ones waiting on // a person; Blocked: releases set aside (#5522). @@ -173,7 +179,7 @@ func (h *handlers) wantsHeader(r *http.Request, q *dbq.Queries) (wantCounts, lib lib.ListedAt = &at } return wantCounts{Artists: counts.Artists, Unmatched: counts.Unmatched, Wanted: counts.Wanted, Missing: counts.Missing, Held: counts.Held, - Following: counts.Following, Activity: counts.Activity, Attention: counts.Attention, Blocked: counts.Blocked}, lib, nil + Upgrades: counts.Upgrades, Following: counts.Following, Activity: counts.Activity, Attention: counts.Attention, Blocked: counts.Blocked}, lib, nil } func offsetParam(r *http.Request) int { @@ -220,8 +226,9 @@ func (h *handlers) handleListWantArtists(w http.ResponseWriter, r *http.Request) } // handleListWants implements -// GET /api/admin/wanted?state=wanted|missing|held|all&artist=&q=&offset=. -// wanted is everything open but missing files. Each list runs artist by +// GET /api/admin/wanted?state=wanted|missing|upgrade|held|all&artist=&q=&offset=. +// wanted is everything open but missing files; upgrade the held albums +// wanted for a better copy (#5524). Each list runs artist by // artist, A–Z, like the library (#5572); q narrows it to an artist or // album, and artist to one artist's page, which it then heads. func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { @@ -229,8 +236,8 @@ func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { if state == "" { state = "wanted" } - if state != "wanted" && state != "missing" && state != "held" && state != "all" { - writeErr(w, apierror.BadRequest("bad_state", "state must be wanted, missing, held or all")) + if state != "wanted" && state != "missing" && state != "upgrade" && state != "held" && state != "all" { + writeErr(w, apierror.BadRequest("bad_state", "state must be wanted, missing, upgrade, held or all")) return } offset := offsetParam(r) @@ -269,7 +276,7 @@ func (h *handlers) handleListWants(w http.ResponseWriter, r *http.Request) { ArtistMBID: row.ArtistMbid, ArtistName: row.ArtistName, PrimaryType: row.PrimaryType, SecondaryTypes: row.SecondaryTypes, Year: first4(row.FirstReleaseDate), Reason: row.Reason, - WantedAt: formatTimestamp(row.WantedAt), + WantedAt: formatTimestamp(row.WantedAt), Upgrade: row.Upgrade, HeldQuality: row.HeldQuality, ImageURL: groupArtURL(musicbrainz.ReleaseGroup{MBID: row.ReleaseGroupMbid, Title: row.Title, ArtistCredit: row.ArtistName}), } if row.LibraryArtistID.Valid { diff --git a/internal/db/dbq/imports.sql.go b/internal/db/dbq/imports.sql.go index 1d1bb0c6..e6920393 100644 --- a/internal/db/dbq/imports.sql.go +++ b/internal/db/dbq/imports.sql.go @@ -59,6 +59,38 @@ func (q *Queries) ClaimGrabImport(ctx context.Context, id pgtype.UUID) (Grab, er return i, err } +const clearTrackSourceVerdict = `-- name: ClearTrackSourceVerdict :exec +UPDATE tracks SET source_verdict = NULL, source_verdict_at = NULL + WHERE id = $1 AND source_verdict = 'suspect' +` + +// A track whose file was replaced in place is no longer the rip it was. +func (q *Queries) ClearTrackSourceVerdict(ctx context.Context, id pgtype.UUID) error { + _, err := q.db.Exec(ctx, clearTrackSourceVerdict, id) + return err +} + +const finishUpgrade = `-- name: FinishUpgrade :exec +UPDATE wants + SET album_id = COALESCE($1, album_id), + upgrade = '', + upgrade_checked_at = NULL, + updated_at = now() + WHERE id = $2 +` + +type FinishUpgradeParams struct { + AlbumID pgtype.UUID + ID pgtype.UUID +} + +// An upgrade was imported: the want holds the album the new tracks are in, +// and is judged again by the next upgrade check. +func (q *Queries) FinishUpgrade(ctx context.Context, arg FinishUpgradeParams) error { + _, err := q.db.Exec(ctx, finishUpgrade, arg.AlbumID, arg.ID) + return err +} + const forceGrabImport = `-- name: ForceGrabImport :one UPDATE grabs SET import_state = '', import_forced = true, import_reason = '', import_updated_at = now() @@ -479,6 +511,57 @@ func (q *Queries) ListReleaseBlocksPage(ctx context.Context, arg ListReleaseBloc return items, nil } +const listUpgradeTracks = `-- name: ListUpgradeTracks :many +SELECT id, file_path, mbid, COALESCE(disc_number, 1)::int AS disc, track_number, + file_format, file_size, duration_ms, source_verdict + FROM tracks + WHERE album_id = $1 AND missing_since IS NULL +` + +type ListUpgradeTracksRow struct { + ID pgtype.UUID + FilePath string + Mbid *string + Disc int32 + TrackNumber *int32 + FileFormat string + FileSize int64 + DurationMs int32 + SourceVerdict *string +} + +// The tracks an upgrade may replace (#5524): the present tracks of the album +// the want holds. +func (q *Queries) ListUpgradeTracks(ctx context.Context, albumID pgtype.UUID) ([]ListUpgradeTracksRow, error) { + rows, err := q.db.Query(ctx, listUpgradeTracks, albumID) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListUpgradeTracksRow + for rows.Next() { + var i ListUpgradeTracksRow + if err := rows.Scan( + &i.ID, + &i.FilePath, + &i.Mbid, + &i.Disc, + &i.TrackNumber, + &i.FileFormat, + &i.FileSize, + &i.DurationMs, + &i.SourceVerdict, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const markWantSearched = `-- name: MarkWantSearched :exec UPDATE wants SET searched_at = now() WHERE id = $1 ` @@ -490,7 +573,7 @@ func (q *Queries) MarkWantSearched(ctx context.Context, id pgtype.UUID) error { } const openWantsForGroups = `-- name: OpenWantsForGroups :many -SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.primary_type, w.first_release_date, w.reason, w.album_id, w.created_by, w.wanted_at, w.fulfilled_at, w.created_at, w.updated_at, w.searched_at, w.search_outcome, w.search_considered, w.best_release, w.best_verdict, w.search_error FROM wants w +SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.primary_type, w.first_release_date, w.reason, w.album_id, w.created_by, w.wanted_at, w.fulfilled_at, w.created_at, w.updated_at, w.searched_at, w.search_outcome, w.search_considered, w.best_release, w.best_verdict, w.search_error, w.upgrade, w.held_tier, w.held_quality, w.upgrade_checked_at FROM wants w WHERE w.fulfilled_at IS NULL AND w.release_group_mbid = ANY($1::text[]) AND NOT EXISTS (SELECT 1 FROM grabs g WHERE g.want_id = w.id AND g.ended_at IS NULL) ` @@ -526,6 +609,10 @@ func (q *Queries) OpenWantsForGroups(ctx context.Context, groups []string) ([]Wa &i.BestRelease, &i.BestVerdict, &i.SearchError, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.UpgradeCheckedAt, ); err != nil { return nil, err } diff --git a/internal/db/dbq/models.go b/internal/db/dbq/models.go index 84b05f94..abfd23f9 100644 --- a/internal/db/dbq/models.go +++ b/internal/db/dbq/models.go @@ -232,6 +232,7 @@ type AcquisitionSetting struct { LibraryListingStartedAt pgtype.Timestamptz LibraryListedAt pgtype.Timestamptz LibraryUnlisted int32 + UpgradeUntil string } type Album struct { @@ -1070,6 +1071,10 @@ type Want struct { BestRelease string BestVerdict string SearchError string + Upgrade string + HeldTier string + HeldQuality string + UpgradeCheckedAt pgtype.Timestamptz } type WantCandidate struct { diff --git a/internal/db/dbq/reacquisition.sql.go b/internal/db/dbq/reacquisition.sql.go index 870f9bad..d5a05760 100644 --- a/internal/db/dbq/reacquisition.sql.go +++ b/internal/db/dbq/reacquisition.sql.go @@ -217,7 +217,7 @@ ON CONFLICT (release_group_mbid) DO UPDATE fulfilled_at = NULL, updated_at = now() WHERE wants.fulfilled_at IS NOT NULL -RETURNING 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 +RETURNING 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, upgrade, held_tier, held_quality, upgrade_checked_at ` type WantMissingAlbumParams struct { @@ -260,6 +260,10 @@ func (q *Queries) WantMissingAlbum(ctx context.Context, arg WantMissingAlbumPara &i.BestRelease, &i.BestVerdict, &i.SearchError, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.UpgradeCheckedAt, ) return i, err } diff --git a/internal/db/dbq/search.sql.go b/internal/db/dbq/search.sql.go index 3b2a75e4..da7089e7 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, library_listing_started_at, library_listed_at, library_unlisted 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, upgrade_until FROM acquisition_settings WHERE id = true ` // Searching for wants and grabbing releases (M502 #5521). @@ -118,6 +118,7 @@ func (q *Queries) GetAcquisitionSettings(ctx context.Context) (AcquisitionSettin &i.LibraryListingStartedAt, &i.LibraryListedAt, &i.LibraryUnlisted, + &i.UpgradeUntil, ) return i, err } @@ -201,6 +202,7 @@ func (q *Queries) GetWantCandidate(ctx context.Context, arg GetWantCandidatePara const getWantForSearch = `-- name: GetWantForSearch :one SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reason, w.fulfilled_at, + w.upgrade, w.held_tier, w.held_quality, w.album_id, COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, COALESCE(g.secondary_types, '{}')::text[] AS secondary_types, COALESCE(NULLIF(w.first_release_date, ''), g.first_release_date, '')::text AS first_release_date @@ -217,6 +219,10 @@ type GetWantForSearchRow struct { Title string Reason string FulfilledAt pgtype.Timestamptz + Upgrade string + HeldTier string + HeldQuality string + AlbumID pgtype.UUID PrimaryType string SecondaryTypes []string FirstReleaseDate string @@ -233,6 +239,10 @@ func (q *Queries) GetWantForSearch(ctx context.Context, id pgtype.UUID) (GetWant &i.Title, &i.Reason, &i.FulfilledAt, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.AlbumID, &i.PrimaryType, &i.SecondaryTypes, &i.FirstReleaseDate, @@ -449,16 +459,20 @@ func (q *Queries) ListWantCandidates(ctx context.Context, wantID pgtype.UUID) ([ } const listWantsDueForSearch = `-- name: ListWantsDueForSearch :many -SELECT w.id, w.release_group_mbid, w.artist_name, w.title, w.reason, +SELECT w.id, w.release_group_mbid, w.artist_name, w.title, w.reason, w.upgrade, w.held_tier, w.held_quality, COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, COALESCE(g.secondary_types, '{}')::text[] AS secondary_types, COALESCE(NULLIF(w.first_release_date, ''), g.first_release_date, '')::text AS first_release_date FROM wants w LEFT JOIN mb_release_groups g ON g.mbid = w.release_group_mbid - WHERE w.fulfilled_at IS NULL + WHERE (w.fulfilled_at IS NULL OR w.upgrade <> '') AND NOT EXISTS (SELECT 1 FROM grabs gr WHERE gr.want_id = w.id AND gr.ended_at IS NULL) - AND (w.searched_at IS NULL OR w.searched_at < now() - make_interval(hours => $1::int)) - ORDER BY w.searched_at IS NOT NULL, + AND (w.searched_at IS NULL + OR w.searched_at < now() - make_interval(hours => CASE WHEN w.fulfilled_at IS NULL + THEN $1::int + ELSE GREATEST($1::int, 168) END)) + ORDER BY w.fulfilled_at IS NOT NULL, + w.searched_at IS NOT NULL, CASE w.reason WHEN 'request' THEN 0 WHEN 'missing' THEN 1 ELSE 2 END, w.searched_at NULLS FIRST, w.wanted_at, w.id LIMIT $2 @@ -475,14 +489,18 @@ type ListWantsDueForSearchRow struct { ArtistName string Title string Reason string + Upgrade string + HeldTier string + HeldQuality string PrimaryType string SecondaryTypes []string FirstReleaseDate string } -// Open wants with no download running that were never searched or not since -// @hours ago: never-searched first, then requests, then missing files, then -// followed artists' releases. +// Wants with no download running that were never searched or not since +// @hours ago: open wants first (never-searched, then requests, then missing +// files, then followed artists' releases), then held albums wanted for an +// upgrade (#5524), which are searched at most weekly. func (q *Queries) ListWantsDueForSearch(ctx context.Context, arg ListWantsDueForSearchParams) ([]ListWantsDueForSearchRow, error) { rows, err := q.db.Query(ctx, listWantsDueForSearch, arg.Hours, arg.MaxRows) if err != nil { @@ -498,6 +516,9 @@ func (q *Queries) ListWantsDueForSearch(ctx context.Context, arg ListWantsDueFor &i.ArtistName, &i.Title, &i.Reason, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, &i.PrimaryType, &i.SecondaryTypes, &i.FirstReleaseDate, @@ -609,9 +630,10 @@ UPDATE acquisition_settings seed_ratio = $5, seed_days = $6, stall_hours = $7, + upgrade_until = $8, 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, library_listing_started_at, library_listed_at, library_unlisted +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, upgrade_until ` type UpdateAcquisitionSettingsParams struct { @@ -622,6 +644,7 @@ type UpdateAcquisitionSettingsParams struct { SeedRatio float32 SeedDays int32 StallHours int32 + UpgradeUntil string } func (q *Queries) UpdateAcquisitionSettings(ctx context.Context, arg UpdateAcquisitionSettingsParams) (AcquisitionSetting, error) { @@ -633,6 +656,7 @@ func (q *Queries) UpdateAcquisitionSettings(ctx context.Context, arg UpdateAcqui arg.SeedRatio, arg.SeedDays, arg.StallHours, + arg.UpgradeUntil, ) var i AcquisitionSetting err := row.Scan( @@ -650,6 +674,7 @@ func (q *Queries) UpdateAcquisitionSettings(ctx context.Context, arg UpdateAcqui &i.LibraryListingStartedAt, &i.LibraryListedAt, &i.LibraryUnlisted, + &i.UpgradeUntil, ) return i, err } diff --git a/internal/db/dbq/tracks.sql.go b/internal/db/dbq/tracks.sql.go index 05387d86..a26f2f9a 100644 --- a/internal/db/dbq/tracks.sql.go +++ b/internal/db/dbq/tracks.sql.go @@ -660,16 +660,17 @@ SELECT t.id, artists.name AS artist_name, t.source_verdict, t.source_verdict_at, - searched.at AS searched_at + -- The album's place on the wanted list: wanted for a clean copy, and + -- when that was last searched for (#5524). + COALESCE(w.upgrade, '')::text AS want_upgrade, + w.searched_at AS searched_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id - -- When the resolver last asked Lidarr to search the album for a better copy - -- (#5447). LEFT JOIN LATERAL ( - SELECT max(l.created_at)::timestamptz AS at FROM audit_log l - WHERE l.action = 'lidarr_rip_search' AND l.metadata->>'album_id' = albums.id::text - ) searched ON true + SELECT x.upgrade, x.searched_at FROM wants x + WHERE x.album_id = albums.id AND x.fulfilled_at IS NOT NULL LIMIT 1 + ) w ON true WHERE t.missing_since IS NULL AND regexp_replace(t.file_path, '^.*/', '') ~* $1::text ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title @@ -696,6 +697,7 @@ type ListSuspectSourceTracksRow struct { ArtistName string SourceVerdict *string SourceVerdictAt pgtype.Timestamptz + WantUpgrade string SearchedAt pgtype.Timestamptz } @@ -732,6 +734,7 @@ func (q *Queries) ListSuspectSourceTracks(ctx context.Context, arg ListSuspectSo &i.ArtistName, &i.SourceVerdict, &i.SourceVerdictAt, + &i.WantUpgrade, &i.SearchedAt, ); err != nil { return nil, err diff --git a/internal/db/dbq/wanted.sql.go b/internal/db/dbq/wanted.sql.go index 3d5204b7..77f6701a 100644 --- a/internal/db/dbq/wanted.sql.go +++ b/internal/db/dbq/wanted.sql.go @@ -20,6 +20,22 @@ func (q *Queries) ClearDefaultReleaseTypeProfile(ctx context.Context) error { return err } +const clearHeldUpgrades = `-- name: ClearHeldUpgrades :execrows +UPDATE wants SET upgrade = '', held_tier = '', held_quality = '', upgrade_checked_at = now() + WHERE fulfilled_at IS NOT NULL AND upgrade <> '' + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = wants.album_id AND t.missing_since IS NULL AND t.duration_ms > 0) +` + +// A held want whose album has no track left to judge is not wanted again. +func (q *Queries) ClearHeldUpgrades(ctx context.Context) (int64, error) { + result, err := q.db.Exec(ctx, clearHeldUpgrades) + if err != nil { + return 0, err + } + return result.RowsAffected(), nil +} + const countAlbumsToMatch = `-- name: CountAlbumsToMatch :one SELECT COUNT(*) FROM albums a @@ -52,6 +68,8 @@ const countWants = `-- name: CountWants :one SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::bigint AS wanted, COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing, COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL)::bigint AS held, + -- Held albums wanted for a better copy (#5524). + COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL AND upgrade <> '')::bigint AS upgrades, COUNT(DISTINCT artist_mbid)::bigint AS artists, -- Library albums waiting to be matched by hand (#5579). (SELECT COUNT(*) FROM library_album_matches m @@ -72,6 +90,7 @@ type CountWantsRow struct { Wanted int64 Missing int64 Held int64 + Upgrades int64 Artists int64 Unmatched int64 Following int64 @@ -87,6 +106,7 @@ func (q *Queries) CountWants(ctx context.Context) (CountWantsRow, error) { &i.Wanted, &i.Missing, &i.Held, + &i.Upgrades, &i.Artists, &i.Unmatched, &i.Following, @@ -350,7 +370,7 @@ func (q *Queries) GetWantAlbum(ctx context.Context, id pgtype.UUID) (GetWantAlbu } const getWantByReleaseGroup = `-- name: GetWantByReleaseGroup :one -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 release_group_mbid = $1 +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, upgrade, held_tier, held_quality, upgrade_checked_at FROM wants WHERE release_group_mbid = $1 ` func (q *Queries) GetWantByReleaseGroup(ctx context.Context, releaseGroupMbid string) (Want, error) { @@ -377,6 +397,10 @@ func (q *Queries) GetWantByReleaseGroup(ctx context.Context, releaseGroupMbid st &i.BestRelease, &i.BestVerdict, &i.SearchError, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.UpgradeCheckedAt, ) return i, err } @@ -700,6 +724,69 @@ func (q *Queries) ListFollowsDue(ctx context.Context, arg ListFollowsDueParams) return items, nil } +const listHeldWantFormats = `-- name: ListHeldWantFormats :many + +SELECT w.id AS want_id, w.upgrade, w.held_tier, w.held_quality, + lower(t.file_format)::text AS file_format, + SUM(t.file_size)::bigint AS bytes, + SUM(t.duration_ms)::bigint AS duration_ms, + COUNT(*)::int AS tracks, + COUNT(*) FILTER (WHERE t.source_verdict = 'suspect')::int AS rips + FROM wants w + JOIN tracks t ON t.album_id = w.album_id + WHERE w.fulfilled_at IS NOT NULL + AND t.missing_since IS NULL AND t.duration_ms > 0 + GROUP BY w.id, lower(t.file_format) + ORDER BY w.id +` + +type ListHeldWantFormatsRow struct { + WantID pgtype.UUID + Upgrade string + HeldTier string + HeldQuality string + FileFormat string + Bytes int64 + DurationMs int64 + Tracks int32 + Rips int32 +} + +// Upgrades (#5524). ------------------------------------------------------- +// What each held album's copy is made of, for judging whether it is worth +// upgrading: its present tracks grouped by format, with their bytes and +// length summed, and how many are held back as video rips. A held want whose +// album has no track with a length yet has no rows. +func (q *Queries) ListHeldWantFormats(ctx context.Context) ([]ListHeldWantFormatsRow, error) { + rows, err := q.db.Query(ctx, listHeldWantFormats) + if err != nil { + return nil, err + } + defer rows.Close() + var items []ListHeldWantFormatsRow + for rows.Next() { + var i ListHeldWantFormatsRow + if err := rows.Scan( + &i.WantID, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.FileFormat, + &i.Bytes, + &i.DurationMs, + &i.Tracks, + &i.Rips, + ); err != nil { + return nil, err + } + items = append(items, i) + } + if err := rows.Err(); err != nil { + return nil, err + } + return items, nil +} + const listLibraryAlbumTracks = `-- name: ListLibraryAlbumTracks :many SELECT title, disc_number, track_number, duration_ms, (missing_since IS NOT NULL)::bool AS missing FROM tracks @@ -779,7 +866,7 @@ func (q *Queries) ListLibraryAlbums(ctx context.Context, arg ListLibraryAlbumsPa } 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 +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, upgrade, held_tier, held_quality, upgrade_checked_at FROM wants WHERE fulfilled_at IS NULL ORDER BY wanted_at, id LIMIT $1 @@ -815,6 +902,10 @@ func (q *Queries) ListOpenWants(ctx context.Context, limit int32) ([]Want, error &i.BestRelease, &i.BestVerdict, &i.SearchError, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.UpgradeCheckedAt, ); err != nil { return nil, err } @@ -1023,6 +1114,8 @@ func (q *Queries) ListWantArtists(ctx context.Context, arg ListWantArtistsParams const listWants = `-- name: ListWants :many SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reason, w.album_id, w.wanted_at, w.fulfilled_at, + -- Why a held album is wanted again, and the copy it has (#5524). + w.upgrade, w.held_quality, -- A request names an album before the catalogue has read it: the -- catalogue's type and date fill in once it has. COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, @@ -1049,6 +1142,7 @@ SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reas WHERE CASE $1::text WHEN 'all' THEN true WHEN 'held' THEN w.fulfilled_at IS NOT NULL + WHEN 'upgrade' THEN w.fulfilled_at IS NOT NULL AND w.upgrade <> '' WHEN 'missing' THEN w.fulfilled_at IS NULL AND w.reason = 'missing' ELSE w.fulfilled_at IS NULL AND w.reason <> 'missing' END @@ -1080,6 +1174,8 @@ type ListWantsRow struct { AlbumID pgtype.UUID WantedAt pgtype.Timestamptz FulfilledAt pgtype.Timestamptz + Upgrade string + HeldQuality string PrimaryType string SecondaryTypes []string FirstReleaseDate string @@ -1137,6 +1233,8 @@ func (q *Queries) ListWants(ctx context.Context, arg ListWantsParams) ([]ListWan &i.AlbumID, &i.WantedAt, &i.FulfilledAt, + &i.Upgrade, + &i.HeldQuality, &i.PrimaryType, &i.SecondaryTypes, &i.FirstReleaseDate, @@ -1330,6 +1428,38 @@ func (q *Queries) SetDefaultReleaseTypeProfile(ctx context.Context, id pgtype.UU return result.RowsAffected(), nil } +const setWantUpgrades = `-- name: SetWantUpgrades :exec +UPDATE wants w + SET upgrade = v.upgrade, + held_tier = v.held_tier, + held_quality = v.held_quality, + upgrade_checked_at = now(), + updated_at = CASE WHEN w.upgrade <> v.upgrade THEN now() ELSE w.updated_at END + FROM (SELECT unnest($1::uuid[]) AS id, + unnest($2::text[]) AS upgrade, + unnest($3::text[]) AS held_tier, + unnest($4::text[]) AS held_quality) v + WHERE w.id = v.id +` + +type SetWantUpgradesParams struct { + Ids []pgtype.UUID + Upgrades []string + HeldTiers []string + HeldQualities []string +} + +// Writes the upgrade check's verdicts, one row per want. +func (q *Queries) SetWantUpgrades(ctx context.Context, arg SetWantUpgradesParams) error { + _, err := q.db.Exec(ctx, setWantUpgrades, + arg.Ids, + arg.Upgrades, + arg.HeldTiers, + arg.HeldQualities, + ) + return err +} + const startLibraryListing = `-- name: StartLibraryListing :one UPDATE acquisition_settings SET library_listing_started_at = COALESCE(library_listing_started_at, now()) @@ -1502,7 +1632,7 @@ INSERT INTO wants (release_group_mbid, artist_mbid, artist_name, title, primary_ VALUES ($1, $2, $3, $4, $5, $6, $7, $8) ON CONFLICT (release_group_mbid) DO UPDATE SET updated_at = wants.updated_at -RETURNING 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 +RETURNING 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, upgrade, held_tier, held_quality, upgrade_checked_at ` type WantReleaseGroupParams struct { @@ -1551,6 +1681,10 @@ func (q *Queries) WantReleaseGroup(ctx context.Context, arg WantReleaseGroupPara &i.BestRelease, &i.BestVerdict, &i.SearchError, + &i.Upgrade, + &i.HeldTier, + &i.HeldQuality, + &i.UpgradeCheckedAt, ) return i, err } diff --git a/internal/db/migrations/0090_upgrades.down.sql b/internal/db/migrations/0090_upgrades.down.sql new file mode 100644 index 00000000..8986bbc1 --- /dev/null +++ b/internal/db/migrations/0090_upgrades.down.sql @@ -0,0 +1,7 @@ +DROP INDEX IF EXISTS wants_upgrade_idx; +ALTER TABLE wants + DROP COLUMN upgrade_checked_at, + DROP COLUMN held_quality, + DROP COLUMN held_tier, + DROP COLUMN upgrade; +ALTER TABLE acquisition_settings DROP COLUMN upgrade_until; diff --git a/internal/db/migrations/0090_upgrades.up.sql b/internal/db/migrations/0090_upgrades.up.sql new file mode 100644 index 00000000..ffa3ad5f --- /dev/null +++ b/internal/db/migrations/0090_upgrades.up.sql @@ -0,0 +1,29 @@ +-- Upgrades (M502 #5524): a held album whose copy is below the quality the +-- operator settles for, or that holds a video rip, stays wanted for a better +-- copy. The import replaces the old tracks with the new ones. +-- +-- acquisition_settings.upgrade_until the tier a held album is upgraded to; +-- '' switches quality upgrades off. +-- Lidarr's "Any" profile upgraded up to +-- High Quality Lossy (note #5527). +-- wants.upgrade why a held album is wanted again: +-- quality (below upgrade_until) or rip +-- (a track is a video rip); '' not. +-- wants.held_tier, held_quality the library copy's tier, estimated +-- from format and size per second, and +-- the same in words. +-- wants.upgrade_checked_at when the wanted worker last judged it. +-- want_candidates.verdict unchanged: a release no better than +-- the copy held is "refused". +ALTER TABLE acquisition_settings + ADD COLUMN upgrade_until text NOT NULL DEFAULT 'lossy_high'; + +ALTER TABLE wants + ADD COLUMN upgrade text NOT NULL DEFAULT '' + CONSTRAINT wants_upgrade_check CHECK (upgrade IN ('', 'quality', 'rip')), + ADD COLUMN held_tier text NOT NULL DEFAULT '', + ADD COLUMN held_quality text NOT NULL DEFAULT '', + ADD COLUMN upgrade_checked_at timestamptz; + +CREATE INDEX wants_upgrade_idx ON wants (searched_at NULLS FIRST) + WHERE fulfilled_at IS NOT NULL AND upgrade <> ''; diff --git a/internal/db/queries/imports.sql b/internal/db/queries/imports.sql index ffc67643..c677ab07 100644 --- a/internal/db/queries/imports.sql +++ b/internal/db/queries/imports.sql @@ -140,3 +140,26 @@ SELECT b.info_hash, b.name, b.reason, b.detail, b.blocked_at, b.expires_at, w.ti WHERE b.expires_at IS NULL OR b.expires_at > now() ORDER BY b.blocked_at DESC, b.info_hash LIMIT sqlc.arg(max_rows) OFFSET sqlc.arg(skip); + +-- name: ListUpgradeTracks :many +-- The tracks an upgrade may replace (#5524): the present tracks of the album +-- the want holds. +SELECT id, file_path, mbid, COALESCE(disc_number, 1)::int AS disc, track_number, + file_format, file_size, duration_ms, source_verdict + FROM tracks + WHERE album_id = $1 AND missing_since IS NULL; + +-- name: FinishUpgrade :exec +-- An upgrade was imported: the want holds the album the new tracks are in, +-- and is judged again by the next upgrade check. +UPDATE wants + SET album_id = COALESCE(sqlc.narg(album_id), album_id), + upgrade = '', + upgrade_checked_at = NULL, + updated_at = now() + WHERE id = sqlc.arg(id); + +-- name: ClearTrackSourceVerdict :exec +-- A track whose file was replaced in place is no longer the rip it was. +UPDATE tracks SET source_verdict = NULL, source_verdict_at = NULL + WHERE id = $1 AND source_verdict = 'suspect'; diff --git a/internal/db/queries/search.sql b/internal/db/queries/search.sql index 30677414..89579c2e 100644 --- a/internal/db/queries/search.sql +++ b/internal/db/queries/search.sql @@ -12,6 +12,7 @@ UPDATE acquisition_settings seed_ratio = sqlc.arg(seed_ratio), seed_days = sqlc.arg(seed_days), stall_hours = sqlc.arg(stall_hours), + upgrade_until = sqlc.arg(upgrade_until), updated_at = now() WHERE id = true RETURNING *; @@ -20,25 +21,31 @@ RETURNING *; UPDATE acquisition_settings SET last_pass_at = now(), last_pass = sqlc.arg(report) WHERE id = true; -- name: ListWantsDueForSearch :many --- Open wants with no download running that were never searched or not since --- @hours ago: never-searched first, then requests, then missing files, then --- followed artists' releases. -SELECT w.id, w.release_group_mbid, w.artist_name, w.title, w.reason, +-- Wants with no download running that were never searched or not since +-- @hours ago: open wants first (never-searched, then requests, then missing +-- files, then followed artists' releases), then held albums wanted for an +-- upgrade (#5524), which are searched at most weekly. +SELECT w.id, w.release_group_mbid, w.artist_name, w.title, w.reason, w.upgrade, w.held_tier, w.held_quality, COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, COALESCE(g.secondary_types, '{}')::text[] AS secondary_types, COALESCE(NULLIF(w.first_release_date, ''), g.first_release_date, '')::text AS first_release_date FROM wants w LEFT JOIN mb_release_groups g ON g.mbid = w.release_group_mbid - WHERE w.fulfilled_at IS NULL + WHERE (w.fulfilled_at IS NULL OR w.upgrade <> '') AND NOT EXISTS (SELECT 1 FROM grabs gr WHERE gr.want_id = w.id AND gr.ended_at IS NULL) - AND (w.searched_at IS NULL OR w.searched_at < now() - make_interval(hours => sqlc.arg(hours)::int)) - ORDER BY w.searched_at IS NOT NULL, + AND (w.searched_at IS NULL + OR w.searched_at < now() - make_interval(hours => CASE WHEN w.fulfilled_at IS NULL + THEN sqlc.arg(hours)::int + ELSE GREATEST(sqlc.arg(hours)::int, 168) END)) + ORDER BY w.fulfilled_at IS NOT NULL, + w.searched_at IS NOT NULL, CASE w.reason WHEN 'request' THEN 0 WHEN 'missing' THEN 1 ELSE 2 END, w.searched_at NULLS FIRST, w.wanted_at, w.id LIMIT sqlc.arg(max_rows); -- name: GetWantForSearch :one SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reason, w.fulfilled_at, + w.upgrade, w.held_tier, w.held_quality, w.album_id, COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, COALESCE(g.secondary_types, '{}')::text[] AS secondary_types, COALESCE(NULLIF(w.first_release_date, ''), g.first_release_date, '')::text AS first_release_date diff --git a/internal/db/queries/tracks.sql b/internal/db/queries/tracks.sql index 449ac0fb..df0a7c91 100644 --- a/internal/db/queries/tracks.sql +++ b/internal/db/queries/tracks.sql @@ -288,16 +288,17 @@ SELECT t.id, artists.name AS artist_name, t.source_verdict, t.source_verdict_at, - searched.at AS searched_at + -- The album's place on the wanted list: wanted for a clean copy, and + -- when that was last searched for (#5524). + COALESCE(w.upgrade, '')::text AS want_upgrade, + w.searched_at AS searched_at FROM tracks t JOIN albums ON albums.id = t.album_id JOIN artists ON artists.id = t.artist_id - -- When the resolver last asked Lidarr to search the album for a better copy - -- (#5447). LEFT JOIN LATERAL ( - SELECT max(l.created_at)::timestamptz AS at FROM audit_log l - WHERE l.action = 'lidarr_rip_search' AND l.metadata->>'album_id' = albums.id::text - ) searched ON true + SELECT x.upgrade, x.searched_at FROM wants x + WHERE x.album_id = albums.id AND x.fulfilled_at IS NOT NULL LIMIT 1 + ) w ON true WHERE t.missing_since IS NULL AND regexp_replace(t.file_path, '^.*/', '') ~* sqlc.arg(pattern)::text ORDER BY directory, t.disc_number NULLS FIRST, t.track_number NULLS FIRST, t.title diff --git a/internal/db/queries/wanted.sql b/internal/db/queries/wanted.sql index cd74b2c2..f2ba3d7b 100644 --- a/internal/db/queries/wanted.sql +++ b/internal/db/queries/wanted.sql @@ -183,6 +183,8 @@ DELETE FROM wants WHERE id = $1; -- artist or album title; total counts every match, past the page. SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reason, w.album_id, w.wanted_at, w.fulfilled_at, + -- Why a held album is wanted again, and the copy it has (#5524). + w.upgrade, w.held_quality, -- A request names an album before the catalogue has read it: the -- catalogue's type and date fill in once it has. COALESCE(NULLIF(w.primary_type, ''), g.primary_type, '')::text AS primary_type, @@ -209,6 +211,7 @@ SELECT w.id, w.release_group_mbid, w.artist_mbid, w.artist_name, w.title, w.reas WHERE CASE sqlc.arg(state)::text WHEN 'all' THEN true WHEN 'held' THEN w.fulfilled_at IS NOT NULL + WHEN 'upgrade' THEN w.fulfilled_at IS NOT NULL AND w.upgrade <> '' WHEN 'missing' THEN w.fulfilled_at IS NULL AND w.reason = 'missing' ELSE w.fulfilled_at IS NULL AND w.reason <> 'missing' END @@ -270,6 +273,8 @@ SELECT title, disc_number, track_number, duration_ms, (missing_since IS NOT NULL SELECT COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason <> 'missing')::bigint AS wanted, COUNT(*) FILTER (WHERE fulfilled_at IS NULL AND reason = 'missing')::bigint AS missing, COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL)::bigint AS held, + -- Held albums wanted for a better copy (#5524). + COUNT(*) FILTER (WHERE fulfilled_at IS NOT NULL AND upgrade <> '')::bigint AS upgrades, COUNT(DISTINCT artist_mbid)::bigint AS artists, -- Library albums waiting to be matched by hand (#5579). (SELECT COUNT(*) FROM library_album_matches m @@ -448,3 +453,44 @@ SELECT a.id, a.title, a.mbid, a.release_group_mbid, ar.name AS artist_name, ar.m UPDATE library_album_matches SET outcome = sqlc.arg(outcome), decided_at = now() WHERE album_id = sqlc.arg(album_id) AND outcome = sqlc.arg(was); + +-- Upgrades (#5524). ------------------------------------------------------- + +-- name: ListHeldWantFormats :many +-- What each held album's copy is made of, for judging whether it is worth +-- upgrading: its present tracks grouped by format, with their bytes and +-- length summed, and how many are held back as video rips. A held want whose +-- album has no track with a length yet has no rows. +SELECT w.id AS want_id, w.upgrade, w.held_tier, w.held_quality, + lower(t.file_format)::text AS file_format, + SUM(t.file_size)::bigint AS bytes, + SUM(t.duration_ms)::bigint AS duration_ms, + COUNT(*)::int AS tracks, + COUNT(*) FILTER (WHERE t.source_verdict = 'suspect')::int AS rips + FROM wants w + JOIN tracks t ON t.album_id = w.album_id + WHERE w.fulfilled_at IS NOT NULL + AND t.missing_since IS NULL AND t.duration_ms > 0 + GROUP BY w.id, lower(t.file_format) + ORDER BY w.id; + +-- name: SetWantUpgrades :exec +-- Writes the upgrade check's verdicts, one row per want. +UPDATE wants w + SET upgrade = v.upgrade, + held_tier = v.held_tier, + held_quality = v.held_quality, + upgrade_checked_at = now(), + updated_at = CASE WHEN w.upgrade <> v.upgrade THEN now() ELSE w.updated_at END + FROM (SELECT unnest(sqlc.arg(ids)::uuid[]) AS id, + unnest(sqlc.arg(upgrades)::text[]) AS upgrade, + unnest(sqlc.arg(held_tiers)::text[]) AS held_tier, + unnest(sqlc.arg(held_qualities)::text[]) AS held_quality) v + WHERE w.id = v.id; + +-- name: ClearHeldUpgrades :execrows +-- A held want whose album has no track left to judge is not wanted again. +UPDATE wants SET upgrade = '', held_tier = '', held_quality = '', upgrade_checked_at = now() + WHERE fulfilled_at IS NOT NULL AND upgrade <> '' + AND NOT EXISTS (SELECT 1 FROM tracks t + WHERE t.album_id = wants.album_id AND t.missing_since IS NULL AND t.duration_ms > 0); diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index ae8b602a..b066fb04 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -191,7 +191,7 @@ func ResetDB(t *testing.T, pool *pgxpool.Pool) { 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, + seed_ratio = DEFAULT, seed_days = DEFAULT, stall_hours = DEFAULT, upgrade_until = 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 { diff --git a/internal/importer/importer.go b/internal/importer/importer.go index 1c070fcd..3c5732fd 100644 --- a/internal/importer/importer.go +++ b/internal/importer/importer.go @@ -66,6 +66,9 @@ type Importer struct { // Roots are the library's folders (the scan paths). New music is filed // under the one the settings name, or the first. Roots []string + // DataDir is where cached art lives, for tidying an artist an upgrade + // leaves with nothing (#5524). Empty skips it. + DataDir string // ClientFor builds the download client a grab went to. Defaults to // acquire.DownloadClientFor. ClientFor func(ctx context.Context, code string) (downloader.Driver, error) @@ -338,6 +341,9 @@ func (im *Importer) place(ctx context.Context, q *dbq.Queries, g dbq.Grab, d dow e = &editions[v.Edition] } plan := Plan(root, w, e, files, v.Pairs) + if w.Upgrade != "" { + return im.placeUpgrade(ctx, q, g, d, w, e, files, plan, v, det, record) + } heldRec, heldPos, err := im.held(ctx, q, w, plan) if err != nil { return record(StateWaiting, "Couldn't read what the library holds: "+err.Error(), "", det, 0, "") @@ -504,10 +510,14 @@ func (im *Importer) want(ctx context.Context, q *dbq.Queries, id pgtype.UUID) (W if err != nil { return Want{}, err } - return Want{ + w := Want{ GroupMBID: row.ReleaseGroupMbid, ArtistMBID: row.ArtistMbid, Artist: row.ArtistName, Title: row.Title, Year: yearOf(row.FirstReleaseDate), PrimaryType: row.PrimaryType, SecondaryTypes: row.SecondaryTypes, - }, nil + } + if row.FulfilledAt.Valid && row.Upgrade != "" && row.AlbumID.Valid { + w.Upgrade, w.AlbumID = row.Upgrade, row.AlbumID + } + return w, nil } func (im *Importer) editions(ctx context.Context, group string) ([]Edition, error) { diff --git a/internal/importer/place.go b/internal/importer/place.go index 5687ca4b..e2e3d5c5 100644 --- a/internal/importer/place.go +++ b/internal/importer/place.go @@ -92,7 +92,19 @@ var taggable = map[string]bool{".flac": true, ".ogg": true, ".opus": true, ".mp3 // container will not take the file's streams) the file is copied as it is // and tagged reports false. An existing dst is never overwritten. func Place(ctx context.Context, src, dst string, t Tags) (tagged bool, err error) { - if _, err := os.Lstat(dst); err == nil { + return placeFile(ctx, src, dst, t, false) +} + +// Overwrite is Place onto a file an upgrade replaces (#5524): the new copy +// takes the old one's path in one rename, so the library never sees the +// path empty, and the track row at that path, history and all, carries on +// with the new file. +func Overwrite(ctx context.Context, src, dst string, t Tags) (tagged bool, err error) { + return placeFile(ctx, src, dst, t, true) +} + +func placeFile(ctx context.Context, src, dst string, t Tags, overwrite bool) (tagged bool, err error) { + if _, err := os.Lstat(dst); err == nil && !overwrite { return false, fmt.Errorf("%w: %s", ErrOccupied, dst) } dir := filepath.Dir(dst) @@ -114,7 +126,7 @@ func Place(ctx context.Context, src, dst string, t Tags) (tagged bool, err error if err := os.Chmod(tmp, 0o644); err != nil { return false, fmt.Errorf("chmod %s: %w", tmp, err) } - if _, err := os.Lstat(dst); err == nil { + if _, err := os.Lstat(dst); err == nil && !overwrite { return false, fmt.Errorf("%w: %s", ErrOccupied, dst) } if err := os.Rename(tmp, dst); err != nil { diff --git a/internal/importer/upgrade.go b/internal/importer/upgrade.go new file mode 100644 index 00000000..8a75272f --- /dev/null +++ b/internal/importer/upgrade.go @@ -0,0 +1,304 @@ +package importer + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/acquire" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/downloader" + "git.fabledsword.com/bvandeusen/minstrel/internal/library" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" +) + +// Importing an upgrade (M502 #5524): a better copy of an album the library +// holds. Each file is matched to the track it would replace, by recording +// and then by disc and track. It replaces that track when the track is a +// video rip or the new file is a better tier; otherwise the old track stays. +// A file with no track to replace fills a gap, as any import does. +// +// The new copy is filed before the old one goes. On the old file's own path +// it takes the path in one rename and the row carries on; at a new path it +// is scanned in as a track of its own and the old track is folded into it +// (library.ReplaceTrack), so likes, plays and playlist entries carry over. + +// OldTrack is a library track an upgrade may replace. +type OldTrack struct { + ID pgtype.UUID + Path string + Recording string + Disc, Track int + Format string + Bytes int64 + Ms int64 + Rip bool +} + +// Tier is the old track's tier, estimated as the upgrade check estimates +// an album's; ok is false for a format the ladder cannot place. +func (o OldTrack) Tier() (release.Tier, bool) { + return release.TierOfFile(o.Format, release.FileKbps(o.Bytes, o.Ms)) +} + +// What an upgrade does with one placement. +const ( + stepPlace = "place" // no track to replace: fills a gap + stepReplace = "replace" // replaces Old from a new path + stepOverwrite = "overwrite" // replaces Old on its own path + stepKeep = "keep" // Old is as good: left as it is +) + +type upgradeStep struct { + Placement int + Action string + Old int // index into olds, or -1 +} + +// planUpgrade matches each placement to the track it would replace and +// decides what to do with it. newTiers is each placement's tier. +func planUpgrade(plan []Placement, newTiers []release.Tier, olds []OldTrack) []upgradeStep { + used := make([]bool, len(olds)) + match := make([]int, len(plan)) + for i := range match { + match[i] = -1 + } + for i, pl := range plan { + if pl.Recording == "" { + continue + } + for j, o := range olds { + if !used[j] && o.Recording == pl.Recording { + match[i], used[j] = j, true + break + } + } + } + for i, pl := range plan { + if match[i] >= 0 || pl.Track <= 0 { + continue + } + for j, o := range olds { + if !used[j] && o.Track == pl.Track && max(o.Disc, 1) == max(pl.Disc, 1) { + match[i], used[j] = j, true + break + } + } + } + steps := make([]upgradeStep, len(plan)) + for i, pl := range plan { + j := match[i] + steps[i] = upgradeStep{Placement: i, Old: j, Action: stepPlace} + if j < 0 { + continue + } + o := olds[j] + oldTier, _ := o.Tier() + better := release.Rank(newTiers[i]) > release.Rank(oldTier) + switch { + case !o.Rip && !better: + steps[i].Action = stepKeep + case filepath.Clean(pl.Dest) == filepath.Clean(o.Path): + steps[i].Action = stepOverwrite + default: + steps[i].Action = stepReplace + } + } + return steps +} + +// fileTier estimates a downloaded file's tier from its size and length, the +// way the library's are estimated, so the two compare like for like. A file +// that cannot be measured takes the release's tier. +func fileTier(f File, fallback release.Tier) release.Tier { + if info, err := os.Stat(f.Path); err == nil && f.LengthMs > 0 { + if t, ok := release.TierOfFile(filepath.Ext(f.Path), release.FileKbps(info.Size(), int64(f.LengthMs))); ok { + return t + } + } + return fallback +} + +func oldTracks(rows []dbq.ListUpgradeTracksRow) []OldTrack { + out := make([]OldTrack, 0, len(rows)) + for _, r := range rows { + o := OldTrack{ID: r.ID, Path: r.FilePath, Disc: int(r.Disc), Format: r.FileFormat, + Bytes: r.FileSize, Ms: int64(r.DurationMs), Rip: r.SourceVerdict != nil && *r.SourceVerdict == "suspect"} + if r.Mbid != nil { + o.Recording = *r.Mbid + } + if r.TrackNumber != nil { + o.Track = int(*r.TrackNumber) + } + out = append(out, o) + } + return out +} + +// placeUpgrade files an accepted upgrade, replacing the tracks it betters. +func (im *Importer) placeUpgrade(ctx context.Context, q *dbq.Queries, g dbq.Grab, d downloader.Driver, w Want, + e *Edition, files []File, plan []Placement, v Verdict, det Detail, record recordFunc) string { + rows, err := q.ListUpgradeTracks(ctx, w.AlbumID) + if err != nil { + return record(StateWaiting, "Couldn't read the album's tracks: "+err.Error(), "", det, 0, "") + } + olds := oldTracks(rows) + grabTier := release.Tier(g.Quality) + newTiers := make([]release.Tier, len(plan)) + for i, pl := range plan { + newTiers[i] = fileTier(files[pl.File], grabTier) + } + steps := planUpgrade(plan, newTiers, olds) + heldRec, heldPos, err := im.held(ctx, q, w, plan) + if err != nil { + return record(StateWaiting, "Couldn't read what the library holds: "+err.Error(), "", det, 0, "") + } + + type swap struct { + old OldTrack + dest string + } + var ( + placed []string // every file written, for the scan + swaps []swap + inPlace []OldTrack + failed []string + ) + for _, st := range steps { + pl := plan[st.Placement] + if st.Action == stepKeep { + det.Skipped++ + continue + } + if st.Action == stepPlace && ((pl.Recording != "" && heldRec[pl.Recording]) || heldPos[[2]int{pl.Disc, pl.Track}]) { + det.Skipped++ + continue + } + tags := Tags{Title: pl.Title, Artist: w.Artist, Album: w.Title, AlbumArtist: w.Artist, Year: w.Year, + Disc: pl.Disc, Discs: pl.Discs, Track: pl.Track, Tracks: pl.Tracks, + GroupMBID: w.GroupMBID, RecordingMBID: pl.Recording, ArtistMBID: w.ArtistMBID} + if e != nil { + tags.ReleaseMBID = e.MBID + } + src := files[pl.File].Path + var tagged bool + if st.Action == stepOverwrite { + tagged, err = Overwrite(ctx, src, pl.Dest, tags) + } else { + tagged, err = Place(ctx, src, pl.Dest, tags) + } + switch { + case errors.Is(err, ErrOccupied): + det.Skipped++ + continue + case err != nil: + // Each replacement stands alone: the old track stays where a + // copy failed, and the rest go ahead. + failed = append(failed, pl.Title) + im.logger.Warn("import: copy an upgrade into the library", "grab", g.Name, "dest", pl.Dest, "err", err) + continue + } + placed = append(placed, pl.Dest) + if !tagged { + det.Untagged++ + } + switch st.Action { + case stepOverwrite: + inPlace = append(inPlace, olds[st.Old]) + case stepReplace: + swaps = append(swaps, swap{old: olds[st.Old], dest: pl.Dest}) + } + } + det.Placed = len(placed) + + if len(placed) == 0 { + why := fmt.Sprintf("Nothing in it is better than the copy of %s you have, so nothing was imported.", w.Title) + if len(failed) > 0 { + return record(StateFailed, fmt.Sprintf("Copying into the library failed for %s.", strings.Join(failed, ", ")), + "", det, 0, "") + } + // Set aside, so the weekly search doesn't take it again. + if err := acquire.NewMemory(im.pool).Block(ctx, dbq.BlockReleaseParams{ + InfoHash: g.InfoHash, Name: g.Name, Reason: acquire.BlockImport, Detail: why, WantID: g.WantID, + }); err != nil { + im.logger.Warn("import: set the release aside", "grab", g.Name, "err", err) + } + if err := q.MarkWantSearched(ctx, g.WantID); err != nil { + im.logger.Warn("import: mark the want searched", "grab", g.Name, "err", err) + } + im.removeFromClient(ctx, d, g) + return record(StateHeld, why, StateHeld, det, 0, "") + } + + if _, err := im.Scanner.ScanFiles(ctx, placed); err != nil { + im.logger.Warn("import: scan the new files", "grab", g.Name, "err", err) + } + for _, o := range inPlace { + if o.Rip { + if err := q.ClearTrackSourceVerdict(ctx, o.ID); err != nil { + im.logger.Warn("import: release a replaced rip", "path", o.Path, "err", err) + } + } + } + replaced, kept := len(inPlace), 0 + for _, s := range swaps { + nt, err := q.GetTrackByPath(ctx, s.dest) + if err != nil { + kept++ + im.logger.Warn("import: find the new track", "path", s.dest, "err", err) + continue + } + moved, err := library.ReplaceTrack(ctx, im.pool, im.logger, im.DataDir, s.old.ID, nt.ID) + if err != nil { + // The old file stays; the duplicate sweep finds the two copies. + kept++ + im.logger.Warn("import: replace the old track", "old", s.old.Path, "new", s.dest, "err", err) + continue + } + replaced++ + im.logger.Info("import: replaced a track with a better copy", "old", s.old.Path, "new", s.dest, + "play_events", moved.PlayEvents, "likes", moved.Likes, "playlist_entries", moved.PlaylistEntries) + } + + // The want holds the album the new tracks are in, once the old album row + // has none left. + var album pgtype.UUID + if left, err := q.ListUpgradeTracks(ctx, w.AlbumID); err == nil && len(left) == 0 { + if nt, err := q.GetTrackByPath(ctx, placed[0]); err == nil { + album = nt.AlbumID + } + } + if err := q.FinishUpgrade(ctx, dbq.FinishUpgradeParams{ID: g.WantID, AlbumID: album}); err != nil { + im.logger.Warn("import: finish the upgrade", "grab", g.Name, "err", err) + } + if err := q.MarkWantSearched(ctx, g.WantID); err != nil { + im.logger.Warn("import: mark the want searched", "grab", g.Name, "err", err) + } + + dest := filepath.Dir(placed[0]) + if e != nil && e.Media() > 1 { + dest = filepath.Dir(dest) + } + reason := fmt.Sprintf("Replaced %s of %s with a better copy.", tracks(replaced), w.Title) + if added := len(placed) - replaced - kept; added > 0 { + reason += fmt.Sprintf(" Added %s it was missing.", tracks(added)) + } + if det.Skipped > 0 { + reason += fmt.Sprintf(" %s already as good left as they were.", tracks(det.Skipped)) + } + if kept > 0 { + reason += fmt.Sprintf(" %s couldn't be removed, so both copies are in the library.", tracks(kept)) + } + if len(failed) > 0 { + reason += fmt.Sprintf(" Copying failed for %s; those stay as they were.", strings.Join(failed, ", ")) + } + if det.Untagged > 0 { + reason += fmt.Sprintf(" %s copied with their own tags: ffmpeg could not rewrite them.", tracks(det.Untagged)) + } + return record(StateImported, reason+" "+v.Reason, StateImported, det, len(placed), dest) +} diff --git a/internal/importer/upgrade_test.go b/internal/importer/upgrade_test.go new file mode 100644 index 00000000..a415fd45 --- /dev/null +++ b/internal/importer/upgrade_test.go @@ -0,0 +1,53 @@ +package importer + +import ( + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/release" +) + +func TestPlanUpgrade_ReplacesOnlyWhatItBetters(t *testing.T) { + plan := []Placement{ + {File: 0, Dest: "/music/A/B (2001)/A - B - 01 - One.flac", Disc: 1, Track: 1, Recording: "rec-1"}, + {File: 1, Dest: "/music/A/B (2001)/A - B - 02 - Two.mp3", Disc: 1, Track: 2}, + {File: 2, Dest: "/music/A/B (2001)/A - B - 03 - Three.flac", Disc: 1, Track: 3}, + {File: 3, Dest: "/music/A/B (2001)/A - B - 04 - Four.flac", Disc: 1, Track: 4}, + {File: 4, Dest: "/music/A/B (2001)/A - B - 05 - Five.flac", Disc: 1, Track: 5}, + } + newTiers := []release.Tier{release.TierLossless, release.TierLossyHigh, release.TierLossless, + release.TierLossless, release.TierLossyLow} + olds := []OldTrack{ + // Matched by recording though its number is wrong: a 192 MP3. + {Path: "/music/A/B/01 One.mp3", Recording: "rec-1", Disc: 1, Track: 9, Format: "mp3", Bytes: 4_800_000, Ms: 200_000}, + // Same path as the new file, a 320 MP3: the new 320 is no better. + {Path: "/music/A/B (2001)/A - B - 02 - Two.mp3", Disc: 1, Track: 2, Format: "mp3", Bytes: 8_000_000, Ms: 200_000}, + // Already lossless: kept. + {Path: "/music/A/B/03 Three.flac", Disc: 1, Track: 3, Format: "flac", Bytes: 22_000_000, Ms: 200_000}, + // A video rip, even though lossless: replaced. + {Path: "/music/A/B/04 Four (Official Video).flac", Disc: 0, Track: 4, Format: "flac", Bytes: 22_000_000, Ms: 200_000, Rip: true}, + } + steps := planUpgrade(plan, newTiers, olds) + want := []struct { + action string + old int + }{ + {stepReplace, 0}, {stepKeep, 1}, {stepKeep, 2}, {stepReplace, 3}, {stepPlace, -1}, + } + for i, w := range want { + if steps[i].Action != w.action || steps[i].Old != w.old { + t.Errorf("placement %d: %s old %d, want %s old %d", i, steps[i].Action, steps[i].Old, w.action, w.old) + } + } +} + +func TestPlanUpgrade_OverwritesAWorseFileOnItsOwnPath(t *testing.T) { + dest := "/music/A/B (2001)/A - B - 01 - One.mp3" + steps := planUpgrade( + []Placement{{File: 0, Dest: dest, Disc: 1, Track: 1}}, + []release.Tier{release.TierLossyHigh}, + []OldTrack{{Path: dest, Disc: 1, Track: 1, Format: "mp3", Bytes: 3_200_000, Ms: 200_000}}, // 128 + ) + if steps[0].Action != stepOverwrite || steps[0].Old != 0 { + t.Errorf("got %+v", steps[0]) + } +} diff --git a/internal/importer/verify.go b/internal/importer/verify.go index b33925e8..a1f50166 100644 --- a/internal/importer/verify.go +++ b/internal/importer/verify.go @@ -16,6 +16,8 @@ import ( "sort" "strings" + "github.com/jackc/pgx/v5/pgtype" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) @@ -111,6 +113,11 @@ type Want struct { Year int PrimaryType string SecondaryTypes []string + // Upgrade: a held album wanted for a better copy (#5524), quality or + // rip; AlbumID is the library album it holds, whose tracks the import + // replaces. + Upgrade string + AlbumID pgtype.UUID } // Pair is a file matched to an edition's track, and what agreed. diff --git a/internal/library/replace.go b/internal/library/replace.go new file mode 100644 index 00000000..80fd2a1f --- /dev/null +++ b/internal/library/replace.go @@ -0,0 +1,83 @@ +package library + +import ( + "context" + "errors" + "fmt" + "log/slog" + + "github.com/jackc/pgx/v5" + "github.com/jackc/pgx/v5/pgtype" + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/coverart" + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + syncpkg "git.fabledsword.com/bvandeusen/minstrel/internal/sync" +) + +// Replacing a track with a better copy (M502 #5524). +// +// An upgrade files the better copy first, as a track of its own, and only +// then replaces the old one: its file is removed, then its likes, plays, +// playlist entries and tags move onto the new row and the old row goes. It +// is the duplicate merge with the survivor chosen by the import, and it +// keeps the merge's order (#3918): if the old file cannot be removed nothing +// in the database changes, and the next scan finds the two copies as a +// duplicate pair. + +// ErrReplaceSame: the old and new track are one row. +var ErrReplaceSame = errors.New("library: a track cannot replace itself") + +// Replaced is what one replacement moved onto the new track. +type Replaced struct { + PlayEvents int64 + SkipEvents int64 + Likes int + PlaylistEntries int +} + +// ReplaceTrack replaces oldID with newID: the old file is removed, then the +// old row folded into the new one. dataDir, when set, also clears the cached +// art of an artist the fold left with nothing. logger may be nil. +func ReplaceTrack( + ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, dataDir string, oldID, newID pgtype.UUID, +) (Replaced, error) { + if logger == nil { + logger = slog.Default() + } + if oldID == newID { + return Replaced{}, ErrReplaceSame + } + old, err := dbq.New(pool).GetTrackByID(ctx, oldID) + if err != nil { + if errors.Is(err, pgx.ErrNoRows) { + return Replaced{}, ErrTrackNotFound + } + return Replaced{}, fmt.Errorf("get track: %w", err) + } + if err := removeTrackFileOnDisk(old.FilePath); err != nil { + return Replaced{}, err + } + + var f foldResult + err = pgx.BeginFunc(ctx, pool, func(tx pgx.Tx) error { + changes := mergeChanges{} + var ferr error + f, ferr = foldTrackInto(ctx, dbq.New(tx), newID, oldID, &changes) + if ferr != nil { + return ferr + } + return changes.log(ctx, tx) + }) + if err != nil { + return Replaced{}, fmt.Errorf("fold the old track into the new: %w", err) + } + if f.DeletedArtistID != nil && dataDir != "" { + if err := coverart.CleanupArtistArt(dataDir, *f.DeletedArtistID); err != nil { + logger.Warn("track replace: artist-art cleanup failed", + "artist_id", syncpkg.FormatUUID(*f.DeletedArtistID), "err", err) + } + } + return Replaced{PlayEvents: f.PlayEvents, SkipEvents: f.SkipEvents, Likes: len(f.Likers), + PlaylistEntries: f.PlaylistEntries}, nil +} diff --git a/internal/release/quality.go b/internal/release/quality.go index 66590f9a..bfde8fc1 100644 --- a/internal/release/quality.go +++ b/internal/release/quality.go @@ -167,3 +167,105 @@ func NewProfile(tiers []string) Profile { // Allows says whether the profile takes a tier. func (p Profile) Allows(t Tier) bool { return p.Allowed[t] } + +// Held is what the library's copy of an album is: its tier, estimated from +// what the scanner records, and the same in words. +type Held struct { + Tier Tier + Words string +} + +// HeldFormat is an album's present tracks of one format, summed. +type HeldFormat struct { + Format string + Bytes, Ms int64 +} + +// HeldCopy judges an album's copy from its tracks grouped by format: the +// lowest tier among them, as Lidarr judged an album by its worst file. +// Summing a format's tracks first keeps a near-silent VBR interlude from +// dragging the album down. ok is false when no format can be placed. +func HeldCopy(formats []HeldFormat) (Held, bool) { + var out Held + found := false + for _, f := range formats { + kbps := FileKbps(f.Bytes, f.Ms) + t, ok := TierOfFile(f.Format, kbps) + if !ok { + continue + } + if !found || Rank(t) < Rank(out.Tier) { + out = Held{Tier: t, Words: HeldWords(f.Format, t, kbps)} + found = true + } + } + return out, found +} + +// FileKbps is the average bitrate of files totalling bytes over ms of audio: +// the scanner records no bitrate (tracks.bitrate is never filled), so size +// per second is the measure there is. Cover art and tags count too, which +// only ever reads high. +func FileKbps(bytes, ms int64) int { + if bytes <= 0 || ms <= 0 { + return 0 + } + return int(bytes * 8 / ms) +} + +// mp3High is the average bitrate from which an MP3 counts as high quality +// when only its size is known. A V0 encode averages 220 to 260 kbps, so the +// line sits below it: an estimate errs toward the copy held, and an album is +// never fetched again on a guess (#5524). +const mp3High = 220 + +// TierOfFile places files of one format on the ladder from their average +// bitrate (FileKbps). format is the scanner's extension, without the dot. +// ok is false for a format the ladder cannot place. +func TierOfFile(format string, kbps int) (Tier, bool) { + switch strings.ToLower(strings.TrimPrefix(format, ".")) { + case "flac", "wav", "aiff", "aif", "ape", "wv", "alac": + // 16-bit/44.1 kHz stereo is 1,411 kbps before compression; past + // 1,600 on average is more bits or a higher rate than CD. + if kbps > 1600 { + return TierLosslessHiRes, true + } + return TierLossless, true + case "dsf", "dff": + return TierLosslessHiRes, true + case "m4a", "mp4": + // ALAC or AAC: the container says nothing, the rate does. No AAC + // encode runs near 500 kbps. + if kbps > 500 { + return TierLossless, true + } + return byBitrate(kbps, 256), kbps > 0 + case "mp3": + if kbps >= mp3High { + return TierLossyHigh, true + } + return byBitrate(kbps, 320), kbps > 0 + case "aac", "ogg", "oga", "opus": + return byBitrate(kbps, 256), kbps > 0 + } + return TierUnknown, false +} + +// HeldWords says what a held copy is: "FLAC", "MP3, about 192 kbps". +func HeldWords(format string, tier Tier, kbps int) string { + f := strings.ToUpper(strings.TrimPrefix(format, ".")) + switch tier { + case TierLosslessHiRes: + return f + ", hi-res" + case TierLossless: + return f + } + if kbps <= 0 { + return f + } + return fmt.Sprintf("%s, about %d kbps", f, roundKbps(kbps)) +} + +// roundKbps rounds an average bitrate to the nearest 8, so a 191 reads as +// the 192 it was encoded at. +func roundKbps(k int) int { return (k + 4) / 8 * 8 } diff --git a/internal/release/release_test.go b/internal/release/release_test.go index 0bde6e41..91ea3843 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -251,3 +251,61 @@ func TestParseTorrent_FilesAndInfohash(t *testing.T) { t.Error("HTML parsed as a torrent") } } + +func TestTierOfFile(t *testing.T) { + cases := []struct { + format string + kbps int + want Tier + ok bool + }{ + {"flac", 900, TierLossless, true}, + {"FLAC", 2800, TierLosslessHiRes, true}, + {"m4a", 750, TierLossless, true}, + {"m4a", 256, TierLossyHigh, true}, + {"mp3", 320, TierLossyHigh, true}, + {"mp3", 245, TierLossyHigh, true}, // a V0, by size + {"mp3", 192, TierLossyLow, true}, + {"mp3", 128, TierLossyPoor, true}, + {"mp3", 96, TierLossyTrash, true}, + {"opus", 160, TierLossyLow, true}, + {"wma", 192, TierUnknown, false}, + {"mp3", 0, TierLossyStandard, false}, + } + for _, c := range cases { + got, ok := TierOfFile(c.format, c.kbps) + if got != c.want || ok != c.ok { + t.Errorf("TierOfFile(%q, %d) = %s, %v; want %s, %v", c.format, c.kbps, got, ok, c.want, c.ok) + } + } +} + +func TestFileKbpsAndWords(t *testing.T) { + // 5 MB over 3:30 of audio is about 190 kbps. + k := FileKbps(5_000_000, 210_000) + if k != 190 { + t.Fatalf("FileKbps = %d, want 190", k) + } + if got := HeldWords("mp3", TierLossyLow, 191); got != "MP3, about 192 kbps" { + t.Errorf("HeldWords = %q", got) + } + if got := HeldWords("flac", TierLossless, 900); got != "FLAC" { + t.Errorf("HeldWords = %q", got) + } + if FileKbps(0, 1000) != 0 || FileKbps(1000, 0) != 0 { + t.Error("FileKbps with nothing to measure is 0") + } +} + +func TestHeldCopy_TakesTheWorstFormat(t *testing.T) { + h, ok := HeldCopy([]HeldFormat{ + {Format: "flac", Bytes: 300_000_000, Ms: 2_400_000}, + {Format: "mp3", Bytes: 4_800_000, Ms: 200_000}, // 192 kbps + }) + if !ok || h.Tier != TierLossyLow || h.Words != "MP3, about 192 kbps" { + t.Fatalf("HeldCopy = %+v, %v", h, ok) + } + if _, ok := HeldCopy([]HeldFormat{{Format: "wma", Bytes: 1, Ms: 1}}); ok { + t.Error("a format the ladder can't place says nothing") + } +} diff --git a/internal/wanted/search.go b/internal/wanted/search.go index c8aa7885..ef915866 100644 --- a/internal/wanted/search.go +++ b/internal/wanted/search.go @@ -48,7 +48,8 @@ const ( passTimeout = 15 * time.Minute ) -// ErrWantHeld: the want is already answered by the library. +// ErrWantHeld: the want is already answered by the library, and is not +// wanted for a better copy. var ErrWantHeld = errors.New("wanted: the library already holds this album") // Searcher searches for wants and grabs what they need. @@ -167,8 +168,9 @@ func (s *Searcher) pass(ctx context.Context) Report { rep.Paced = ctx.Err() == nil return rep } - want := s.wantFor(ctx, row.ID, row.ArtistName, row.Title, row.PrimaryType, row.SecondaryTypes, - row.FirstReleaseDate, row.ReleaseGroupMbid) + want := s.wantFor(ctx, searchRow{id: row.ID, artist: row.ArtistName, title: row.Title, + primary: row.PrimaryType, secondary: row.SecondaryTypes, date: row.FirstReleaseDate, + group: row.ReleaseGroupMbid, upgrade: row.Upgrade, heldTier: row.HeldTier, held: row.HeldQuality}) out := s.one(ctx, svc, want, settings.AutoGrab) rep.Looked++ switch out.Outcome { @@ -186,17 +188,31 @@ func (s *Searcher) pass(ctx context.Context) Report { return rep } +// searchRow is what a search needs of a wants row. +type searchRow struct { + id pgtype.UUID + artist, title, primary, date, group string + secondary []string + // upgrade, heldTier and held: a held album wanted for a better copy + // (#5524), and the copy it has. + upgrade, heldTier, held string +} + // wantFor describes a want for the matcher, with its editions' track counts // from the catalogue (read from MusicBrainz the first time when it can be). -func (s *Searcher) wantFor(ctx context.Context, id pgtype.UUID, artist, title, primary string, secondary []string, date, group string) acquire.Want { - w := acquire.Want{ID: id, Want: release.Want{ - Artist: artist, Title: title, Year: yearOf(date), PrimaryType: primary, SecondaryTypes: secondary, +// A held album wanted for better quality takes its copy's tier as a floor. +func (s *Searcher) wantFor(ctx context.Context, r searchRow) acquire.Want { + w := acquire.Want{ID: r.id, Want: release.Want{ + Artist: r.artist, Title: r.title, Year: yearOf(r.date), PrimaryType: r.primary, SecondaryTypes: r.secondary, }} + if r.upgrade == UpgradeQuality && release.KnownTier(r.heldTier) { + w.Floor, w.Held = release.Tier(r.heldTier), r.held + } q := dbq.New(s.pool) - span, err := q.EditionTrackSpan(ctx, group) + span, err := q.EditionTrackSpan(ctx, r.group) if err == nil && span.MinTracks == 0 && s.cat != nil { - if _, rerr := s.cat.Releases(ctx, group); rerr == nil { - span, err = q.EditionTrackSpan(ctx, group) + if _, rerr := s.cat.Releases(ctx, r.group); rerr == nil { + span, err = q.EditionTrackSpan(ctx, r.group) } } if err == nil { @@ -329,15 +345,17 @@ func (s *Searcher) Take(ctx context.Context, id pgtype.UUID, key string, force b return took, nil } -// load reads an open want for a search. +// load reads a want for a search: an open one, or a held album wanted for a +// better copy (#5524). func (s *Searcher) load(ctx context.Context, id pgtype.UUID) (acquire.Want, error) { row, err := dbq.New(s.pool).GetWantForSearch(ctx, id) if err != nil { return acquire.Want{}, err } - if row.FulfilledAt.Valid { + if row.FulfilledAt.Valid && row.Upgrade == "" { return acquire.Want{}, ErrWantHeld } - return s.wantFor(ctx, row.ID, row.ArtistName, row.Title, row.PrimaryType, row.SecondaryTypes, - row.FirstReleaseDate, row.ReleaseGroupMbid), nil + return s.wantFor(ctx, searchRow{id: row.ID, artist: row.ArtistName, title: row.Title, + primary: row.PrimaryType, secondary: row.SecondaryTypes, date: row.FirstReleaseDate, + group: row.ReleaseGroupMbid, upgrade: row.Upgrade, heldTier: row.HeldTier, held: row.HeldQuality}), nil } diff --git a/internal/wanted/upgrade.go b/internal/wanted/upgrade.go new file mode 100644 index 00000000..a5959626 --- /dev/null +++ b/internal/wanted/upgrade.go @@ -0,0 +1,125 @@ +package wanted + +import ( + "context" + "fmt" + "time" + + "github.com/jackc/pgx/v5/pgtype" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" +) + +// Upgrades (M502 #5524): a held album stays wanted when the copy the library +// has is below the quality the operator settles for (acquisition_settings. +// upgrade_until), or when one of its tracks is a video rip. The search +// schedule then looks for a better release weekly, and the import replaces +// the old tracks with the new ones, carrying their history across. + +// Why a held album is wanted again, matching wants_upgrade_check. +const ( + UpgradeQuality = "quality" + UpgradeRip = "rip" +) + +// upgradeCheckEvery is how often held albums are judged. Their files change +// only by an import or a scan, and the check reads every held album. +const upgradeCheckEvery = time.Hour + +// UpgradeFor says why a held album is wanted again, if it is: a video rip +// among its tracks always (a rip is no copy of the album, whatever the +// setting), else a copy below until. until "" switches quality upgrades off. +func UpgradeFor(held release.Held, judged bool, rips int, until release.Tier) string { + switch { + case rips > 0: + return UpgradeRip + case until == "" || !judged: + return "" + case release.Rank(held.Tier) < release.Rank(until): + return UpgradeQuality + } + return "" +} + +// heldVerdict is one held want as the check judged it. +type heldVerdict struct { + id pgtype.UUID + upgrade, tier, words string + oldUpgrade, oldTier, old string +} + +func (v heldVerdict) changed() bool { + return v.upgrade != v.oldUpgrade || v.tier != v.oldTier || v.words != v.old +} + +// checkUpgrades judges every held album, at most once an hour, and records +// which are wanted for a better copy. +func (s *Service) checkUpgrades(ctx context.Context, res *PassResult) error { + if !s.upgradesChecked.IsZero() && s.now().Sub(s.upgradesChecked) < upgradeCheckEvery { + return nil + } + q := dbq.New(s.pool) + settings, err := q.GetAcquisitionSettings(ctx) + if err != nil { + return fmt.Errorf("read settings: %w", err) + } + rows, err := q.ListHeldWantFormats(ctx) + if err != nil { + return fmt.Errorf("read held albums: %w", err) + } + until := release.Tier(settings.UpgradeUntil) + var out []heldVerdict + flush := func(id pgtype.UUID, group []dbq.ListHeldWantFormatsRow) { + if len(group) == 0 { + return + } + formats := make([]release.HeldFormat, 0, len(group)) + rips := 0 + for _, r := range group { + formats = append(formats, release.HeldFormat{Format: r.FileFormat, Bytes: r.Bytes, Ms: r.DurationMs}) + rips += int(r.Rips) + } + held, judged := release.HeldCopy(formats) + v := heldVerdict{id: id, upgrade: UpgradeFor(held, judged, rips, until), + oldUpgrade: group[0].Upgrade, oldTier: group[0].HeldTier, old: group[0].HeldQuality} + if judged { + v.tier, v.words = string(held.Tier), held.Words + } + if v.upgrade != "" { + res.Upgrades++ + } + if v.changed() { + out = append(out, v) + } + } + var group []dbq.ListHeldWantFormatsRow + for _, r := range rows { + if len(group) > 0 && group[0].WantID != r.WantID { + flush(group[0].WantID, group) + group = group[:0] + } + group = append(group, r) + } + if len(group) > 0 { + flush(group[0].WantID, group) + } + + if len(out) > 0 { + p := dbq.SetWantUpgradesParams{} + for _, v := range out { + p.Ids = append(p.Ids, v.id) + p.Upgrades = append(p.Upgrades, v.upgrade) + p.HeldTiers = append(p.HeldTiers, v.tier) + p.HeldQualities = append(p.HeldQualities, v.words) + } + if err := q.SetWantUpgrades(ctx, p); err != nil { + return fmt.Errorf("record upgrades: %w", err) + } + } + if _, err := q.ClearHeldUpgrades(ctx); err != nil { + return fmt.Errorf("clear upgrades: %w", err) + } + s.upgradesChecked = s.now() + return nil +} diff --git a/internal/wanted/upgrade_test.go b/internal/wanted/upgrade_test.go new file mode 100644 index 00000000..9bc39008 --- /dev/null +++ b/internal/wanted/upgrade_test.go @@ -0,0 +1,137 @@ +package wanted + +import ( + "context" + "fmt" + "testing" + "time" + + "github.com/jackc/pgx/v5/pgxpool" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" +) + +func TestUpgradeFor(t *testing.T) { + low := release.Held{Tier: release.TierLossyLow} + flac := release.Held{Tier: release.TierLossless} + high := release.Held{Tier: release.TierLossyHigh} + cases := []struct { + name string + held release.Held + judged bool + rips int + until release.Tier + want string + }{ + {"below the cutoff", low, true, 0, release.TierLossyHigh, UpgradeQuality}, + {"at the cutoff", high, true, 0, release.TierLossyHigh, ""}, + {"above the cutoff", flac, true, 0, release.TierLossyHigh, ""}, + {"upgrades off", low, true, 0, "", ""}, + {"a rip, upgrades off", flac, true, 1, "", UpgradeRip}, + {"nothing to judge", release.Held{}, false, 0, release.TierLossless, ""}, + } + for _, c := range cases { + if got := UpgradeFor(c.held, c.judged, c.rips, c.until); got != c.want { + t.Errorf("%s: %q, want %q", c.name, got, c.want) + } + } +} + +// seedMP3s puts Blur's The Ballad of Darren in the library as MP3s at about +// kbps, two tracks, the first held back as a video rip when rip is set. +func seedMP3s(t *testing.T, pool *pgxpool.Pool, kbps int64, rip bool) dbq.Album { + t.Helper() + ctx := context.Background() + q := dbq.New(pool) + mbid, group, rel := blur, theBallad, "r-ballad-mp3" + artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Blur", SortName: "Blur", Mbid: &mbid}) + if err != nil { + t.Fatal(err) + } + album, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "The Ballad of Darren", SortTitle: "Ballad of Darren", + ArtistID: artist.ID, Mbid: &rel, ReleaseGroupMbid: &group}) + if err != nil { + t.Fatal(err) + } + for i := range 2 { + num := int32(i + 1) + tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: fmt.Sprintf("Track %d", i+1), AlbumID: album.ID, ArtistID: artist.ID, TrackNumber: &num, + DurationMs: 200000, FilePath: fmt.Sprintf("/music/ballad/%02d.mp3", i+1), + FileSize: kbps * 200000 / 8, FileFormat: "mp3", + }) + if err != nil { + t.Fatal(err) + } + if rip && i == 0 { + if _, err := pool.Exec(ctx, `UPDATE tracks SET source_verdict = 'suspect' WHERE id = $1`, tr.ID); err != nil { + t.Fatal(err) + } + } + } + return album +} + +func TestCheckUpgrades_AHeldAlbumBelowTheCutoffIsWantedAgain_Integration(t *testing.T) { + pool := newPool(t) + q := dbq.New(pool) + ctx := context.Background() + s := newService(t, pool, newFakeMB(t)) + + if _, err := WantWith(ctx, q, WantParams{ + GroupMBID: theBallad, ArtistMBID: blur, ArtistName: "Blur", Title: "The Ballad of Darren", Reason: ReasonRequest, + }); err != nil { + t.Fatal(err) + } + seedMP3s(t, pool, 192, false) + res, err := s.Pass(ctx) + if err != nil || res.Held != 1 || res.Upgrades != 1 { + t.Fatalf("pass: %+v, %v", res, err) + } + w, _ := q.GetWantByReleaseGroup(ctx, theBallad) + if w.Upgrade != UpgradeQuality || w.HeldTier != string(release.TierLossyLow) || w.HeldQuality != "MP3, about 192 kbps" { + t.Fatalf("want = upgrade %q, tier %q, %q", w.Upgrade, w.HeldTier, w.HeldQuality) + } + due, err := q.ListWantsDueForSearch(ctx, dbq.ListWantsDueForSearchParams{Hours: 24, MaxRows: 10}) + if err != nil || len(due) != 1 || due[0].Upgrade != UpgradeQuality { + t.Fatalf("due for search: %+v, %v", due, err) + } + + // Upgrades switched off: no longer wanted again. + if _, err := pool.Exec(ctx, `UPDATE acquisition_settings SET upgrade_until = ''`); err != nil { + t.Fatal(err) + } + s.upgradesChecked = time.Time{} + if _, err := s.Pass(ctx); err != nil { + t.Fatal(err) + } + w, _ = q.GetWantByReleaseGroup(ctx, theBallad) + if w.Upgrade != "" { + t.Errorf("upgrades off: upgrade %q", w.Upgrade) + } +} + +func TestCheckUpgrades_ARipIsReplacedWhateverTheSetting_Integration(t *testing.T) { + pool := newPool(t) + q := dbq.New(pool) + ctx := context.Background() + s := newService(t, pool, newFakeMB(t)) + + if _, err := WantWith(ctx, q, WantParams{ + GroupMBID: theBallad, ArtistMBID: blur, ArtistName: "Blur", Title: "The Ballad of Darren", Reason: ReasonRequest, + }); err != nil { + t.Fatal(err) + } + seedMP3s(t, pool, 320, true) + if _, err := pool.Exec(ctx, `UPDATE acquisition_settings SET upgrade_until = ''`); err != nil { + t.Fatal(err) + } + if _, err := s.Pass(ctx); err != nil { + t.Fatal(err) + } + w, _ := q.GetWantByReleaseGroup(ctx, theBallad) + if w.Upgrade != UpgradeRip { + t.Errorf("a rip: upgrade %q", w.Upgrade) + } +} diff --git a/internal/wanted/wanted.go b/internal/wanted/wanted.go index 3efbf930..e23ea9d5 100644 --- a/internal/wanted/wanted.go +++ b/internal/wanted/wanted.go @@ -50,6 +50,9 @@ type Service struct { // want recorded under a release id (#5241) to its group. A field so tests // stand in for MusicBrainz. releaseGroup func(ctx context.Context, releaseMBID string) (string, error) + // upgradesChecked is when held albums were last judged for an upgrade + // (#5524); the worker alone touches it. + upgradesChecked time.Time } // New makes a Service reading discographies and editions through cat. @@ -175,9 +178,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 || res.Listed > 0: + case res.Wanted > 0 || res.Held > 0 || res.Baselined > 0 || res.Listed > 0 || res.Upgrades > 0: s.logger.Info("wanted: pass complete", "follows", res.Follows, "baselined", res.Baselined, - "wanted", res.Wanted, "held", res.Held, "listed", res.Listed) + "wanted", res.Wanted, "held", res.Held, "listed", res.Listed, "upgrades", res.Upgrades) } select { case <-ctx.Done(): @@ -194,6 +197,9 @@ type PassResult struct { Wanted int // wants recorded from follows Held int // wants the library now answers Listed int // library albums put on the list as held (#5567) + // Upgrades: held albums wanted for a better copy, when the hourly check + // ran this pass (#5524). + Upgrades int } // Pass applies the follows due and settles open wants. An unreachable @@ -213,6 +219,9 @@ func (s *Service) Pass(ctx context.Context) (PassResult, error) { if err := s.settleWants(ctx, &res); err != nil { return res, err } + if err := s.checkUpgrades(ctx, &res); err != nil && ctx.Err() == nil { + s.logger.Warn("wanted: judge held albums for upgrades", "err", err) + } return res, ferr } diff --git a/web/src/lib/api/admin.suspect-sources.test.ts b/web/src/lib/api/admin.suspect-sources.test.ts index 59017a66..9f8624c3 100644 --- a/web/src/lib/api/admin.suspect-sources.test.ts +++ b/web/src/lib/api/admin.suspect-sources.test.ts @@ -17,6 +17,7 @@ function track(id: string): AdminSuspectTrack { markers: [], verdict: null, verdict_at: null, + replacing: false, searched_at: null }; } diff --git a/web/src/lib/api/types.ts b/web/src/lib/api/types.ts index 2fac4486..d3487934 100644 --- a/web/src/lib/api/types.ts +++ b/web/src/lib/api/types.ts @@ -443,7 +443,9 @@ export type AdminSuspectTrack = { // back. null: the resolver hasn't looked yet (M498 #5439). verdict: SuspectVerdict | null; verdict_at: string | null; - // When Lidarr was last asked to search the album for a better copy (#5447). + // The album is on the wanted list for a clean copy, and when one was last + // searched for (#5524). + replacing: boolean; searched_at: string | null; }; @@ -572,7 +574,11 @@ export type ResolvedDuplicatesResponse = { // library: in the library when the list began (#5567). export type WantReason = 'request' | 'follow' | 'missing' | 'library'; // all: one artist's page, every album of theirs on the list (#5572). -export type WantState = 'wanted' | 'missing' | 'held' | 'all'; +// upgrade: held albums wanted for a better copy (#5524). +export type WantState = 'wanted' | 'missing' | 'upgrade' | 'held' | 'all'; +// Why a held album is wanted again: its copy is below the quality settled +// for, or one of its tracks is a video rip (#5524). +export type WantUpgrade = 'quality' | 'rip'; // One release group on the wanted list. held_at is set once the library // holds it; artist_id and album_id link into the library when it has them. @@ -592,6 +598,10 @@ export type Want = { wanted_at: string; held_at: string | null; image_url: string; + // A held album wanted for a better copy, and what the copy it has is + // ("MP3, about 192 kbps") (#5524). + upgrade?: WantUpgrade; + held_quality?: string; // The search schedule's last look, and the download running (#5521). search?: WantSearch | null; grab?: WantGrab | null; @@ -720,6 +730,8 @@ export type SearchSettings = { seed_ratio: number; seed_days: number; stall_hours: number; + // The tier a held album is upgraded to; "" is no quality upgrades (#5524). + upgrade_until: string; }; export type WantCounts = { @@ -728,6 +740,8 @@ export type WantCounts = { wanted: number; missing: number; held: number; + // Held albums wanted for a better copy (#5524). + upgrades: number; following: number; // Downloads running, those of them waiting on a person, releases set aside. activity: number; diff --git a/web/src/lib/api/wanted.ts b/web/src/lib/api/wanted.ts index e2b59706..ee6ca812 100644 --- a/web/src/lib/api/wanted.ts +++ b/web/src/lib/api/wanted.ts @@ -247,6 +247,7 @@ export async function putSearchSettings(body: { seed_ratio: number; seed_days: number; stall_hours: number; + upgrade_until: string; }): Promise { return api.put('/api/admin/acquisition/settings', body); } diff --git a/web/src/lib/components/SearchSettingsCard.svelte b/web/src/lib/components/SearchSettingsCard.svelte index 8a9073b5..dc4c7df9 100644 --- a/web/src/lib/components/SearchSettingsCard.svelte +++ b/web/src/lib/components/SearchSettingsCard.svelte @@ -15,7 +15,8 @@ // keeps to each indexer's searches-per-hour. A finished download is // checked and filed into one of the library's folders (#5522); it then // seeds until the ratio or the days below, and a download nobody shares - // is given up after the hours below (#5523). + // is given up after the hours below (#5523). A held album below "Upgrade + // until" is searched for weekly and replaced by a better copy (#5524). const client = useQueryClient(); const button = @@ -32,6 +33,7 @@ seed_ratio: number; seed_days: number; stall_hours: number; + upgrade_until: string; }; // The form is the server's settings until edited; saved holds what a save @@ -50,7 +52,8 @@ import_root: s.import_root, seed_ratio: s.seed_ratio, seed_days: s.seed_days, - stall_hours: s.stall_hours + stall_hours: s.stall_hours, + upgrade_until: s.upgrade_until }; } @@ -76,7 +79,8 @@ import_root: form.import_root, seed_ratio: Number(form.seed_ratio), seed_days: Number(form.seed_days), - stall_hours: Number(form.stall_hours) + stall_hours: Number(form.stall_hours), + upgrade_until: form.upgrade_until }); edited = null; await client.invalidateQueries({ queryKey: qk.searchSettings() }); @@ -155,6 +159,24 @@ {/each} + + + Upgrades + {#if data.scan_paths.length > 1}