fix: an album with missing tracks is wanted back by its release group, not only when it carries a release MBID (#6167)
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m24s
release / go (push) Successful in 1m32s
release / integration (push) Successful in 5m1s
release / android (push) Successful in 5m7s
release / Build signed APK (releases and dev) (push) Successful in 5m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
release / govulncheck (push) Successful in 17s
release / web (push) Successful in 1m24s
release / go (push) Successful in 1m32s
release / integration (push) Successful in 5m1s
release / android (push) Successful in 5m7s
release / Build signed APK (releases and dev) (push) Successful in 5m17s
release / Attach APK to the Release (tag releases only) (push) Skipped
release / Build + push container image (push) Successful in 16s
release / Verify release artifacts (tag releases only) (push) Skipped
The release id was only ever used to find the group. 238 albums matched to a group carried none, so their losses, five of the resolver's wrong-slot removals among them, were never fetched again. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -17,7 +17,7 @@ SELECT COUNT(DISTINCT albums.id)::bigint
|
|||||||
JOIN artists ON artists.id = albums.artist_id
|
JOIN artists ON artists.id = albums.artist_id
|
||||||
JOIN tracks ON tracks.album_id = albums.id
|
JOIN tracks ON tracks.album_id = albums.id
|
||||||
WHERE tracks.missing_since IS NOT NULL
|
WHERE tracks.missing_since IS NOT NULL
|
||||||
AND (albums.mbid IS NULL OR artists.mbid IS NULL)
|
AND ((albums.mbid IS NULL AND albums.release_group_mbid IS NULL) OR artists.mbid IS NULL)
|
||||||
`
|
`
|
||||||
|
|
||||||
// Albums with missing files that can never be wanted automatically because
|
// Albums with missing files that can never be wanted automatically because
|
||||||
@@ -105,7 +105,7 @@ SELECT albums.id AS album_id,
|
|||||||
JOIN tracks ON tracks.album_id = albums.id
|
JOIN tracks ON tracks.album_id = albums.id
|
||||||
WHERE tracks.missing_since IS NOT NULL
|
WHERE tracks.missing_since IS NOT NULL
|
||||||
AND tracks.missing_since <= now() - make_interval(hours => $1::int)
|
AND tracks.missing_since <= now() - make_interval(hours => $1::int)
|
||||||
AND albums.mbid IS NOT NULL
|
AND (albums.mbid IS NOT NULL OR albums.release_group_mbid IS NOT NULL)
|
||||||
AND artists.mbid IS NOT NULL
|
AND artists.mbid IS NOT NULL
|
||||||
GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid,
|
GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid,
|
||||||
artists.name, artists.mbid
|
artists.name, artists.mbid
|
||||||
@@ -141,8 +141,11 @@ type ListAlbumsDueReacquisitionRow struct {
|
|||||||
// - it still has at least one track whose file has been missing longer than
|
// - it still has at least one track whose file has been missing longer than
|
||||||
// the grace window. Measured on missing_since, which reconcile never
|
// the grace window. Measured on missing_since, which reconcile never
|
||||||
// re-stamps (#2523), so it is a genuine "gone since" clock;
|
// re-stamps (#2523), so it is a genuine "gone since" clock;
|
||||||
// - MusicBrainz can name it: the album and its artist have MBIDs. Albums
|
// - MusicBrainz can name it: the artist has an MBID, and the album a
|
||||||
// failing this are counted separately (CountAlbumsMissingWithoutMbid);
|
// release MBID or a release group. The release MBID is only ever used to
|
||||||
|
// find the group, and 238 albums matched to a group carried no release
|
||||||
|
// id, so their losses were never wanted back (#6167). Albums failing this
|
||||||
|
// are counted separately (CountAlbumsMissingWithoutMbid);
|
||||||
// - it is on the wanted list (#5567): an album taken off the list, or
|
// - it is on the wanted list (#5567): an album taken off the list, or
|
||||||
// dropped into the library by hand, is not fetched back;
|
// dropped into the library by hand, is not fetched back;
|
||||||
// - no want already covers it: none open, and none held since its files
|
// - no want already covers it: none open, and none held since its files
|
||||||
|
|||||||
@@ -21,8 +21,11 @@ RETURNING *;
|
|||||||
-- * it still has at least one track whose file has been missing longer than
|
-- * it still has at least one track whose file has been missing longer than
|
||||||
-- the grace window. Measured on missing_since, which reconcile never
|
-- the grace window. Measured on missing_since, which reconcile never
|
||||||
-- re-stamps (#2523), so it is a genuine "gone since" clock;
|
-- re-stamps (#2523), so it is a genuine "gone since" clock;
|
||||||
-- * MusicBrainz can name it: the album and its artist have MBIDs. Albums
|
-- * MusicBrainz can name it: the artist has an MBID, and the album a
|
||||||
-- failing this are counted separately (CountAlbumsMissingWithoutMbid);
|
-- release MBID or a release group. The release MBID is only ever used to
|
||||||
|
-- find the group, and 238 albums matched to a group carried no release
|
||||||
|
-- id, so their losses were never wanted back (#6167). Albums failing this
|
||||||
|
-- are counted separately (CountAlbumsMissingWithoutMbid);
|
||||||
-- * it is on the wanted list (#5567): an album taken off the list, or
|
-- * it is on the wanted list (#5567): an album taken off the list, or
|
||||||
-- dropped into the library by hand, is not fetched back;
|
-- dropped into the library by hand, is not fetched back;
|
||||||
-- * no want already covers it: none open, and none held since its files
|
-- * no want already covers it: none open, and none held since its files
|
||||||
@@ -41,7 +44,7 @@ SELECT albums.id AS album_id,
|
|||||||
JOIN tracks ON tracks.album_id = albums.id
|
JOIN tracks ON tracks.album_id = albums.id
|
||||||
WHERE tracks.missing_since IS NOT NULL
|
WHERE tracks.missing_since IS NOT NULL
|
||||||
AND tracks.missing_since <= now() - make_interval(hours => sqlc.arg(grace_hours)::int)
|
AND tracks.missing_since <= now() - make_interval(hours => sqlc.arg(grace_hours)::int)
|
||||||
AND albums.mbid IS NOT NULL
|
AND (albums.mbid IS NOT NULL OR albums.release_group_mbid IS NOT NULL)
|
||||||
AND artists.mbid IS NOT NULL
|
AND artists.mbid IS NOT NULL
|
||||||
GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid,
|
GROUP BY albums.id, albums.title, albums.mbid, albums.release_group_mbid,
|
||||||
artists.name, artists.mbid
|
artists.name, artists.mbid
|
||||||
@@ -64,7 +67,7 @@ SELECT COUNT(DISTINCT albums.id)::bigint
|
|||||||
JOIN artists ON artists.id = albums.artist_id
|
JOIN artists ON artists.id = albums.artist_id
|
||||||
JOIN tracks ON tracks.album_id = albums.id
|
JOIN tracks ON tracks.album_id = albums.id
|
||||||
WHERE tracks.missing_since IS NOT NULL
|
WHERE tracks.missing_since IS NOT NULL
|
||||||
AND (albums.mbid IS NULL OR artists.mbid IS NULL);
|
AND ((albums.mbid IS NULL AND albums.release_group_mbid IS NULL) OR artists.mbid IS NULL);
|
||||||
|
|
||||||
-- name: WantMissingAlbum :one
|
-- name: WantMissingAlbum :one
|
||||||
-- Records an album with missing files as wanted. A held want for the same
|
-- Records an album with missing files as wanted. A held want for the same
|
||||||
|
|||||||
@@ -103,7 +103,8 @@ func (s *Sweeper) SweepOnce(ctx context.Context) error {
|
|||||||
// want records one album as wanted under its release group.
|
// want records one album as wanted under its release group.
|
||||||
func (s *Sweeper) want(ctx context.Context, q *dbq.Queries, album dbq.ListAlbumsDueReacquisitionRow, res *PassResult) error {
|
func (s *Sweeper) want(ctx context.Context, q *dbq.Queries, album dbq.ListAlbumsDueReacquisitionRow, res *PassResult) error {
|
||||||
// The query already filters these out; belt and braces.
|
// The query already filters these out; belt and braces.
|
||||||
if album.AlbumMbid == nil || album.ArtistMbid == nil {
|
hasGroup := album.AlbumReleaseGroupMbid != nil && *album.AlbumReleaseGroupMbid != ""
|
||||||
|
if (album.AlbumMbid == nil && !hasGroup) || album.ArtistMbid == nil {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
group, err := s.albumReleaseGroup(ctx, q, album)
|
group, err := s.albumReleaseGroup(ctx, q, album)
|
||||||
|
|||||||
@@ -38,17 +38,21 @@ func newSweepPool(t *testing.T) *pgxpool.Pool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// lostAlbum seeds an album whose only track has been missing for two days,
|
// lostAlbum seeds an album whose only track has been missing for two days,
|
||||||
// named by MusicBrainz release id and (when group is set) release group.
|
// named by MusicBrainz release id (when release is set) and release group
|
||||||
|
// (when group is set).
|
||||||
func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) pgtype.UUID {
|
func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) pgtype.UUID {
|
||||||
t.Helper()
|
t.Helper()
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
q := dbq.New(pool)
|
q := dbq.New(pool)
|
||||||
artistMBID := "artist-" + release
|
artistMBID := "artist-" + title
|
||||||
ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: title + " Artist", SortName: title, Mbid: &artistMBID})
|
ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: title + " Artist", SortName: title, Mbid: &artistMBID})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
params := dbq.UpsertAlbumParams{Title: title, SortTitle: title, ArtistID: ar.ID, Mbid: &release}
|
params := dbq.UpsertAlbumParams{Title: title, SortTitle: title, ArtistID: ar.ID}
|
||||||
|
if release != "" {
|
||||||
|
params.Mbid = &release
|
||||||
|
}
|
||||||
if group != "" {
|
if group != "" {
|
||||||
params.ReleaseGroupMbid = &group
|
params.ReleaseGroupMbid = &group
|
||||||
}
|
}
|
||||||
@@ -58,7 +62,7 @@ func lostAlbum(t *testing.T, pool *pgxpool.Pool, title, release, group string) p
|
|||||||
}
|
}
|
||||||
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
||||||
Title: "Lost", AlbumID: al.ID, ArtistID: ar.ID, DurationMs: 180000,
|
Title: "Lost", AlbumID: al.ID, ArtistID: ar.ID, DurationMs: 180000,
|
||||||
FilePath: "/music/" + release + "/01.flac", FileSize: 1, FileFormat: "flac",
|
FilePath: "/music/" + title + "/01.flac", FileSize: 1, FileFormat: "flac",
|
||||||
})
|
})
|
||||||
if err != nil {
|
if err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
@@ -99,7 +103,11 @@ func TestSweep_WantsBackOnlyAlbumsOnTheList_Integration(t *testing.T) {
|
|||||||
resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "")
|
resolved := lostAlbum(t, pool, "Resolved", "release-resolved", "")
|
||||||
unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "")
|
unknown := lostAlbum(t, pool, "Unknown", "release-unknown", "")
|
||||||
_ = lostAlbum(t, pool, "Off The List", "release-off", "group-off")
|
_ = lostAlbum(t, pool, "Off The List", "release-off", "group-off")
|
||||||
|
// Matched to a group with no release id: 238 such albums on the operator's
|
||||||
|
// library were never wanted back (#6167).
|
||||||
|
groupOnly := lostAlbum(t, pool, "Group Only", "", "group-only")
|
||||||
heldWant(t, pool, "group-tagged", tagged, "3 days")
|
heldWant(t, pool, "group-tagged", tagged, "3 days")
|
||||||
|
heldWant(t, pool, "group-only", groupOnly, "3 days")
|
||||||
heldWant(t, pool, "group-resolved", resolved, "3 days")
|
heldWant(t, pool, "group-resolved", resolved, "3 days")
|
||||||
heldWant(t, pool, "group-unknown", unknown, "3 days")
|
heldWant(t, pool, "group-unknown", unknown, "3 days")
|
||||||
|
|
||||||
@@ -128,6 +136,7 @@ func TestSweep_WantsBackOnlyAlbumsOnTheList_Integration(t *testing.T) {
|
|||||||
}
|
}
|
||||||
wantFor("group-tagged", tagged)
|
wantFor("group-tagged", tagged)
|
||||||
wantFor("group-resolved", resolved)
|
wantFor("group-resolved", resolved)
|
||||||
|
wantFor("group-only", groupOnly)
|
||||||
if w, _ := q.GetWantByReleaseGroup(ctx, "group-unknown"); !w.FulfilledAt.Valid {
|
if w, _ := q.GetWantByReleaseGroup(ctx, "group-unknown"); !w.FulfilledAt.Valid {
|
||||||
t.Error("an album MusicBrainz cannot name was reopened under the wrong group")
|
t.Error("an album MusicBrainz cannot name was reopened under the wrong group")
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user