fix: a file tagged with no release group joins the same-named album its folder holds
release / govulncheck (push) Successful in 16s
release / web (push) Successful in 1m20s
release / go (push) Successful in 1m31s
release / integration (push) Successful in 4m58s
release / android (push) Successful in 5m1s
release / Build signed APK (releases and dev) (push) Successful in 5m14s
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 16s
release / web (push) Successful in 1m20s
release / go (push) Successful in 1m31s
release / integration (push) Successful in 4m58s
release / android (push) Successful in 5m1s
release / Build signed APK (releases and dev) (push) Successful in 5m14s
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
Under e45561e2 a file with no group tag took the oldest same-named album row,
which left three of Weezer's 1994 bonus tracks on the 2016 album. The lookup
now ranks same-named rows by how many tracks they hold from the file's
folder; tagReadVersion 8 re-places the existing ones.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -72,11 +72,14 @@ func (q *Queries) DeleteAlbumIfEmpty(ctx context.Context, id pgtype.UUID) (Delet
|
|||||||
}
|
}
|
||||||
|
|
||||||
const getAlbumByArtistAndTitle = `-- name: GetAlbumByArtistAndTitle :one
|
const getAlbumByArtistAndTitle = `-- name: GetAlbumByArtistAndTitle :one
|
||||||
SELECT id, title, sort_title, artist_id, release_date, mbid, cover_art_path, created_at, updated_at, cover_art_source, cover_art_sources_version, release_group_mbid FROM albums
|
SELECT albums.id, albums.title, albums.sort_title, albums.artist_id, albums.release_date, albums.mbid, albums.cover_art_path, albums.created_at, albums.updated_at, albums.cover_art_source, albums.cover_art_sources_version, albums.release_group_mbid FROM albums
|
||||||
WHERE artist_id = $1 AND title = $2
|
WHERE albums.artist_id = $1 AND albums.title = $2
|
||||||
AND ($3::text IS NULL OR release_group_mbid IS NULL
|
AND ($3::text IS NULL OR albums.release_group_mbid IS NULL
|
||||||
OR release_group_mbid = $3)
|
OR albums.release_group_mbid = $3)
|
||||||
ORDER BY (release_group_mbid IS NOT DISTINCT FROM $3) DESC, id
|
ORDER BY (albums.release_group_mbid IS NOT DISTINCT FROM $3) DESC,
|
||||||
|
(SELECT count(*) FROM tracks t
|
||||||
|
WHERE t.album_id = albums.id AND starts_with(t.file_path, $4::text || '/')) DESC,
|
||||||
|
albums.id
|
||||||
LIMIT 1
|
LIMIT 1
|
||||||
`
|
`
|
||||||
|
|
||||||
@@ -84,15 +87,24 @@ type GetAlbumByArtistAndTitleParams struct {
|
|||||||
ArtistID pgtype.UUID
|
ArtistID pgtype.UUID
|
||||||
Title string
|
Title string
|
||||||
ReleaseGroupMbid *string
|
ReleaseGroupMbid *string
|
||||||
|
Dir string
|
||||||
}
|
}
|
||||||
|
|
||||||
// Scanner uses this for the no-mbid dedupe path: resolve-or-create. An album
|
// Scanner uses this for the no-mbid dedupe path: resolve-or-create. An album
|
||||||
// of the same name in another release group is a different album (Weezer's
|
// of the same name in another release group is a different album (Weezer's
|
||||||
// six self-titled albums shared one row until 2026-10-20), so a row whose
|
// six self-titled albums shared one row until 2026-10-20), so a row whose
|
||||||
// group is known and differs is not returned. A row of the file's own group
|
// group is known and differs is not returned. A row of the file's own group
|
||||||
// comes first, then one with no group yet.
|
// comes first, then one with no group yet, then the one holding the most
|
||||||
|
// tracks from the file's folder: a file tagged with no group (a live bonus
|
||||||
|
// track beside Weezer's 1994 album) belongs with its folder, not the oldest
|
||||||
|
// row, and a few strays already misfiled do not outvote the folder.
|
||||||
func (q *Queries) GetAlbumByArtistAndTitle(ctx context.Context, arg GetAlbumByArtistAndTitleParams) (Album, error) {
|
func (q *Queries) GetAlbumByArtistAndTitle(ctx context.Context, arg GetAlbumByArtistAndTitleParams) (Album, error) {
|
||||||
row := q.db.QueryRow(ctx, getAlbumByArtistAndTitle, arg.ArtistID, arg.Title, arg.ReleaseGroupMbid)
|
row := q.db.QueryRow(ctx, getAlbumByArtistAndTitle,
|
||||||
|
arg.ArtistID,
|
||||||
|
arg.Title,
|
||||||
|
arg.ReleaseGroupMbid,
|
||||||
|
arg.Dir,
|
||||||
|
)
|
||||||
var i Album
|
var i Album
|
||||||
err := row.Scan(
|
err := row.Scan(
|
||||||
&i.ID,
|
&i.ID,
|
||||||
|
|||||||
@@ -40,12 +40,18 @@ ORDER BY release_date NULLS LAST, sort_title;
|
|||||||
-- of the same name in another release group is a different album (Weezer's
|
-- of the same name in another release group is a different album (Weezer's
|
||||||
-- six self-titled albums shared one row until 2026-10-20), so a row whose
|
-- six self-titled albums shared one row until 2026-10-20), so a row whose
|
||||||
-- group is known and differs is not returned. A row of the file's own group
|
-- group is known and differs is not returned. A row of the file's own group
|
||||||
-- comes first, then one with no group yet.
|
-- comes first, then one with no group yet, then the one holding the most
|
||||||
SELECT * FROM albums
|
-- tracks from the file's folder: a file tagged with no group (a live bonus
|
||||||
WHERE artist_id = sqlc.arg(artist_id) AND title = sqlc.arg(title)
|
-- track beside Weezer's 1994 album) belongs with its folder, not the oldest
|
||||||
AND (sqlc.narg(release_group_mbid)::text IS NULL OR release_group_mbid IS NULL
|
-- row, and a few strays already misfiled do not outvote the folder.
|
||||||
OR release_group_mbid = sqlc.narg(release_group_mbid))
|
SELECT albums.* FROM albums
|
||||||
ORDER BY (release_group_mbid IS NOT DISTINCT FROM sqlc.narg(release_group_mbid)) DESC, id
|
WHERE albums.artist_id = sqlc.arg(artist_id) AND albums.title = sqlc.arg(title)
|
||||||
|
AND (sqlc.narg(release_group_mbid)::text IS NULL OR albums.release_group_mbid IS NULL
|
||||||
|
OR albums.release_group_mbid = sqlc.narg(release_group_mbid))
|
||||||
|
ORDER BY (albums.release_group_mbid IS NOT DISTINCT FROM sqlc.narg(release_group_mbid)) DESC,
|
||||||
|
(SELECT count(*) FROM tracks t
|
||||||
|
WHERE t.album_id = albums.id AND starts_with(t.file_path, sqlc.arg(dir)::text || '/')) DESC,
|
||||||
|
albums.id
|
||||||
LIMIT 1;
|
LIMIT 1;
|
||||||
|
|
||||||
-- name: ListAlbumsByArtist :many
|
-- name: ListAlbumsByArtist :many
|
||||||
|
|||||||
@@ -78,7 +78,10 @@ var audioExtensions = map[string]bool{
|
|||||||
// (resolveAlbum). Weezer's six self-titled albums were one 118-track row,
|
// (resolveAlbum). Weezer's six self-titled albums were one 118-track row,
|
||||||
// so their merges, wants and upgrades all counted as one album; a re-read
|
// so their merges, wants and upgrades all counted as one album; a re-read
|
||||||
// files each existing track under its own album.
|
// files each existing track under its own album.
|
||||||
const tagReadVersion int16 = 7
|
// 8: a file tagged with no release group joins the same-named album that
|
||||||
|
// already holds a track from its folder. Under 7 it took the oldest row,
|
||||||
|
// which left three of Weezer's 1994 bonus tracks on the 2016 album.
|
||||||
|
const tagReadVersion int16 = 8
|
||||||
|
|
||||||
type Stats struct {
|
type Stats struct {
|
||||||
Scanned int `json:"scanned"`
|
Scanned int `json:"scanned"`
|
||||||
@@ -319,7 +322,7 @@ func (s *Scanner) scanFile(
|
|||||||
if err != nil {
|
if err != nil {
|
||||||
return pgtype.UUID{}, false, fmt.Errorf("artist: %w", err)
|
return pgtype.UUID{}, false, fmt.Errorf("artist: %w", err)
|
||||||
}
|
}
|
||||||
album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID, releaseGroupMBID)
|
album, err := s.resolveAlbum(ctx, q, artist.ID, albumTitle, meta.Year(), albumMBID, releaseGroupMBID, filepath.Dir(path))
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return pgtype.UUID{}, false, fmt.Errorf("album: %w", err)
|
return pgtype.UUID{}, false, fmt.Errorf("album: %w", err)
|
||||||
}
|
}
|
||||||
@@ -565,8 +568,8 @@ func (s *Scanner) resolveArtist(ctx context.Context, q *dbq.Queries, name, mbid
|
|||||||
return artist, nil
|
return artist, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid, releaseGroupMBID string) (dbq.Album, error) {
|
func (s *Scanner) resolveAlbum(ctx context.Context, q *dbq.Queries, artistID pgtype.UUID, title string, year int, mbid, releaseGroupMBID, dir string) (dbq.Album, error) {
|
||||||
lookup := dbq.GetAlbumByArtistAndTitleParams{ArtistID: artistID, Title: title}
|
lookup := dbq.GetAlbumByArtistAndTitleParams{ArtistID: artistID, Title: title, Dir: dir}
|
||||||
if releaseGroupMBID != "" {
|
if releaseGroupMBID != "" {
|
||||||
rg := releaseGroupMBID
|
rg := releaseGroupMBID
|
||||||
lookup.ReleaseGroupMbid = &rg
|
lookup.ReleaseGroupMbid = &rg
|
||||||
|
|||||||
@@ -459,20 +459,29 @@ func TestScanner_SameTitledAlbumsOfTwoGroupsAreTwoAlbums_Integration(t *testing.
|
|||||||
t.Fatalf("albums by group = %v, want blue with 2 tracks and green with 1", got)
|
t.Fatalf("albums by group = %v, want blue with 2 tracks and green with 1", got)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A bonus track tagged with no release group joins the album its folder
|
||||||
|
// holds, whichever row is older.
|
||||||
|
writeTestMP3(t, filepath.Join(root, "Weezer (2001)/02.mp3"), map[string]string{
|
||||||
|
"TIT2": "Photograph (live)", "TPE1": "Weezer", "TALB": "Weezer",
|
||||||
|
})
|
||||||
|
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||||
|
t.Fatalf("scan the bonus track: %v", err)
|
||||||
|
}
|
||||||
|
if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 2 {
|
||||||
|
t.Fatalf("albums by group = %v, want the ungrouped 2001 bonus track on green", got)
|
||||||
|
}
|
||||||
|
|
||||||
// The green track as the old lookup left it: on the blue album's row,
|
// The green track as the old lookup left it: on the blue album's row,
|
||||||
// under an older tag-read version, its own album row gone.
|
// under an older tag-read version.
|
||||||
if _, err := pool.Exec(ctx, `
|
if _, err := pool.Exec(ctx, `
|
||||||
WITH b AS (SELECT id FROM albums WHERE release_group_mbid = $1)
|
WITH b AS (SELECT id FROM albums WHERE release_group_mbid = $1)
|
||||||
UPDATE tracks SET album_id = (SELECT id FROM b), tag_read_version = 6 WHERE file_path = $2`, blue, greenPath); err != nil {
|
UPDATE tracks SET album_id = (SELECT id FROM b), tag_read_version = 6 WHERE file_path = $2`, blue, greenPath); err != nil {
|
||||||
t.Fatal(err)
|
t.Fatal(err)
|
||||||
}
|
}
|
||||||
if _, err := pool.Exec(ctx, `DELETE FROM albums WHERE release_group_mbid = $1`, green); err != nil {
|
|
||||||
t.Fatal(err)
|
|
||||||
}
|
|
||||||
if _, err := scanner.Scan(ctx, nil); err != nil {
|
if _, err := scanner.Scan(ctx, nil); err != nil {
|
||||||
t.Fatalf("rescan: %v", err)
|
t.Fatalf("rescan: %v", err)
|
||||||
}
|
}
|
||||||
if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 1 {
|
if got := albumsByGroup(); len(got) != 2 || got[blue] != 2 || got[green] != 2 {
|
||||||
t.Errorf("after the re-read, albums by group = %v, want the green track on its own album", got)
|
t.Errorf("after the re-read, albums by group = %v, want the green track on its own album", got)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user