feat: the same song on several releases counts as one song (M498 #5438)
release / govulncheck (push) Successful in 21s
release / web (push) Successful in 1m7s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 4m11s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m58s
release / Build signed APK (releases and dev) (push) Canceled after 5m59s
release / govulncheck (push) Successful in 21s
release / web (push) Successful in 1m7s
release / go (push) Successful in 1m29s
release / integration (push) Successful in 4m11s
release / Attach APK to the Release (tag releases only) (push) Canceled after 0s
release / Build + push container image (push) Canceled after 0s
release / Verify release artifacts (tag releases only) (push) Canceled after 0s
release / android (push) Canceled after 5m58s
release / Build signed APK (releases and dev) (push) Canceled after 5m59s
A single and the album it is on stay two files, since each fulfils its own
release in Lidarr, but they are one song to the listener.
- tracks.song_id links copies; the generated song_key (song_id, else the
track's own id) is what they share (migration 0076).
- The resolver links each cross-release group every pass (idempotent; only
with auto-resolve on) and, when a link is new, shares existing likes across
the song and logs them for sync.
- A like or unlike (web and Subsonic) reaches every copy; each change is
logged and published so clients update every heart.
- The Liked list and its count show the song once; Shuffle and the mix writer
take one copy per song.
- A merge keeps the removed copy's song link; "Not the same song" on the
Across releases tab dismisses the group and undoes the link.
Shared plays ("heard via another copy") are left for a later step.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -269,6 +269,9 @@ func foldTrackInto(
|
||||
if err := tq.MergeInheritTrackMbid(ctx, dbq.MergeInheritTrackMbidParams{SurvivorID: survivorID, LoserID: loserID}); err != nil {
|
||||
return f, fmt.Errorf("inherit recording mbid: %w", err)
|
||||
}
|
||||
if err := tq.MergeInheritSongLink(ctx, dbq.MergeInheritSongLinkParams{SurvivorID: survivorID, LoserID: loserID}); err != nil {
|
||||
return f, fmt.Errorf("inherit song link: %w", err)
|
||||
}
|
||||
|
||||
// Everything the loser carried now sits on the survivor, so the CASCADE
|
||||
// this delete sets off has nothing left to destroy.
|
||||
|
||||
@@ -89,6 +89,8 @@ type DuplicateResolveResult struct {
|
||||
Merged int
|
||||
MergeFailed int
|
||||
ReleaseChanges []ReleaseChange
|
||||
// SongsLinked counts cross-release groups newly linked as one song.
|
||||
SongsLinked int
|
||||
}
|
||||
|
||||
// LidarrPathKey is the part of a path Minstrel and Lidarr agree on: the last
|
||||
@@ -177,7 +179,18 @@ func ResolveDuplicates(
|
||||
return res, err
|
||||
}
|
||||
|
||||
if !act || !res.LidarrConsulted {
|
||||
if !act {
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// Linking removes nothing and needs nothing from Lidarr.
|
||||
linked, err := linkCrossRelease(ctx, pool, logger, groups)
|
||||
res.SongsLinked = linked
|
||||
if err != nil {
|
||||
return res, err
|
||||
}
|
||||
|
||||
if !res.LidarrConsulted {
|
||||
return res, nil
|
||||
}
|
||||
|
||||
@@ -222,6 +235,66 @@ func ResolveDuplicates(
|
||||
return res, nil
|
||||
}
|
||||
|
||||
// linkCrossRelease links each cross-release group's copies as one song
|
||||
// (#5438): a like on one is a like on all, and the Liked list, shuffle and the
|
||||
// mixes count the song once. Linking an already-linked group changes nothing,
|
||||
// so this runs every pass; likes are shared only when a link is new, since a
|
||||
// like made after it reaches every copy as it is made.
|
||||
func linkCrossRelease(ctx context.Context, pool *pgxpool.Pool, logger *slog.Logger, groups []*resolveGroup) (int, error) {
|
||||
linked := 0
|
||||
for _, g := range groups {
|
||||
if g.class != ClassCrossRelease {
|
||||
continue
|
||||
}
|
||||
if ctx.Err() != nil {
|
||||
return linked, ctx.Err()
|
||||
}
|
||||
ids := make([]pgtype.UUID, len(g.members))
|
||||
for i, m := range g.members {
|
||||
ids[i] = m.TrackID
|
||||
}
|
||||
moved, err := linkSong(ctx, pool, ids)
|
||||
if err != nil {
|
||||
logger.Warn("duplicate resolve: song link failed", "group_id", syncpkg.FormatUUID(g.id), "err", err)
|
||||
continue
|
||||
}
|
||||
if moved {
|
||||
linked++
|
||||
}
|
||||
}
|
||||
return linked, nil
|
||||
}
|
||||
|
||||
// linkSong links the tracks as one song and, when that changed anything,
|
||||
// shares their likes across the song and logs each added like for sync.
|
||||
func linkSong(ctx context.Context, pool *pgxpool.Pool, ids []pgtype.UUID) (bool, error) {
|
||||
tx, err := pool.Begin(ctx)
|
||||
if err != nil {
|
||||
return false, err
|
||||
}
|
||||
defer func() { _ = tx.Rollback(ctx) }()
|
||||
tq := dbq.New(tx)
|
||||
link, err := tq.LinkTracksAsSong(ctx, ids)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("link: %w", err)
|
||||
}
|
||||
if link.Moved == 0 {
|
||||
return false, nil
|
||||
}
|
||||
added, err := tq.ShareSongLikes(ctx, link.SongKey)
|
||||
if err != nil {
|
||||
return false, fmt.Errorf("share likes: %w", err)
|
||||
}
|
||||
likeIDs := make([]string, len(added))
|
||||
for i, a := range added {
|
||||
likeIDs[i] = syncpkg.EncodeLikeID(syncpkg.FormatUUID(a.UserID), syncpkg.FormatUUID(a.TrackID))
|
||||
}
|
||||
if err := syncpkg.LogChanges(ctx, tx, syncpkg.EntityLikeTrack, likeIDs, syncpkg.OpUpsert); err != nil {
|
||||
return false, fmt.Errorf("log shared likes: %w", err)
|
||||
}
|
||||
return true, tx.Commit(ctx)
|
||||
}
|
||||
|
||||
func foldResolveGroups(rows []dbq.ListDuplicateGroupsForResolveRow) []*resolveGroup {
|
||||
var groups []*resolveGroup
|
||||
for _, r := range rows {
|
||||
@@ -250,7 +323,7 @@ func resolveNote(g *resolveGroup) string {
|
||||
return "Lidarr tracks every copy as its own track, so removing one would make Lidarr download it again."
|
||||
}
|
||||
case ClassCrossRelease:
|
||||
return "Each copy belongs to its own release, and Lidarr keeps each one. Nothing is removed."
|
||||
return "Each copy belongs to its own release, and Lidarr keeps each one. Nothing is removed; they count as one song."
|
||||
case ClassMismatch:
|
||||
return "Identical audio filed under different titles: one file carries another song's tags."
|
||||
}
|
||||
@@ -674,5 +747,5 @@ func (w *DuplicateResolveWorker) tickOnce(ctx context.Context) {
|
||||
}
|
||||
w.logger.Info("duplicate resolve complete",
|
||||
"groups", res.Groups, "lidarr", res.LidarrConsulted, "merged", res.Merged,
|
||||
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges))
|
||||
"merge_failed", res.MergeFailed, "release_changes", len(res.ReleaseChanges), "songs_linked", res.SongsLinked)
|
||||
}
|
||||
|
||||
@@ -0,0 +1,208 @@
|
||||
package library
|
||||
|
||||
import (
|
||||
"context"
|
||||
"testing"
|
||||
|
||||
"github.com/jackc/pgx/v5/pgtype"
|
||||
"github.com/jackc/pgx/v5/pgxpool"
|
||||
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq"
|
||||
"git.fabledsword.com/bvandeusen/minstrel/internal/dbtest"
|
||||
)
|
||||
|
||||
// songLinkFixture is the single and the album it came from: one song on two
|
||||
// releases, in one pending acoustic group, and a user who liked the single.
|
||||
type songLinkFixture struct {
|
||||
single, albumCut dbq.Track
|
||||
groupID pgtype.UUID
|
||||
user dbq.User
|
||||
}
|
||||
|
||||
func newSongLinkFixture(t *testing.T, pool *pgxpool.Pool) songLinkFixture {
|
||||
t.Helper()
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
exec := func(sql string, args ...any) {
|
||||
t.Helper()
|
||||
if _, err := pool.Exec(ctx, sql, args...); err != nil {
|
||||
t.Fatalf("exec %q: %v", sql, err)
|
||||
}
|
||||
}
|
||||
artist, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: "Link Artist", SortName: "Link Artist"})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
track := func(album, path string) dbq.Track {
|
||||
t.Helper()
|
||||
a, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{Title: album, SortTitle: album, ArtistID: artist.ID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
tr, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
||||
Title: "Feel Good Inc.", AlbumID: a.ID, ArtistID: artist.ID,
|
||||
DurationMs: 1000, FilePath: path, FileSize: 100, FileFormat: "mp3",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
return tr
|
||||
}
|
||||
f := songLinkFixture{
|
||||
single: track("Feel Good Inc. (single)", "/music/Link Artist/Feel Good Inc/01 Feel Good Inc.mp3"),
|
||||
albumCut: track("Demon Days", "/music/Link Artist/Demon Days/06 Feel Good Inc.mp3"),
|
||||
}
|
||||
f.user, err = q.CreateUser(ctx, dbq.CreateUserParams{
|
||||
Username: dbtest.TestUserPrefix + "song-link", PasswordHash: "x", ApiTokenHash: "song-link-token",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := pool.QueryRow(ctx,
|
||||
`INSERT INTO duplicate_groups (member_key, tier) VALUES ('song-link', 'acoustic') RETURNING id`,
|
||||
).Scan(&f.groupID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
exec(`INSERT INTO duplicate_group_members (group_id, track_id) VALUES ($1, $2), ($1, $3)`, f.groupID, f.single.ID, f.albumCut.ID)
|
||||
exec(`INSERT INTO general_likes (user_id, track_id) VALUES ($1, $2)`, f.user.ID, f.single.ID)
|
||||
return f
|
||||
}
|
||||
|
||||
// A cross-release group becomes one song: its copies share a song key, the
|
||||
// like on the single reaches the album cut (and is logged for sync), the Liked
|
||||
// list shows the song once, a like or unlike on either copy reaches both, and
|
||||
// "not the same song" undoes the link.
|
||||
func TestCrossReleaseCopiesCountAsOneSong_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
f := newSongLinkFixture(t, pool)
|
||||
|
||||
res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if res.Classes[ClassCrossRelease] != 1 || res.SongsLinked != 1 {
|
||||
t.Fatalf("classes %v, linked %d; want one cross-release group linked", res.Classes, res.SongsLinked)
|
||||
}
|
||||
keys := songKeys(t, q, f.single.ID, f.albumCut.ID)
|
||||
if keys[0] != keys[1] {
|
||||
t.Fatalf("song keys differ after linking: %v", keys)
|
||||
}
|
||||
|
||||
liked, err := q.ListLikedTrackIDs(ctx, f.user.ID)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(liked) != 2 {
|
||||
t.Errorf("liked track ids = %d, want both copies", len(liked))
|
||||
}
|
||||
var logged int
|
||||
if err := pool.QueryRow(ctx,
|
||||
`SELECT count(*) FROM library_changes WHERE entity_type = 'like_track' AND op = 'upsert'`,
|
||||
).Scan(&logged); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if logged != 1 {
|
||||
t.Errorf("logged %d shared likes, want 1 (the album cut)", logged)
|
||||
}
|
||||
if n, err := q.CountLikedTracks(ctx, f.user.ID); err != nil || n != 1 {
|
||||
t.Errorf("CountLikedTracks = %d (%v), want 1 song", n, err)
|
||||
}
|
||||
rows, err := q.ListLikedTrackRows(ctx, dbq.ListLikedTrackRowsParams{UserID: f.user.ID, Limit: 10})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(rows) != 1 {
|
||||
t.Errorf("Liked list has %d rows, want the song once", len(rows))
|
||||
}
|
||||
|
||||
// A second pass changes nothing.
|
||||
if res, err := ResolveDuplicates(ctx, pool, nil, "", nil, true); err != nil || res.SongsLinked != 0 {
|
||||
t.Errorf("second pass linked %d (%v), want 0", res.SongsLinked, err)
|
||||
}
|
||||
|
||||
unliked, err := q.UnlikeTrack(ctx, dbq.UnlikeTrackParams{UserID: f.user.ID, TrackID: f.albumCut.ID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(unliked) != 2 {
|
||||
t.Errorf("unlike removed %d likes, want both copies", len(unliked))
|
||||
}
|
||||
added, err := q.LikeTrack(ctx, dbq.LikeTrackParams{UserID: f.user.ID, TrackID: f.single.ID})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if len(added) != 2 {
|
||||
t.Errorf("like added %d, want both copies", len(added))
|
||||
}
|
||||
if again, err := q.LikeTrack(ctx, dbq.LikeTrackParams{UserID: f.user.ID, TrackID: f.single.ID}); err != nil || len(again) != 0 {
|
||||
t.Errorf("repeat like added %d (%v), want 0", len(again), err)
|
||||
}
|
||||
|
||||
if err := q.UnlinkDuplicateGroupSongs(ctx, f.groupID); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
keys = songKeys(t, q, f.single.ID, f.albumCut.ID)
|
||||
if keys[0] == keys[1] {
|
||||
t.Error("song keys still shared after unlinking")
|
||||
}
|
||||
}
|
||||
|
||||
// Without the operator's permission the resolver links nothing.
|
||||
func TestCrossReleaseLinkNeedsAutoResolve_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
f := newSongLinkFixture(t, pool)
|
||||
if _, err := ResolveDuplicates(ctx, pool, nil, "", nil, false); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
keys := songKeys(t, q, f.single.ID, f.albumCut.ID)
|
||||
if keys[0] == keys[1] {
|
||||
t.Error("linked with auto-resolve off")
|
||||
}
|
||||
}
|
||||
|
||||
// A merge keeps the removed copy's link to the song on another release.
|
||||
func TestMergeInheritsSongLink_Integration(t *testing.T) {
|
||||
pool := newPool(t)
|
||||
ctx := context.Background()
|
||||
q := dbq.New(pool)
|
||||
f := newSongLinkFixture(t, pool)
|
||||
if _, err := q.LinkTracksAsSong(ctx, []pgtype.UUID{f.single.ID, f.albumCut.ID}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
// A third, unlinked copy of the album cut stands in for a survivor.
|
||||
other, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{
|
||||
Title: "Feel Good Inc.", AlbumID: f.albumCut.AlbumID, ArtistID: f.albumCut.ArtistID,
|
||||
DurationMs: 1000, FilePath: "/music/Link Artist/Demon Days/06 Feel Good Inc (1).mp3", FileSize: 90, FileFormat: "mp3",
|
||||
})
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
if err := q.MergeInheritSongLink(ctx, dbq.MergeInheritSongLinkParams{SurvivorID: other.ID, LoserID: f.albumCut.ID}); err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
keys := songKeys(t, q, other.ID, f.single.ID)
|
||||
if keys[0] != keys[1] {
|
||||
t.Error("survivor did not join the song")
|
||||
}
|
||||
}
|
||||
|
||||
func songKeys(t *testing.T, q *dbq.Queries, ids ...pgtype.UUID) []pgtype.UUID {
|
||||
t.Helper()
|
||||
rows, err := q.ListTrackSongKeys(context.Background(), ids)
|
||||
if err != nil {
|
||||
t.Fatal(err)
|
||||
}
|
||||
byID := map[pgtype.UUID]pgtype.UUID{}
|
||||
for _, r := range rows {
|
||||
byID[r.ID] = r.SongKey
|
||||
}
|
||||
out := make([]pgtype.UUID, len(ids))
|
||||
for i, id := range ids {
|
||||
out[i] = byID[id]
|
||||
}
|
||||
return out
|
||||
}
|
||||
Reference in New Issue
Block a user