diff --git a/conf/configuration.go b/conf/configuration.go index b867e0d5..dc5fd1b5 100644 --- a/conf/configuration.go +++ b/conf/configuration.go @@ -71,6 +71,7 @@ type configOptions struct { CoverArtPriority string CoverArtQuality int ArtistArtPriority string + DiscArtPriority string LyricsPriority string EnableGravatar bool EnableFavourites bool @@ -660,6 +661,7 @@ func setViperDefaults() { viper.SetDefault("coverartpriority", "cover.*, folder.*, front.*, embedded, external") viper.SetDefault("coverartquality", 75) viper.SetDefault("artistartpriority", "artist.*, album/artist.*, external") + viper.SetDefault("discartpriority", "disc*.*, cd*.*, cover.*, folder.*, front.*, discsubtitle, embedded") viper.SetDefault("lyricspriority", ".lrc,.txt,embedded") viper.SetDefault("enablegravatar", false) viper.SetDefault("enablefavourites", true) diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 2e92b24c..4a0a32af 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -122,6 +122,8 @@ func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, s artReader, err = newMediafileArtworkReader(ctx, a, artID) case model.KindPlaylistArtwork: artReader, err = newPlaylistArtworkReader(ctx, a, artID) + case model.KindDiscArtwork: + artReader, err = newDiscArtworkReader(ctx, a, artID) default: return nil, ErrUnavailable } diff --git a/core/artwork/reader_disc.go b/core/artwork/reader_disc.go new file mode 100644 index 00000000..7548f76d --- /dev/null +++ b/core/artwork/reader_disc.go @@ -0,0 +1,268 @@ +package artwork + +import ( + "context" + "crypto/md5" + "fmt" + "io" + "os" + "path/filepath" + "strconv" + "strings" + "time" + + "github.com/Masterminds/squirrel" + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/ffmpeg" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" +) + +type discArtworkReader struct { + cacheKey + a *artwork + album model.Album + discNumber int + imgFiles []string + discFolders map[string]bool + isMultiFolder bool + firstTrackPath string + updatedAt *time.Time +} + +func newDiscArtworkReader(ctx context.Context, a *artwork, artID model.ArtworkID) (*discArtworkReader, error) { + albumID, discNumber, err := model.ParseDiscArtworkID(artID.ID) + if err != nil { + return nil, fmt.Errorf("invalid disc artwork id '%s': %w", artID.ID, err) + } + + al, err := a.ds.Album(ctx).Get(albumID) + if err != nil { + return nil, err + } + + _, imgFiles, imagesUpdatedAt, err := loadAlbumFoldersPaths(ctx, a.ds, *al) + if err != nil { + return nil, err + } + + // Query mediafiles for this album + disc to find folder associations and first track + mfs, err := a.ds.MediaFile(ctx).GetAll(model.QueryOptions{ + Sort: "track_number", + Order: "ASC", + Filters: squirrel.Eq{"album_id": albumID, "disc_number": discNumber}, + }) + if err != nil { + return nil, err + } + + // Build disc folder set and find first track + discFolders := make(map[string]bool) + var firstTrackPath string + allFolderIDs := make(map[string]bool) + for _, mf := range mfs { + allFolderIDs[mf.FolderID] = true + if firstTrackPath == "" { + firstTrackPath = mf.Path + } + } + + // Resolve folder IDs to absolute paths + if len(allFolderIDs) > 0 { + folderIDs := make([]string, 0, len(allFolderIDs)) + for id := range allFolderIDs { + folderIDs = append(folderIDs, id) + } + folders, err := a.ds.Folder(ctx).GetAll(model.QueryOptions{ + Filters: squirrel.Eq{"folder.id": folderIDs}, + }) + if err != nil { + return nil, err + } + for _, f := range folders { + discFolders[f.AbsolutePath()] = true + } + } + + isMultiFolder := len(al.FolderIDs) > 1 + + r := &discArtworkReader{ + a: a, + album: *al, + discNumber: discNumber, + imgFiles: imgFiles, + discFolders: discFolders, + isMultiFolder: isMultiFolder, + firstTrackPath: core.AbsolutePath(ctx, a.ds, al.LibraryID, firstTrackPath), + updatedAt: imagesUpdatedAt, + } + r.cacheKey.artID = artID + if r.updatedAt != nil && r.updatedAt.After(al.UpdatedAt) { + r.cacheKey.lastUpdate = *r.updatedAt + } else { + r.cacheKey.lastUpdate = al.UpdatedAt + } + return r, nil +} + +func (d *discArtworkReader) Key() string { + hash := md5.Sum([]byte(conf.Server.DiscArtPriority)) + return fmt.Sprintf( + "%s.%x", + d.cacheKey.Key(), + hash, + ) +} + +func (d *discArtworkReader) LastUpdated() time.Time { + return d.album.UpdatedAt +} + +func (d *discArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) { + var ff = d.fromDiscArtPriority(ctx, d.a.ffmpeg, conf.Server.DiscArtPriority) + // Fallback to album cover art + albumArtID := model.NewArtworkID(model.KindAlbumArtwork, d.album.ID, &d.album.UpdatedAt) + ff = append(ff, fromAlbum(ctx, d.a, albumArtID)) + return selectImageReader(ctx, d.cacheKey.artID, ff...) +} + +func (d *discArtworkReader) fromDiscArtPriority(ctx context.Context, ffmpeg ffmpeg.FFmpeg, priority string) []sourceFunc { + var ff []sourceFunc + for pattern := range strings.SplitSeq(strings.ToLower(priority), ",") { + pattern = strings.TrimSpace(pattern) + switch { + case pattern == "embedded": + ff = append(ff, fromTag(ctx, d.firstTrackPath), fromFFmpegTag(ctx, ffmpeg, d.firstTrackPath)) + case pattern == "external": + // Not supported for disc art, silently ignore + case pattern == "discsubtitle": + if subtitle := strings.TrimSpace(d.album.Discs[d.discNumber]); subtitle != "" { + ff = append(ff, d.fromDiscSubtitle(ctx, subtitle)) + } + case len(d.imgFiles) > 0: + ff = append(ff, d.fromExternalFile(ctx, pattern)) + } + } + return ff +} + +// fromDiscSubtitle returns a sourceFunc that matches image files whose stem +// (filename without extension) equals the disc subtitle (case-insensitive). +func (d *discArtworkReader) fromDiscSubtitle(ctx context.Context, subtitle string) sourceFunc { + return func() (io.ReadCloser, string, error) { + for _, file := range d.imgFiles { + _, name := filepath.Split(file) + stem := strings.TrimSuffix(name, filepath.Ext(name)) + if !strings.EqualFold(stem, subtitle) { + continue + } + f, err := os.Open(file) + if err != nil { + log.Warn(ctx, "Could not open disc art file", "file", file, err) + continue + } + return f, file, nil + } + return nil, "", fmt.Errorf("disc %d: no image file matching subtitle %q", d.discNumber, subtitle) + } +} + +// extractDiscNumber extracts a disc number from a filename based on a glob pattern. +// It finds the portion of the filename that the wildcard matched and parses leading +// digits as the disc number. Returns (0, false) if the pattern doesn't match or +// no leading digits are found in the wildcard portion. +func extractDiscNumber(pattern, filename string) (int, bool) { + filename = strings.ToLower(filename) + pattern = strings.ToLower(pattern) + + matched, err := filepath.Match(pattern, filename) + if err != nil || !matched { + return 0, false + } + + // Find the prefix before the first '*' in the pattern + starIdx := strings.IndexByte(pattern, '*') + if starIdx < 0 { + return 0, false + } + prefix := pattern[:starIdx] + + // Strip the prefix from the filename to get the wildcard-matched portion + if !strings.HasPrefix(filename, prefix) { + return 0, false + } + remainder := filename[len(prefix):] + + // Extract leading ASCII digits from the remainder + var digits []byte + for _, r := range remainder { + if r >= '0' && r <= '9' { + digits = append(digits, byte(r)) + } else { + break + } + } + + if len(digits) == 0 { + return 0, false + } + + num, err := strconv.Atoi(string(digits)) + if err != nil { + return 0, false + } + return num, true +} + +// fromExternalFile returns a sourceFunc that matches image files against a glob +// pattern with disc-number-aware filtering. +// +// Matching rules: +// - If a disc number can be extracted from the filename, the file matches only if +// the number equals the target disc number. +// - If no number is found and this is a multi-folder album, the file matches if +// it's in a folder containing tracks for this disc. +// - If no number is found and this is a single-folder album, the file is skipped +// (ambiguous). +func (d *discArtworkReader) fromExternalFile(ctx context.Context, pattern string) sourceFunc { + return func() (io.ReadCloser, string, error) { + for _, file := range d.imgFiles { + _, name := filepath.Split(file) + match, err := filepath.Match(pattern, strings.ToLower(name)) + if err != nil { + log.Warn(ctx, "Error matching disc art file to pattern", "pattern", pattern, "file", file) + continue + } + if !match { + continue + } + + // Try to extract disc number from filename + num, hasNum := extractDiscNumber(pattern, name) + if hasNum { + // File has a disc number — must match target disc + if num != d.discNumber { + continue + } + } else if d.isMultiFolder { + // No number, multi-folder: match by folder association + dir := filepath.Dir(file) + if !d.discFolders[dir] { + continue + } + } else { + // No number, single-folder: ambiguous, skip + continue + } + + f, err := os.Open(file) + if err != nil { + log.Warn(ctx, "Could not open disc art file", "file", file, err) + continue + } + return f, file, nil + } + return nil, "", fmt.Errorf("disc %d: pattern '%s' not matched by files", d.discNumber, pattern) + } +} diff --git a/core/artwork/reader_disc_test.go b/core/artwork/reader_disc_test.go new file mode 100644 index 00000000..f8193e24 --- /dev/null +++ b/core/artwork/reader_disc_test.go @@ -0,0 +1,285 @@ +package artwork + +import ( + "context" + "os" + "path/filepath" + + "github.com/navidrome/navidrome/model" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Disc Artwork Reader", func() { + Describe("extractDiscNumber", func() { + DescribeTable("extracts disc number from filename based on glob pattern", + func(pattern, filename string, expectedNum int, expectedOk bool) { + num, ok := extractDiscNumber(pattern, filename) + Expect(ok).To(Equal(expectedOk)) + if expectedOk { + Expect(num).To(Equal(expectedNum)) + } + }, + // Standard disc patterns + Entry("disc1.jpg", "disc*.*", "disc1.jpg", 1, true), + Entry("disc2.png", "disc*.*", "disc2.png", 2, true), + Entry("disc01.jpg", "disc*.*", "disc01.jpg", 1, true), + Entry("disc02.png", "disc*.*", "disc02.png", 2, true), + Entry("disc10.jpg", "disc*.*", "disc10.jpg", 10, true), + + // CD patterns + Entry("cd1.jpg", "cd*.*", "cd1.jpg", 1, true), + Entry("cd02.png", "cd*.*", "cd02.png", 2, true), + + // No number in filename + Entry("disc.jpg has no number", "disc*.*", "disc.jpg", 0, false), + Entry("cd.jpg has no number", "cd*.*", "cd.jpg", 0, false), + + // Extra text after number + Entry("disc2-bonus.jpg", "disc*.*", "disc2-bonus.jpg", 2, true), + Entry("disc01_front.png", "disc*.*", "disc01_front.png", 1, true), + + // Case insensitive (filename already lowered by caller) + Entry("Disc1.jpg lowered", "disc*.*", "disc1.jpg", 1, true), + + // Pattern doesn't match + Entry("cover.jpg doesn't match disc*.*", "disc*.*", "cover.jpg", 0, false), + + // Pattern with no wildcard before dot + Entry("front1.jpg with front*.*", "front*.*", "front1.jpg", 1, true), + ) + }) + + Describe("fromExternalFile", func() { + var ( + ctx context.Context + tmpDir string + ) + + BeforeEach(func() { + ctx = context.Background() + tmpDir = GinkgoT().TempDir() + }) + + createFile := func(path string) string { + fullPath := filepath.Join(tmpDir, filepath.FromSlash(path)) + Expect(os.MkdirAll(filepath.Dir(fullPath), 0755)).To(Succeed()) + Expect(os.WriteFile(fullPath, []byte("image data"), 0600)).To(Succeed()) + return fullPath + } + + It("matches file with disc number in single-folder album", func() { + f1 := createFile("album/disc1.jpg") + f2 := createFile("album/disc2.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("skips file without number in single-folder album", func() { + f1 := createFile("album/disc.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, _, _ := sf() + Expect(r).To(BeNil()) + }) + + It("matches file without number in multi-folder album by folder", func() { + f1 := createFile("album/cd1/disc.jpg") + f2 := createFile("album/cd2/disc.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album", "cd1"): true}, + isMultiFolder: true, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("prefers disc number over folder when number is present", func() { + // disc2.jpg in cd1 folder should match disc 2, not disc 1 + f1 := createFile("album/cd1/disc2.jpg") + reader := &discArtworkReader{ + discNumber: 2, + imgFiles: []string{f1}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album", "cd1"): true}, + isMultiFolder: true, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("does not match disc2.jpg when looking for disc 1", func() { + f1 := createFile("album/disc2.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1}, + discFolders: map[string]bool{filepath.Join(tmpDir, "album"): true}, + } + + sf := reader.fromExternalFile(ctx, "disc*.*") + r, _, _ := sf() + Expect(r).To(BeNil()) + }) + }) + + Describe("fromDiscSubtitle", func() { + var ( + ctx context.Context + tmpDir string + ) + + BeforeEach(func() { + ctx = context.Background() + tmpDir = GinkgoT().TempDir() + }) + + createFile := func(path string) string { + fullPath := filepath.Join(tmpDir, filepath.FromSlash(path)) + Expect(os.MkdirAll(filepath.Dir(fullPath), 0755)).To(Succeed()) + Expect(os.WriteFile(fullPath, []byte("image data"), 0600)).To(Succeed()) + return fullPath + } + + It("matches image file whose stem equals the disc subtitle (case-insensitive)", func() { + f1 := createFile("album/The Blue Disc.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1}, + } + + sf := reader.fromDiscSubtitle(ctx, "The Blue Disc") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("matches case-insensitively", func() { + f1 := createFile("album/bonus tracks.png") + reader := &discArtworkReader{ + discNumber: 2, + imgFiles: []string{f1}, + } + + sf := reader.fromDiscSubtitle(ctx, "Bonus Tracks") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + + It("returns error when no matching file found", func() { + f1 := createFile("album/cover.jpg") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1}, + } + + sf := reader.fromDiscSubtitle(ctx, "The Blue Disc") + _, _, err := sf() + Expect(err).To(HaveOccurred()) + }) + + It("matches first file when multiple extensions exist", func() { + f1 := createFile("album/The Blue Disc.jpg") + f2 := createFile("album/The Blue Disc.png") + reader := &discArtworkReader{ + discNumber: 1, + imgFiles: []string{f1, f2}, + } + + sf := reader.fromDiscSubtitle(ctx, "The Blue Disc") + r, path, err := sf() + Expect(err).ToNot(HaveOccurred()) + Expect(r).ToNot(BeNil()) + r.Close() + Expect(path).To(Equal(f1)) + }) + }) + + Describe("discArtworkReader", func() { + Describe("fromDiscArtPriority", func() { + var reader *discArtworkReader + + BeforeEach(func() { + reader = &discArtworkReader{ + discNumber: 2, + isMultiFolder: true, + discFolders: map[string]bool{"/music/album/cd2": true}, + imgFiles: []string{ + "/music/album/cd1/disc.jpg", + "/music/album/cd2/disc.jpg", + "/music/album/cd2/disc2.jpg", + }, + firstTrackPath: "/music/album/cd2/track1.flac", + } + }) + + It("returns source funcs for glob patterns", func() { + ff := reader.fromDiscArtPriority(context.Background(), nil, "disc*.*") + Expect(ff).To(HaveLen(1)) + }) + + It("returns source funcs for embedded pattern", func() { + ff := reader.fromDiscArtPriority(context.Background(), nil, "embedded") + Expect(ff).To(HaveLen(2)) // fromTag + fromFFmpegTag + }) + + It("handles multiple comma-separated patterns", func() { + ff := reader.fromDiscArtPriority(context.Background(), nil, "disc*.*, cd*.*, embedded") + Expect(ff).To(HaveLen(4)) // disc*.* + cd*.* + fromTag + fromFFmpegTag + }) + + It("ignores 'external' pattern silently", func() { + ff := reader.fromDiscArtPriority(context.Background(), nil, "external") + Expect(ff).To(HaveLen(0)) + }) + + It("returns no source funcs when imgFiles is empty and pattern is not embedded", func() { + reader.imgFiles = nil + ff := reader.fromDiscArtPriority(context.Background(), nil, "disc*.*") + Expect(ff).To(HaveLen(0)) + }) + + It("returns source func for discsubtitle pattern", func() { + reader.album = model.Album{Discs: model.Discs{2: "Bonus Tracks"}} + ff := reader.fromDiscArtPriority(context.Background(), nil, "discsubtitle") + Expect(ff).To(HaveLen(1)) + }) + + It("returns no source func for discsubtitle when disc has no subtitle", func() { + reader.album = model.Album{Discs: model.Discs{2: ""}} + ff := reader.fromDiscArtPriority(context.Background(), nil, "discsubtitle") + Expect(ff).To(HaveLen(0)) + }) + }) + }) +}) diff --git a/model/artwork_id.go b/model/artwork_id.go index 36026dd0..8d935f42 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -22,6 +22,7 @@ var ( KindArtistArtwork = Kind{"ar", "artist"} KindAlbumArtwork = Kind{"al", "album"} KindPlaylistArtwork = Kind{"pl", "playlist"} + KindDiscArtwork = Kind{"dc", "disc"} ) var artworkKindMap = map[string]Kind{ @@ -29,6 +30,7 @@ var artworkKindMap = map[string]Kind{ KindArtistArtwork.prefix: KindArtistArtwork, KindAlbumArtwork.prefix: KindAlbumArtwork, KindPlaylistArtwork.prefix: KindPlaylistArtwork, + KindDiscArtwork.prefix: KindDiscArtwork, } type ArtworkID struct { @@ -91,6 +93,22 @@ func MustParseArtworkID(id string) ArtworkID { return artID } +func DiscArtworkID(albumID string, discNumber int) string { + return fmt.Sprintf("%s:%d", albumID, discNumber) +} + +func ParseDiscArtworkID(id string) (albumID string, discNumber int, err error) { + parts := strings.SplitN(id, ":", 2) + if len(parts) != 2 || parts[1] == "" { + return "", 0, errors.New("invalid disc artwork id") + } + num, err := strconv.Atoi(parts[1]) + if err != nil { + return "", 0, fmt.Errorf("invalid disc number in artwork id: %w", err) + } + return parts[0], num, nil +} + func artworkIDFromAlbum(al Album) ArtworkID { return ArtworkID{ Kind: KindAlbumArtwork, diff --git a/model/artwork_id_test.go b/model/artwork_id_test.go index 2f42217f..b634e7cb 100644 --- a/model/artwork_id_test.go +++ b/model/artwork_id_test.go @@ -28,6 +28,40 @@ var _ = Describe("ArtworkID", func() { Expect(parsedId.LastUpdate.Unix()).To(Equal(id.LastUpdate.Unix())) }) }) + Describe("ParseArtworkID - disc kind", func() { + It("parses a disc artwork ID with dc prefix", func() { + now := time.Now() + id := model.NewArtworkID(model.KindDiscArtwork, "albumid123:2", &now) + parsedId, err := model.ParseArtworkID(id.String()) + Expect(err).ToNot(HaveOccurred()) + Expect(parsedId.Kind).To(Equal(model.KindDiscArtwork)) + Expect(parsedId.ID).To(Equal("albumid123:2")) + Expect(parsedId.LastUpdate.Unix()).To(Equal(now.Unix())) + }) + }) + + Describe("ParseDiscArtworkID", func() { + DescribeTable("parses composite disc artwork IDs", + func(id string, expectedAlbum string, expectedDisc int, expectErr bool) { + albumID, discNumber, err := model.ParseDiscArtworkID(id) + if expectErr { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).ToNot(HaveOccurred()) + Expect(albumID).To(Equal(expectedAlbum)) + Expect(discNumber).To(Equal(expectedDisc)) + } + }, + Entry("valid id", "albumid123:2", "albumid123", 2, false), + Entry("disc number 1", "abc:1", "abc", 1, false), + Entry("large disc number", "abc:10", "abc", 10, false), + Entry("missing colon", "albumid123", "", 0, true), + Entry("missing disc number", "albumid123:", "", 0, true), + Entry("non-numeric disc", "albumid123:abc", "", 0, true), + Entry("empty string", "", "", 0, true), + ) + }) + Describe("ParseArtworkID()", func() { It("parses album artwork ids", func() { id, err := model.ParseArtworkID("al-1234") diff --git a/server/subsonic/helpers.go b/server/subsonic/helpers.go index 59834690..e930aa63 100644 --- a/server/subsonic/helpers.go +++ b/server/subsonic/helpers.go @@ -392,7 +392,13 @@ func buildDiscSubtitles(a model.Album) []responses.DiscTitle { } var discTitles []responses.DiscTitle for num, title := range a.Discs { - discTitles = append(discTitles, responses.DiscTitle{Disc: int32(num), Title: title}) + artID := model.NewArtworkID(model.KindDiscArtwork, + model.DiscArtworkID(a.ID, num), &a.UpdatedAt) + discTitles = append(discTitles, responses.DiscTitle{ + Disc: int32(num), + Title: title, + CoverArt: artID.String(), + }) } if len(discTitles) == 1 && discTitles[0].Title == "" { return nil diff --git a/server/subsonic/helpers_test.go b/server/subsonic/helpers_test.go index 2099c8f6..d20ca89a 100644 --- a/server/subsonic/helpers_test.go +++ b/server/subsonic/helpers_test.go @@ -3,6 +3,7 @@ package subsonic import ( "context" "net/http/httptest" + "time" "github.com/go-chi/jwtauth/v5" "github.com/navidrome/navidrome/conf" @@ -103,27 +104,43 @@ var _ = Describe("helpers", func() { Expect(buildDiscSubtitles(album)).To(BeNil()) }) - It("should return the disc title for a single disc", func() { + It("should return the disc title with cover art for a single disc", func() { + updatedAt := time.Now().Truncate(time.Second) album := model.Album{ + ID: "album1", + UpdatedAt: updatedAt, Discs: map[int]string{ 1: "Special Edition", }, } - Expect(buildDiscSubtitles(album)).To(Equal([]responses.DiscTitle{{Disc: 1, Title: "Special Edition"}})) + result := buildDiscSubtitles(album) + Expect(result).To(HaveLen(1)) + Expect(result[0].Disc).To(Equal(int32(1))) + Expect(result[0].Title).To(Equal("Special Edition")) + expectedArtID := model.NewArtworkID(model.KindDiscArtwork, "album1:1", &updatedAt) + Expect(result[0].CoverArt).To(Equal(expectedArtID.String())) }) - It("should return correct disc titles when album has discs with valid disc numbers", func() { + It("should return correct disc titles with cover art when album has multiple discs", func() { + updatedAt := time.Now().Truncate(time.Second) album := model.Album{ + ID: "album1", + UpdatedAt: updatedAt, Discs: map[int]string{ 1: "Disc 1", 2: "Disc 2", }, } - expected := []responses.DiscTitle{ - {Disc: 1, Title: "Disc 1"}, - {Disc: 2, Title: "Disc 2"}, - } - Expect(buildDiscSubtitles(album)).To(Equal(expected)) + result := buildDiscSubtitles(album) + Expect(result).To(HaveLen(2)) + Expect(result[0].Disc).To(Equal(int32(1))) + Expect(result[0].Title).To(Equal("Disc 1")) + expectedArtID1 := model.NewArtworkID(model.KindDiscArtwork, "album1:1", &updatedAt) + Expect(result[0].CoverArt).To(Equal(expectedArtID1.String())) + Expect(result[1].Disc).To(Equal(int32(2))) + Expect(result[1].Title).To(Equal("Disc 2")) + expectedArtID2 := model.NewArtworkID(model.KindDiscArtwork, "album1:2", &updatedAt) + Expect(result[1].CoverArt).To(Equal(expectedArtID2.String())) }) }) diff --git a/server/subsonic/responses/responses.go b/server/subsonic/responses/responses.go index be59e585..c550ce61 100644 --- a/server/subsonic/responses/responses.go +++ b/server/subsonic/responses/responses.go @@ -575,8 +575,9 @@ func (r ReplayGain) MarshalXML(e *xml.Encoder, start xml.StartElement) error { } type DiscTitle struct { - Disc int32 `xml:"disc,attr" json:"disc"` - Title string `xml:"title,attr" json:"title"` + Disc int32 `xml:"disc,attr" json:"disc"` + Title string `xml:"title,attr" json:"title"` + CoverArt string `xml:"coverArt,attr,omitempty" json:"coverArt,omitempty"` } type ItemDate struct { diff --git a/ui/src/common/SongDatagrid.jsx b/ui/src/common/SongDatagrid.jsx index 3586cf22..fbf2b9e3 100644 --- a/ui/src/common/SongDatagrid.jsx +++ b/ui/src/common/SongDatagrid.jsx @@ -1,4 +1,10 @@ -import React, { isValidElement, useMemo, useCallback, forwardRef } from 'react' +import React, { + isValidElement, + useMemo, + useCallback, + useState, + forwardRef, +} from 'react' import { useDispatch } from 'react-redux' import { Datagrid, @@ -17,7 +23,10 @@ import { makeStyles } from '@material-ui/core/styles' import AlbumIcon from '@material-ui/icons/Album' import clsx from 'clsx' import { useDrag } from 'react-dnd' +import Lightbox from 'react-image-lightbox' +import 'react-image-lightbox/style.css' import { playTracks } from '../actions' +import subsonic from '../subsonic' import { AlbumContextMenu } from '../common' import { DraggableTypes } from '../consts' import { formatFullDate } from '../utils' @@ -28,10 +37,20 @@ const useStyles = makeStyles({ overflow: 'hidden', textOverflow: 'ellipsis', verticalAlign: 'middle', + display: 'flex', + alignItems: 'center', }, discIcon: { - verticalAlign: 'text-top', - marginRight: '4px', + marginRight: '14px', + }, + discCoverArt: { + width: '48px', + height: '48px', + marginRight: '14px', + objectFit: 'cover', + borderRadius: '4px', + flexShrink: 0, + cursor: 'pointer', }, row: { cursor: 'pointer', @@ -61,19 +80,45 @@ const useStyles = makeStyles({ const DiscSubtitleRow = forwardRef( ({ record, onClick, colSpan, contextAlwaysVisible }, ref) => { + const translate = useTranslate() const isDesktop = useMediaQuery((theme) => theme.breakpoints.up('md')) const classes = useStyles({ isDesktop }) + const [imageError, setImageError] = useState(false) + const [isLightboxOpen, setLightboxOpen] = useState(false) const handlePlaySubset = (discNumber) => () => { onClick(discNumber) } - let subtitle = [] - if (record.discNumber > 0) { - subtitle.push(record.discNumber) - } - if (record.discSubtitle) { - subtitle.push(record.discSubtitle) - } + const coverArtUrl = subsonic.getDiscCoverArtUrl( + record.albumId, + record.discNumber, + record.updatedAt, + 96, + ) + + const fullImageUrl = subsonic.getDiscCoverArtUrl( + record.albumId, + record.discNumber, + record.updatedAt, + ) + + const handleOpenLightbox = useCallback( + (e) => { + if (!imageError) { + e.stopPropagation() + setLightboxOpen(true) + } + }, + [imageError], + ) + + const handleCloseLightbox = useCallback(() => setLightboxOpen(false), []) + + const subtitle = record.discSubtitle + ? record.discSubtitle + : translate('resources.song.fields.disc', { + discNumber: record.discNumber, + }) return ( - - {subtitle.join(': ')} + {!imageError ? ( + setImageError(true)} + /> + ) : ( + + )} + {subtitle} + {isLightboxOpen && !imageError && ( + + )} { } } +const getDiscCoverArtUrl = (albumId, discNumber, updatedAt, size) => { + const options = { + ...(updatedAt && { _: updatedAt }), + ...(size && { size }), + } + return baseUrl( + url('getCoverArt', 'dc-' + albumId + ':' + discNumber, options), + ) +} + const getArtistInfo = (id) => { return httpClient(url('getArtistInfo', id)) } @@ -129,6 +139,7 @@ export default { getScanStatus, getNowPlaying, getCoverArtUrl, + getDiscCoverArtUrl, getAvatarUrl, streamUrl, getAlbumInfo, diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index 1e0fbeaa..2544e03f 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -105,6 +105,56 @@ describe('getCoverArtUrl', () => { }) }) +describe('getDiscCoverArtUrl', () => { + beforeEach(() => { + const localStorageMock = { + getItem: vi.fn((key) => { + const values = { + username: 'testuser', + 'subsonic-token': 'testtoken', + 'subsonic-salt': 'testsalt', + } + return values[key] || null + }), + } + Object.defineProperty(window, 'localStorage', { value: localStorageMock }) + }) + + it('should construct URL with dc-albumId:discNumber format, size, and cache param', () => { + const url = subsonic.getDiscCoverArtUrl( + 'album-123', + 2, + '2023-01-01T00:00:00Z', + 48, + ) + + expect(url).toContain('getCoverArt') + expect(url).toContain('id=dc-album-123%3A2') + expect(url).toContain('size=48') + expect(url).toContain('_=2023-01-01T00%3A00%3A00Z') + }) + + it('should handle missing updatedAt', () => { + const url = subsonic.getDiscCoverArtUrl('album-123', 1, undefined, 48) + + expect(url).toContain('id=dc-album-123%3A1') + expect(url).toContain('size=48') + expect(url).not.toContain('_=') + }) + + it('should handle missing size', () => { + const url = subsonic.getDiscCoverArtUrl( + 'album-123', + 1, + '2023-01-01T00:00:00Z', + ) + + expect(url).toContain('id=dc-album-123%3A1') + expect(url).toContain('_=2023-01-01T00%3A00%3A00Z') + expect(url).not.toContain('size=') + }) +}) + describe('getAvatarUrl', () => { beforeEach(() => { // Mock localStorage values required by subsonic