diff --git a/internal/release/match.go b/internal/release/match.go index 77ae1575..3fb25006 100644 --- a/internal/release/match.go +++ b/internal/release/match.go @@ -55,7 +55,7 @@ var versions = []struct { {[]string{"karaoke"}, "a karaoke version", ""}, {[]string{"acapella", "acappella", "accapella", "acapellas"}, "an a cappella version", ""}, {[]string{"remix", "remixes", "remixed"}, "a remix release", "Remix"}, - {[]string{"live", "bootleg"}, "a live recording", "Live"}, + {[]string{"live", "bootleg", "bbc", "sessions", "unplugged"}, "a live recording", "Live"}, {[]string{"demo", "demos"}, "demo recordings", "Demo"}, // Not "covers": in a release name that is usually the artwork scans. {[]string{"tribute", "lullaby", "lullabies"}, "a tribute release", ""}, diff --git a/internal/release/release_test.go b/internal/release/release_test.go index a5c07015..0bde6e41 100644 --- a/internal/release/release_test.go +++ b/internal/release/release_test.go @@ -62,6 +62,8 @@ func TestJudge_TheAlbumAndWhatIsNot(t *testing.T) { "Daft Punk - Random Access Memories (Instrumentals) [MP3]": VetoVersion, "Daft Punk - Random Access Memories Remixes": VetoVersion, "Daft Punk - Random Access Memories (Live) FLAC": VetoVersion, + "Daft Punk - Random Access Memories (BBC Sessions) FLAC": VetoVersion, + "Daft Punk - Random Access Memories Unplugged [MP3]": VetoVersion, "Daft Punk - Discovery (2001) [FLAC]": VetoTitle, "Random Access Memories (2013) [FLAC]": VetoArtist, "Daft Punk - Discography 1997-2013 FLAC": VetoTitle, diff --git a/internal/release/version.go b/internal/release/version.go new file mode 100644 index 00000000..c90dbf50 --- /dev/null +++ b/internal/release/version.go @@ -0,0 +1,79 @@ +package release + +import "strings" + +// TitleVersion is the version of a recording a release group's title names +// in its qualifier: the words in brackets, or after a dash. MusicBrainz +// does not always type such a group: My Chemical Romance's "Common People +// (Recorded For BBC Radio 1)" is a plain Single (#5573). Typ is the +// MusicBrainz secondary type the version is, or "" for one no type names +// (an acoustic or instrumental version); Word is the word that said so. +type TitleVersion struct { + Word string + What string + Typ string +} + +// titleVersions are the qualifier words that name a version, in the order +// they are looked for: "rough mix" is a demo before "mix" is a remix. +var titleVersions = []struct { + words []string + what string + typ string +}{ + {[]string{"demo", "demos", "outtake", "outtakes", "rehearsal", "rough mix"}, "demo recordings", "Demo"}, + {[]string{"live", "bootleg", "bbc", "session", "sessions", "unplugged", "recorded for", "in concert"}, "a live recording", "Live"}, + {[]string{"remix", "remixes", "remixed", "rmx", "mix", "mixes"}, "a remix", "Remix"}, + {[]string{"instrumental", "instrumentals", "acoustic", "karaoke", "acapella", "acappella", "a cappella", + "drumless", "nightcore", "slowed", "sped up"}, "another version", ""}, + {[]string{"commentary", "interview", "interviews"}, "commentary or interviews", "Interview"}, +} + +// VersionInTitle reads a release group's title for the version it names, +// if any. Only the qualifier counts, so "Live Through This" and "Live +// Forever" name no version. +func VersionInTitle(title string) (TitleVersion, bool) { + for _, q := range qualifiers(title) { + tokens := Tokens(q) + for _, v := range titleVersions { + for _, w := range v.words { + if contains(tokens, strings.Fields(w)) { + return TitleVersion{Word: w, What: v.what, Typ: v.typ}, true + } + } + } + } + return TitleVersion{}, false +} + +// qualifiers are the parts of a title in brackets, and what follows a +// spaced dash. +func qualifiers(title string) []string { + var out []string + depth, start := 0, 0 + for i, c := range title { + switch c { + case '(', '[', '{': + if depth == 0 { + start = i + 1 + } + depth++ + case ')', ']', '}': + if depth > 0 { + depth-- + if depth == 0 { + out = append(out, title[start:i]) + } + } + } + } + if depth > 0 { + out = append(out, title[start:]) + } + for _, dash := range []string{" - ", " – ", " — "} { + if i := strings.Index(title, dash); i >= 0 { + out = append(out, title[i+len(dash):]) + } + } + return out +} diff --git a/internal/release/version_test.go b/internal/release/version_test.go new file mode 100644 index 00000000..c6a877c9 --- /dev/null +++ b/internal/release/version_test.go @@ -0,0 +1,32 @@ +package release + +import "testing" + +func TestVersionInTitle(t *testing.T) { + for title, want := range map[string]string{ + // My Chemical Romance's groups MusicBrainz types as plain Singles (#5573). + "Common People (Recorded For BBC Radio 1)": "Live", + "Welcome to the Black Parade (Steve Aoki remix)": "Remix", + "The Five of Us Are Dying (Rough mix)": "Demo", + "I'm Not Okay (I Promise) [outtake version 2]": "Demo", + "Na Na Na (live from Sessions@AOL)": "Live", + "Song - Live at Wembley": "Live", + "Song – Club Mix": "Remix", + "Helena (Acoustic)": "", + "Teenagers (a cappella)": "", + "Hybrid Theory (20th Anniversary Edition)": "none", + "Live Through This": "none", + "Live Forever": "none", + "The Black Parade": "none", + "Warped Tour ’05: Bootleg Series (live at Warped Tour": "Live", // unclosed + } { + v, ok := VersionInTitle(title) + got := v.Typ + if !ok { + got = "none" + } + if got != want { + t.Errorf("VersionInTitle(%q) = %q (%q), want %q", title, got, v.Word, want) + } + } +} diff --git a/internal/wanted/profiles.go b/internal/wanted/profiles.go index d0a466b1..3cd86f3b 100644 --- a/internal/wanted/profiles.go +++ b/internal/wanted/profiles.go @@ -12,6 +12,7 @@ import ( "github.com/jackc/pgx/v5/pgtype" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" ) // PrimaryTypes and SecondaryTypes are MusicBrainz's release-group types, in @@ -37,7 +38,12 @@ var ( // on profile p wants: its primary type is listed, and every secondary type // it carries is allowed. A group with no secondary type is a studio release, // which every profile passes on its primary type alone. -func Passes(p dbq.ReleaseTypeProfile, primary string, secondary []string) bool { +// +// A version its title names counts as a secondary type MusicBrainz left +// off (#5573): "Common People (Recorded For BBC Radio 1)" is live whatever +// its types say, and a version no type names, an acoustic one, no profile +// passes. +func Passes(p dbq.ReleaseTypeProfile, title, primary string, secondary []string) bool { if !containsFold(p.PrimaryTypes, primary) { return false } @@ -46,6 +52,9 @@ func Passes(p dbq.ReleaseTypeProfile, primary string, secondary []string) bool { return false } } + if v, ok := release.VersionInTitle(title); ok { + return v.Typ != "" && containsFold(p.SecondaryTypes, v.Typ) + } return true } diff --git a/internal/wanted/wanted.go b/internal/wanted/wanted.go index 6f21ec0c..3efbf930 100644 --- a/internal/wanted/wanted.go +++ b/internal/wanted/wanted.go @@ -21,6 +21,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/catalogue" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" "git.fabledsword.com/bvandeusen/minstrel/internal/musicbrainz" + "git.fabledsword.com/bvandeusen/minstrel/internal/release" "git.fabledsword.com/bvandeusen/minstrel/internal/tags" ) @@ -275,7 +276,13 @@ func (s *Service) applyFollow(ctx context.Context, q *dbq.Queries, f dbq.Follow) wanted := 0 for _, g := range groups { judge := (baseline && f.WantExisting) || (!baseline && !g.Seen && s.recent(g.FirstReleaseDate, f.BaselineAt.Time)) - if judge && Passes(profile, g.PrimaryType, g.SecondaryTypes) { + passes := Passes(profile, g.Title, g.PrimaryType, g.SecondaryTypes) + if !passes { + if err := s.dropRefusedVersion(ctx, q, g.Mbid, g.Title); err != nil { + return wanted, err + } + } + if judge && passes { held, err := q.LibraryHoldsReleaseGroup(ctx, dbq.LibraryHoldsReleaseGroupParams{ GroupMbid: g.Mbid, Title: g.Title, ArtistMbid: f.ArtistMbid, }) @@ -305,6 +312,32 @@ func (s *Service) applyFollow(ctx context.Context, q *dbq.Queries, f dbq.Follow) return wanted, q.MarkFollowChecked(ctx, f.ArtistMbid) } +// dropRefusedVersion takes off the list a group the follow wanted before +// Passes read the version its title names (#5573), while it is still only +// wanted: one the operator holds, or wanted for another reason, stays. +func (s *Service) dropRefusedVersion(ctx context.Context, q *dbq.Queries, group, title string) error { + v, ok := release.VersionInTitle(title) + if !ok { + return nil + } + w, err := q.GetWantByReleaseGroup(ctx, group) + if errors.Is(err, pgx.ErrNoRows) { + return nil + } + if err != nil { + return fmt.Errorf("read want %s: %w", title, err) + } + if w.Reason != ReasonFollow || w.FulfilledAt.Valid { + return nil + } + if _, err := q.DeleteWant(ctx, w.ID); err != nil { + return fmt.Errorf("drop want %s: %w", title, err) + } + s.logger.Info("wanted: a followed artist's release is another version, taken off the list", + "want", title, "version", v.What, "word", v.Word) + return nil +} + // recent reports whether a release dated date (MusicBrainz's partial date: // "2026", "2026-10", "2026-10-09", or empty when not yet known) came out no // earlier than NewReleaseGrace before the follow's baseline. A partial date diff --git a/internal/wanted/wanted_test.go b/internal/wanted/wanted_test.go index e0f5270e..a25e9cdf 100644 --- a/internal/wanted/wanted_test.go +++ b/internal/wanted/wanted_test.go @@ -30,21 +30,29 @@ func TestPasses_PrimaryListedAndEverySecondaryAllowed(t *testing.T) { withLive := dbq.ReleaseTypeProfile{PrimaryTypes: []string{"Album"}, SecondaryTypes: []string{"Live"}} for _, c := range []struct { profile dbq.ReleaseTypeProfile + title string primary string secondary []string want bool }{ - {standard, "Album", nil, true}, - {standard, "single", nil, true}, // MusicBrainz's case is not relied on - {standard, "Album", []string{"Live"}, false}, - {standard, "Broadcast", nil, false}, - {withLive, "Album", []string{"Live"}, true}, - {withLive, "Album", []string{"Live", "Compilation"}, false}, - {withLive, "EP", nil, false}, + {standard, "Revenge", "Album", nil, true}, + {standard, "Helena", "single", nil, true}, // MusicBrainz's case is not relied on + {standard, "Live at Koko", "Album", []string{"Live"}, false}, + {standard, "Radio Show", "Broadcast", nil, false}, + {withLive, "Live at Koko", "Album", []string{"Live"}, true}, + {withLive, "Live & Rare", "Album", []string{"Live", "Compilation"}, false}, + {withLive, "Like Phantoms, Forever", "EP", nil, false}, + // MusicBrainz types these plain Singles; their titles say otherwise (#5573). + {standard, "Common People (Recorded For BBC Radio 1)", "Single", nil, false}, + {standard, "Welcome to the Black Parade (Steve Aoki remix)", "Single", nil, false}, + {standard, "The Five of Us Are Dying (Rough mix)", "Single", nil, false}, + {withLive, "Desolation Row (live at BBC Maida Vale)", "Album", nil, true}, + {withLive, "Helena (acoustic)", "Album", nil, false}, + {standard, "Live Through This", "Album", nil, true}, } { - if got := Passes(c.profile, c.primary, c.secondary); got != c.want { - t.Errorf("Passes(%v/%v, %s %v) = %v, want %v", c.profile.PrimaryTypes, c.profile.SecondaryTypes, - c.primary, c.secondary, got, c.want) + if got := Passes(c.profile, c.title, c.primary, c.secondary); got != c.want { + t.Errorf("Passes(%v/%v, %q %s %v) = %v, want %v", c.profile.PrimaryTypes, c.profile.SecondaryTypes, + c.title, c.primary, c.secondary, got, c.want) } } } @@ -177,6 +185,7 @@ const ( theBallad = "11111111-0000-4000-8000-000000000003" // 2023 album, not held newSingle = "11111111-0000-4000-8000-000000000004" // appears after the baseline oldSingle = "11111111-0000-4000-8000-000000000005" // 1997 single, catalogued late + bbcSingle = "11111111-0000-4000-8000-000000000006" // live, typed a plain Single (#5573) parkRelease = "22222222-0000-4000-8000-000000000001" ) @@ -206,12 +215,13 @@ func newFakeMB(t *testing.T) *fakeMB { case r.URL.Path == "/release-group" && r.URL.Query().Get("artist") == blur: groups := group(parklife, "Parklife", "Album", "", "1994-04-25") + "," + group(liveAtWemb, "Live at Wembley Stadium", "Album", `"Live"`, "2009-07-02") + "," + - group(theBallad, "The Ballad of Darren", "Album", "", "2023-07-21") - n := 3 + group(theBallad, "The Ballad of Darren", "Album", "", "2023-07-21") + "," + + group(bbcSingle, "Song 2 (Recorded For BBC Radio 1)", "Single", "", "2023-08-01") + n := 4 if f.added.Load() { groups += "," + group(newSingle, "New Single", "Single", "", "") + "," + group(oldSingle, "Old Single", "Single", "", "1997-02-03") - n = 5 + n = 6 } _, _ = fmt.Fprintf(w, `{"release-group-count": %d, "release-groups": [%s]}`, n, groups) case r.URL.Path == "/release-group/"+theBallad: @@ -328,6 +338,11 @@ func TestFollow_WantExistingWantsWhatPassesAndIsNotHeld_Integration(t *testing.T f := newFakeMB(t) s := newService(t, pool, f) _ = seedHeld(t, q, "Parklife", parkRelease, parklife, 2) + // Wanted before the follow read the version a title names (#5573). + if _, err := WantWith(ctx, q, WantParams{GroupMBID: bbcSingle, ArtistMBID: blur, ArtistName: "Blur", + Title: "Song 2 (Recorded For BBC Radio 1)", PrimaryType: "Single", Reason: ReasonFollow}); err != nil { + t.Fatal(err) + } if _, err := s.Follow(ctx, FollowParams{ArtistMBID: blur, ArtistName: "Blur", WantExisting: true}); err != nil { t.Fatal(err) @@ -338,7 +353,8 @@ func TestFollow_WantExistingWantsWhatPassesAndIsNotHeld_Integration(t *testing.T if _, err := q.GetWantByReleaseGroup(ctx, theBallad); err != nil { t.Errorf("the studio album not held should be wanted: %v", err) } - for group, why := range map[string]string{parklife: "held", liveAtWemb: "live, and Standard is studio only"} { + for group, why := range map[string]string{parklife: "held", liveAtWemb: "live, and Standard is studio only", + bbcSingle: "recorded for the BBC, which its title says, and Standard is studio only"} { if _, err := q.GetWantByReleaseGroup(ctx, group); err == nil { t.Errorf("%s wanted, but it is %s", group, why) }