fix: a release group whose title names a live, remix or demo version is wanted only when the profile allows that version (M502 #5573)
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m31s
release / go (push) Successful in 1m51s
release / android (push) Successful in 5m18s
release / integration (push) Successful in 5m23s
release / Build signed APK (releases and dev) (push) Successful in 5m30s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 21s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m31s
release / go (push) Successful in 1m51s
release / android (push) Successful in 5m18s
release / integration (push) Successful in 5m23s
release / Build signed APK (releases and dev) (push) Successful in 5m30s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 21s
release / Verify release artifacts (tag releases only) (push) Skipped
MusicBrainz types My Chemical Romance's "Common People (Recorded For BBC Radio 1)" as a plain Single, so a studio-only follow wanted it. Passes now reads the title's qualifier (brackets, or after a dash) for the version it names and treats it as a secondary type; a version no type names, an acoustic or instrumental one, no profile passes. The follow check takes off the list an open follow want the rule now refuses. Judge counts bbc, sessions and unplugged as live. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -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", ""},
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -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
|
||||
}
|
||||
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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)
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user