feat: Collection › Unmatched lists the library albums no MusicBrainz album matched, each matched by hand from a search or a link, or left off (M502 #5579)
release / govulncheck (push) Successful in 16s
release / web (push) Successful in 1m17s
release / go (push) Successful in 1m34s
release / integration (push) Failing after 5m34s
release / android (push) Successful in 5m36s
release / Build signed APK (releases and dev) (push) Successful in 5m44s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 16s
release / web (push) Successful in 1m17s
release / go (push) Successful in 1m34s
release / integration (push) Failing after 5m34s
release / android (push) Successful in 5m36s
release / Build signed APK (releases and dev) (push) Successful in 5m44s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Skipped
release / Verify release artifacts (tag releases only) (push) Skipped
Like Lidarr's Unmapped Files. A local band's album, which MusicBrainz will never have, is left off: never matched again and not counted. A re-tagged album is matched again on the next listing. The Collection's verdict now says when library albums need matching. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -50,7 +50,10 @@ const libraryMatcher = 4
|
||||
//
|
||||
// Each album decided is recorded in library_album_matches, so an album taken
|
||||
// off the list stays off. Once done the listing runs again only for albums
|
||||
// an older matcher refused (libraryMatcher).
|
||||
// an older matcher refused (libraryMatcher), refused albums whose tracks
|
||||
// have changed since, and library tracks a re-tag filed under a new album
|
||||
// (#5579). A person matches the rest by hand, or leaves them off, in the
|
||||
// Collection's Unmatched section (MatchAlbumTo, LeaveAlbumOff).
|
||||
func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
||||
q := dbq.New(s.pool)
|
||||
state, err := q.GetLibraryListing(ctx)
|
||||
@@ -58,9 +61,10 @@ func (s *Service) listLibrary(ctx context.Context, res *PassResult) error {
|
||||
return fmt.Errorf("wanted: read the library listing: %w", err)
|
||||
}
|
||||
if state.LibraryListedAt.Valid {
|
||||
stale, err := q.CountAlbumsToRematch(ctx, libraryMatcher)
|
||||
stale, err := q.CountAlbumsToMatch(ctx, dbq.CountAlbumsToMatchParams{
|
||||
Cutoff: state.LibraryListingStartedAt, Matcher: libraryMatcher})
|
||||
if err != nil {
|
||||
return fmt.Errorf("wanted: count refusals to retry: %w", err)
|
||||
return fmt.Errorf("wanted: count albums to match again: %w", err)
|
||||
}
|
||||
if stale == 0 {
|
||||
return nil
|
||||
|
||||
@@ -0,0 +1,171 @@
|
||||
package wanted
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"fmt"
|
||||
"regexp"
|
||||
"strings"
|
||||
|
||||
"github.com/jackc/pgx/v5"
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
// Matching a library album by hand (#5579): the Collection's Unmatched
|
||||
// section, like Lidarr's "Unmapped Files". A person names the MusicBrainz
|
||||
// album an album is, by a link or a search, and it goes on the list as held;
|
||||
// or leaves it off for good, as a local band's album MusicBrainz will never
|
||||
// have.
|
||||
|
||||
const matchManual = "manual"
|
||||
|
||||
// Manual-match errors, which the API answers as 400, 404 or 409.
|
||||
var (
|
||||
ErrMatchRef = errors.New("wanted: not a MusicBrainz album link or id")
|
||||
ErrMatchNoGroup = errors.New("wanted: MusicBrainz has no such album")
|
||||
ErrAlbumNotUnmatched = errors.New("wanted: no such unmatched album")
|
||||
)
|
||||
|
||||
// ManualMatch is what matching an album by hand did: the group it is now,
|
||||
// and how many of its tracks agree with the group's closest edition, so the
|
||||
// person can see whether the link was the right one.
|
||||
type ManualMatch struct {
|
||||
Group string
|
||||
Title string
|
||||
Artist string
|
||||
Agree int
|
||||
Tracks int
|
||||
}
|
||||
|
||||
var mbidPattern = regexp.MustCompile(`(?i)(?:(release-group|release)/)?([0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12})`)
|
||||
|
||||
// ParseMusicBrainzRef reads a MusicBrainz link (a release group's or a
|
||||
// release's page) or a bare id. Kind is "release-group", "release", or ""
|
||||
// for a bare id, which is tried as a group first.
|
||||
func ParseMusicBrainzRef(ref string) (kind, id string, ok bool) {
|
||||
m := mbidPattern.FindStringSubmatch(strings.TrimSpace(ref))
|
||||
if m == nil {
|
||||
return "", "", false
|
||||
}
|
||||
return strings.ToLower(m[1]), strings.ToLower(m[2]), true
|
||||
}
|
||||
|
||||
// MatchAlbumTo puts an unmatched (or left-off) library album on the list as
|
||||
// held, as the MusicBrainz album ref names. The person's word is taken: the
|
||||
// tracks are compared only to say how well they agree.
|
||||
func (s *Service) MatchAlbumTo(ctx context.Context, albumID pgtype.UUID, ref string) (ManualMatch, error) {
|
||||
kind, id, ok := ParseMusicBrainzRef(ref)
|
||||
if !ok {
|
||||
return ManualMatch{}, ErrMatchRef
|
||||
}
|
||||
q := dbq.New(s.pool)
|
||||
a, err := q.GetAlbumToMatch(ctx, albumID)
|
||||
if errors.Is(err, pgx.ErrNoRows) {
|
||||
return ManualMatch{}, ErrAlbumNotUnmatched
|
||||
}
|
||||
if err != nil {
|
||||
return ManualMatch{}, fmt.Errorf("wanted: read album: %w", err)
|
||||
}
|
||||
group, err := s.groupOf(ctx, kind, id)
|
||||
if err != nil {
|
||||
return ManualMatch{}, err
|
||||
}
|
||||
g, err := s.cat.MusicBrainz().ReleaseGroup(ctx, group)
|
||||
if errors.Is(err, musicbrainz.ErrNotFound) {
|
||||
return ManualMatch{}, ErrMatchNoGroup
|
||||
}
|
||||
if err != nil {
|
||||
return ManualMatch{}, err
|
||||
}
|
||||
artist := creditedArtist(g.ArtistMBIDs, deref(a.ArtistMbid))
|
||||
if artist == "" {
|
||||
return ManualMatch{}, fmt.Errorf("%w: it credits no artist", ErrMatchNoGroup)
|
||||
}
|
||||
rows, err := q.ListAlbumTracksToMatch(ctx, a.ID)
|
||||
if err != nil {
|
||||
return ManualMatch{}, fmt.Errorf("wanted: read %s's tracks: %w", a.Title, err)
|
||||
}
|
||||
tracks := tracksToMatch(rows)
|
||||
n, _, _, err := s.agreementWith(ctx, group, tracks, func(string) bool { return true })
|
||||
if err != nil {
|
||||
return ManualMatch{}, err
|
||||
}
|
||||
err = pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error {
|
||||
tq := dbq.New(tx)
|
||||
if err := tq.SetAlbumReleaseGroup(ctx, dbq.SetAlbumReleaseGroupParams{ID: a.ID, ReleaseGroupMbid: &group}); err != nil {
|
||||
return err
|
||||
}
|
||||
if _, err := tq.InsertLibraryWant(ctx, dbq.InsertLibraryWantParams{
|
||||
ReleaseGroupMbid: group, ArtistMbid: artist, ArtistName: g.ArtistCredit, Title: g.Title,
|
||||
PrimaryType: g.PrimaryType, FirstReleaseDate: g.FirstReleaseDate, AlbumID: a.ID,
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
if err := tq.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
||||
AlbumID: a.ID, Outcome: "listed", Method: matchManual, ReleaseGroupMbid: group, Matcher: libraryMatcher,
|
||||
}); err != nil {
|
||||
return err
|
||||
}
|
||||
return tq.RecountLibraryUnlisted(ctx)
|
||||
})
|
||||
if err != nil {
|
||||
return ManualMatch{}, fmt.Errorf("wanted: match %s by hand: %w", a.Title, err)
|
||||
}
|
||||
s.logger.Info("wanted: a library album matched by hand", "album", a.Title, "group", group, "agree", max(n, 0), "tracks", len(tracks))
|
||||
return ManualMatch{Group: group, Title: g.Title, Artist: g.ArtistCredit, Agree: max(n, 0), Tracks: len(tracks)}, nil
|
||||
}
|
||||
|
||||
// groupOf names the release group a MusicBrainz id is, or belongs to.
|
||||
func (s *Service) groupOf(ctx context.Context, kind, id string) (string, error) {
|
||||
if kind != "release" {
|
||||
_, err := s.cat.MusicBrainz().ReleaseGroup(ctx, id)
|
||||
if err == nil {
|
||||
return id, nil
|
||||
}
|
||||
if !errors.Is(err, musicbrainz.ErrNotFound) {
|
||||
return "", err
|
||||
}
|
||||
if kind == "release-group" {
|
||||
return "", ErrMatchNoGroup
|
||||
}
|
||||
}
|
||||
group, err := s.releaseGroup(ctx, id)
|
||||
switch {
|
||||
case errors.Is(err, tags.ErrNotFound):
|
||||
return "", ErrMatchNoGroup
|
||||
case err != nil:
|
||||
return "", err
|
||||
case group == "":
|
||||
return "", ErrMatchNoGroup
|
||||
}
|
||||
return group, nil
|
||||
}
|
||||
|
||||
// LeaveAlbumOff leaves an unmatched library album off the list for good:
|
||||
// matching never tries it again, and it is no longer counted as unmatched.
|
||||
func (s *Service) LeaveAlbumOff(ctx context.Context, albumID pgtype.UUID) error {
|
||||
return s.setMatchOutcome(ctx, albumID, "unmatched", "dismissed")
|
||||
}
|
||||
|
||||
// PutAlbumBack undoes LeaveAlbumOff: the album is unmatched again.
|
||||
func (s *Service) PutAlbumBack(ctx context.Context, albumID pgtype.UUID) error {
|
||||
return s.setMatchOutcome(ctx, albumID, "dismissed", "unmatched")
|
||||
}
|
||||
|
||||
func (s *Service) setMatchOutcome(ctx context.Context, albumID pgtype.UUID, was, outcome string) error {
|
||||
return pgx.BeginFunc(ctx, s.pool, func(tx pgx.Tx) error {
|
||||
tq := dbq.New(tx)
|
||||
n, err := tq.SetAlbumMatchOutcome(ctx, dbq.SetAlbumMatchOutcomeParams{AlbumID: albumID, Was: was, Outcome: outcome})
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
if n == 0 {
|
||||
return ErrAlbumNotUnmatched
|
||||
}
|
||||
return tq.RecountLibraryUnlisted(ctx)
|
||||
})
|
||||
}
|
||||
@@ -0,0 +1,162 @@
|
||||
package wanted
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/tags"
|
||||
)
|
||||
|
||||
func TestParseMusicBrainzRef(t *testing.T) {
|
||||
const id = "11111111-0000-4000-8000-000000000003"
|
||||
for ref, want := range map[string]string{
|
||||
"https://musicbrainz.org/release-group/" + id: "release-group " + id,
|
||||
"https://musicbrainz.org/release/" + id + "?tab=x": "release " + id,
|
||||
" " + id + " ": " " + id,
|
||||
"https://MusicBrainz.org/Release-Group/" + "11111111-0000-4000-8000-00000000000A": "release-group 11111111-0000-4000-8000-00000000000a",
|
||||
} {
|
||||
kind, got, ok := ParseMusicBrainzRef(ref)
|
||||
if !ok || kind+" "+got != want {
|
||||
t.Errorf("ParseMusicBrainzRef(%q) = %q %q %v, want %q", ref, kind, got, ok, want)
|
||||
}
|
||||
}
|
||||
for _, bad := range []string{"", "Dial Tone", "https://musicbrainz.org/artist/nope"} {
|
||||
if _, _, ok := ParseMusicBrainzRef(bad); ok {
|
||||
t.Errorf("ParseMusicBrainzRef(%q) read an id", bad)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// A person names the MusicBrainz album an unmatched album is, and it goes
|
||||
// on the list as held; or leaves it off, and matching never tries it again
|
||||
// (#5579).
|
||||
func TestMatchAlbumTo_ByLinkOrLeaveItOff_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
s := newService(t, pool, newFakeMB(t))
|
||||
s.releaseGroup = func(context.Context, string) (string, error) { return "", tags.ErrNotFound }
|
||||
split := seedAlbum(t, q, blur, "Blur + Friends", "", []string{"The Ballad", "St. Charles Square"})
|
||||
local := seedAlbum(t, q, blur, "Dial Tone", "", []string{"Intro"})
|
||||
for _, a := range []pgtype.UUID{split.ID, local.ID} {
|
||||
if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
||||
AlbumID: a, Outcome: "unmatched", Detail: "no match", Matcher: libraryMatcher}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
}
|
||||
if _, err := pool.Exec(ctx, `UPDATE acquisition_settings
|
||||
SET library_listing_started_at = now(), library_listed_at = now(), library_unlisted = 2`); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
for ref, want := range map[string]error{
|
||||
"not a link": ErrMatchRef,
|
||||
"https://musicbrainz.org/release-group/99999999-0000-4000-8000-000000000000": ErrMatchNoGroup,
|
||||
"99999999-0000-4000-8000-000000000000": ErrMatchNoGroup,
|
||||
} {
|
||||
if _, err := s.MatchAlbumTo(ctx, split.ID, ref); !errors.Is(err, want) {
|
||||
t.Errorf("MatchAlbumTo(%q) = %v, want %v", ref, err, want)
|
||||
}
|
||||
}
|
||||
|
||||
m, err := s.MatchAlbumTo(ctx, split.ID, "https://musicbrainz.org/release-group/"+theBallad)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if m.Group != theBallad || m.Title != "The Ballad of Darren" || m.Agree != 2 || m.Tracks != 2 {
|
||||
t.Errorf("match = %+v", m)
|
||||
}
|
||||
w, err := q.GetWantByReleaseGroup(ctx, theBallad)
|
||||
if err != nil || w.Reason != ReasonLibrary || !w.FulfilledAt.Valid || w.AlbumID != split.ID || w.ArtistMbid != blur {
|
||||
t.Fatalf("want = %+v, %v: held, about the album", w, err)
|
||||
}
|
||||
if outcome, method, group, _ := matchOf(t, pool, split.ID); outcome != "listed" || method != matchManual || group != theBallad {
|
||||
t.Errorf("decided %s by %s as %s", outcome, method, group)
|
||||
}
|
||||
if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 1 {
|
||||
t.Errorf("unlisted = %d, want the local album alone", state.LibraryUnlisted)
|
||||
}
|
||||
|
||||
// The local band's album: left off, it is not counted, and a newer
|
||||
// matcher does not try it again.
|
||||
if err := s.LeaveAlbumOff(ctx, local.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "dismissed" {
|
||||
t.Errorf("left off = %s", outcome)
|
||||
}
|
||||
if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 0 {
|
||||
t.Errorf("unlisted = %d after leaving it off", state.LibraryUnlisted)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, `UPDATE library_album_matches SET matcher = 0 WHERE album_id = $1`, local.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := s.listLibrary(ctx, &PassResult{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "dismissed" {
|
||||
t.Errorf("an album left off was matched again: %s", outcome)
|
||||
}
|
||||
rows, err := q.ListUnmatchedLibraryAlbums(ctx, 10)
|
||||
if err != nil || len(rows) != 1 || rows[0].Outcome != "dismissed" || rows[0].Tracks != 1 {
|
||||
t.Errorf("unmatched section = %+v, %v: the left-off album alone", rows, err)
|
||||
}
|
||||
|
||||
// Put back, it is unmatched again; putting it back twice is not.
|
||||
if err := s.PutAlbumBack(ctx, local.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if outcome, _, _, _ := matchOf(t, pool, local.ID); outcome != "unmatched" {
|
||||
t.Errorf("put back = %s", outcome)
|
||||
}
|
||||
if err := s.PutAlbumBack(ctx, local.ID); !errors.Is(err, ErrAlbumNotUnmatched) {
|
||||
t.Errorf("put back twice: %v", err)
|
||||
}
|
||||
}
|
||||
|
||||
// A re-tag files a library track under a new album row. That album is the
|
||||
// library's, not one dropped in by hand, so the listing matches it; the
|
||||
// old row, its tracks gone, is no longer unmatched (#5579).
|
||||
func TestListLibrary_MatchesARetaggedAlbum_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
s := newService(t, pool, newFakeMB(t))
|
||||
untagged := seedAlbum(t, q, blur, "Unknown Album", "", []string{"The Ballad", "St. Charles Square"})
|
||||
if err := q.RecordAlbumMatch(ctx, dbq.RecordAlbumMatchParams{
|
||||
AlbumID: untagged.ID, Outcome: "unmatched", Detail: "no match", Matcher: libraryMatcher}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, `
|
||||
UPDATE albums SET created_at = now() - interval '2 hours' WHERE id = $1;
|
||||
UPDATE tracks SET added_at = now() - interval '2 hours' WHERE album_id = $1;
|
||||
UPDATE acquisition_settings SET library_listing_started_at = now() - interval '1 hour',
|
||||
library_listed_at = now() - interval '1 hour', library_unlisted = 1`, untagged.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// The re-tag: the same tracks, under the album their new tags name.
|
||||
retagged, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: "The Ballad of Darren", SortTitle: "The Ballad of Darren", ArtistID: untagged.ArtistID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if _, err := pool.Exec(ctx, `UPDATE tracks SET album_id = $1, updated_at = now() WHERE album_id = $2`, retagged.ID, untagged.ID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
|
||||
if err := s.listLibrary(ctx, &PassResult{}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if outcome, method, group, _ := matchOf(t, pool, retagged.ID); outcome != "listed" || method != matchDiscography || group != theBallad {
|
||||
t.Errorf("the re-tagged album was decided %s by %s as %s", outcome, method, group)
|
||||
}
|
||||
if rows, _ := q.ListUnmatchedLibraryAlbums(ctx, 10); len(rows) != 0 {
|
||||
t.Errorf("unmatched = %+v: the old row has no tracks left", rows)
|
||||
}
|
||||
if state, _ := q.GetLibraryListing(ctx); state.LibraryUnlisted != 0 {
|
||||
t.Errorf("unlisted = %d", state.LibraryUnlisted)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user