diff --git a/internal/playlists/collage.go b/internal/playlists/collage.go index 0a913acc..ae363ffa 100644 --- a/internal/playlists/collage.go +++ b/internal/playlists/collage.go @@ -33,13 +33,20 @@ const ( // "" when the playlist is empty (cover_path also cleared). func GenerateCollage(ctx context.Context, pool *pgxpool.Pool, playlistID pgtype.UUID, dataDir string) (string, error) { q := dbq.New(pool) - rows, err := q.ListAllPlaylistTracksForCollage(ctx, dbq.ListAllPlaylistTracksForCollageParams{ + // Pull more rows than cells so we can dedupe identical covers and + // still fill every cell. A 25-track "Songs like X" playlist that + // has 5 tracks each from 5 albums shouldn't render as four + // identical cells of the first album's cover; pulling 50 and + // deduping by cover_art_path gives us four distinct covers in + // realistic libraries. + allRows, err := q.ListAllPlaylistTracksForCollage(ctx, dbq.ListAllPlaylistTracksForCollageParams{ PlaylistID: playlistID, - Limit: 4, + Limit: 50, }) if err != nil { return "", fmt.Errorf("list collage tracks: %w", err) } + rows := dedupeCollageRows(allRows, 4) if len(rows) == 0 { // Empty playlist: clear cover_path and remove any stale file. @@ -101,6 +108,34 @@ func GenerateCollage(ctx context.Context, pool *pgxpool.Pool, playlistID pgtype. return relPath, nil } +// dedupeCollageRows trims the input to at most max rows such that no +// non-empty cover_art_path appears twice. Rows with NULL or empty +// cover_art_path are kept as-is — they each render the fallback glyph +// in their own cell, which is the right behavior (a partially-covered +// playlist still gets a sensible visual rather than collapsing every +// missing cover into a single cell). +// +// Preserves input order (which is playlist position order from the SQL). +func dedupeCollageRows(rows []dbq.ListAllPlaylistTracksForCollageRow, max int) []dbq.ListAllPlaylistTracksForCollageRow { + seen := make(map[string]bool, max) + out := make([]dbq.ListAllPlaylistTracksForCollageRow, 0, max) + for _, r := range rows { + if len(out) >= max { + break + } + if r.AlbumCoverPath == nil || *r.AlbumCoverPath == "" { + out = append(out, r) + continue + } + if seen[*r.AlbumCoverPath] { + continue + } + seen[*r.AlbumCoverPath] = true + out = append(out, r) + } + return out +} + // loadCellImage tries to load the album cover at the given path. On any // failure (empty path, missing file, decode error), returns the rendered // fallback glyph as a 300x300 image. diff --git a/internal/playlists/collage_dedupe_test.go b/internal/playlists/collage_dedupe_test.go new file mode 100644 index 00000000..9a08f086 --- /dev/null +++ b/internal/playlists/collage_dedupe_test.go @@ -0,0 +1,109 @@ +package playlists + +import ( + "testing" + + "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" +) + +func strPtr(s string) *string { return &s } + +func TestDedupeCollageRows_DropsDuplicates(t *testing.T) { + rows := []dbq.ListAllPlaylistTracksForCollageRow{ + {Position: 0, AlbumCoverPath: strPtr("a.jpg")}, + {Position: 1, AlbumCoverPath: strPtr("a.jpg")}, // dup → drop + {Position: 2, AlbumCoverPath: strPtr("b.jpg")}, + {Position: 3, AlbumCoverPath: strPtr("a.jpg")}, // dup → drop + {Position: 4, AlbumCoverPath: strPtr("c.jpg")}, + {Position: 5, AlbumCoverPath: strPtr("d.jpg")}, + {Position: 6, AlbumCoverPath: strPtr("e.jpg")}, // beyond max=4 → drop + } + got := dedupeCollageRows(rows, 4) + if len(got) != 4 { + t.Fatalf("len = %d, want 4", len(got)) + } + wantPaths := []string{"a.jpg", "b.jpg", "c.jpg", "d.jpg"} + for i, w := range wantPaths { + if got[i].AlbumCoverPath == nil || *got[i].AlbumCoverPath != w { + t.Errorf("got[%d].AlbumCoverPath = %v, want %q", i, got[i].AlbumCoverPath, w) + } + } +} + +func TestDedupeCollageRows_NullCoversNotDeduped(t *testing.T) { + // NULL covers each render the fallback glyph in their own cell. + // They shouldn't dedupe against each other; the playlist with all + // missing covers should still produce 4 cells. + rows := []dbq.ListAllPlaylistTracksForCollageRow{ + {Position: 0, AlbumCoverPath: nil}, + {Position: 1, AlbumCoverPath: nil}, + {Position: 2, AlbumCoverPath: nil}, + {Position: 3, AlbumCoverPath: nil}, + } + got := dedupeCollageRows(rows, 4) + if len(got) != 4 { + t.Errorf("len = %d, want 4 (nil covers shouldn't dedupe)", len(got)) + } +} + +func TestDedupeCollageRows_EmptyStringCoverNotDeduped(t *testing.T) { + // An empty-string cover_art_path is treated the same as NULL — + // renders the glyph fallback. Should not dedupe. + empty := "" + rows := []dbq.ListAllPlaylistTracksForCollageRow{ + {Position: 0, AlbumCoverPath: &empty}, + {Position: 1, AlbumCoverPath: &empty}, + } + got := dedupeCollageRows(rows, 4) + if len(got) != 2 { + t.Errorf("len = %d, want 2", len(got)) + } +} + +func TestDedupeCollageRows_PreservesPositionOrder(t *testing.T) { + rows := []dbq.ListAllPlaylistTracksForCollageRow{ + {Position: 0, AlbumCoverPath: strPtr("z.jpg")}, + {Position: 1, AlbumCoverPath: strPtr("a.jpg")}, + {Position: 2, AlbumCoverPath: strPtr("m.jpg")}, + } + got := dedupeCollageRows(rows, 4) + if len(got) != 3 { + t.Fatalf("len = %d, want 3", len(got)) + } + // First-seen wins; original input order is preserved (z, a, m). + wantPaths := []string{"z.jpg", "a.jpg", "m.jpg"} + for i, w := range wantPaths { + if got[i].AlbumCoverPath == nil || *got[i].AlbumCoverPath != w { + t.Errorf("got[%d] = %v, want %q", i, got[i].AlbumCoverPath, w) + } + } +} + +func TestDedupeCollageRows_MixedNullAndDups(t *testing.T) { + // Mixed: a few NULL covers + dup non-NULL. NULLs always pass; + // non-NULL dedups. + rows := []dbq.ListAllPlaylistTracksForCollageRow{ + {Position: 0, AlbumCoverPath: strPtr("a.jpg")}, + {Position: 1, AlbumCoverPath: nil}, + {Position: 2, AlbumCoverPath: strPtr("a.jpg")}, // dup of [0] → drop + {Position: 3, AlbumCoverPath: nil}, + {Position: 4, AlbumCoverPath: strPtr("b.jpg")}, + } + got := dedupeCollageRows(rows, 4) + if len(got) != 4 { + t.Fatalf("len = %d, want 4", len(got)) + } + // Expected: a.jpg, nil, nil, b.jpg + if got[0].AlbumCoverPath == nil || *got[0].AlbumCoverPath != "a.jpg" { + t.Errorf("got[0] = %v, want a.jpg", got[0].AlbumCoverPath) + } + if got[1].AlbumCoverPath != nil { + t.Errorf("got[1] = %v, want nil", got[1].AlbumCoverPath) + } + if got[2].AlbumCoverPath != nil { + t.Errorf("got[2] = %v, want nil", got[2].AlbumCoverPath) + } + if got[3].AlbumCoverPath == nil || *got[3].AlbumCoverPath != "b.jpg" { + t.Errorf("got[3] = %v, want b.jpg", got[3].AlbumCoverPath) + } +}