From cbd56d7e46b6d390fbe7d15337ee834ccfe22031 Mon Sep 17 00:00:00 2001 From: paddy <106592312+BorisLord@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:04:57 +0200 Subject: [PATCH 1/3] feat(artwork): add album image listing and indexed cover art --- cmd/wire_gen.go | 2 +- core/artwork/artwork.go | 43 ++++++ core/artwork/artwork_internal_test.go | 76 +++++++++++ core/artwork/cache_warmer_test.go | 4 + core/artwork/reader_album.go | 166 +++++++++++++++++++++-- core/artwork/reader_album_test.go | 71 ++++++++++ model/artwork_id.go | 22 +++ model/artwork_id_test.go | 28 ++++ server/e2e/e2e_suite_test.go | 4 + server/nativeapi/album_images.go | 41 ++++++ server/nativeapi/album_images_test.go | 76 +++++++++++ server/nativeapi/config_test.go | 2 +- server/nativeapi/library_test.go | 2 +- server/nativeapi/native_api.go | 11 +- server/nativeapi/native_api_song_test.go | 2 +- server/nativeapi/playlists_test.go | 2 +- server/nativeapi/plugin_test.go | 2 +- 17 files changed, 533 insertions(+), 21 deletions(-) create mode 100644 server/nativeapi/album_images.go create mode 100644 server/nativeapi/album_images_test.go diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index 0939eef4d..35f913ada 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -82,7 +82,7 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { library := core.NewLibrary(dataStore, modelScanner, watcher, broker, manager) user := core.NewUser(dataStore, manager) maintenance := core.NewMaintenance(dataStore) - router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, imageUploadService) + router := nativeapi.New(dataStore, share, playlistsPlaylists, insights, library, user, maintenance, manager, imageUploadService, artworkArtwork) return router } diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index b8c395c12..cd4c443e0 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -7,6 +7,7 @@ import ( "io" "time" + "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core/external" "github.com/navidrome/navidrome/core/ffmpeg" @@ -22,6 +23,16 @@ var ErrUnavailable = errors.New("artwork unavailable") type Artwork interface { Get(ctx context.Context, artID model.ArtworkID, size int, square bool) (io.ReadCloser, time.Time, error) GetOrPlaceholder(ctx context.Context, id string, size int, square bool) (io.ReadCloser, time.Time, error) + // AlbumImages lists the album's images: primary cover first, then recognized + // scans (back, booklet, ...). Each CoverArt is a getCoverArt id. + AlbumImages(ctx context.Context, albumID string) ([]AlbumImageInfo, error) +} + +// AlbumImageInfo describes one album image for the native API gallery. +type AlbumImageInfo struct { + CoverArt string `json:"coverArt"` + Type string `json:"type"` + Name string `json:"name,omitempty"` } func NewArtwork(ds model.DataStore, cache cache.FileCache, ffmpeg ffmpeg.FFmpeg, provider external.Provider) Artwork { @@ -73,6 +84,38 @@ func (a *artwork) Get(ctx context.Context, artID model.ArtworkID, size int, squa return r, artReader.LastUpdated(), nil } +func (a *artwork) AlbumImages(ctx context.Context, albumID string) ([]AlbumImageInfo, error) { + al, err := a.ds.Album(ctx).Get(albumID) + if err != nil { + return nil, err + } + _, imgFiles, _, err := loadAlbumFoldersPaths(ctx, a.ds, *al) + if err != nil { + return nil, err + } + images := recognizedAlbumImages(imgFiles) + coverFile := resolveCoverFile(imgFiles, conf.Server.CoverArtPriority) + + // Slide 0 is the primary cover (al-), same as the thumbnail; works even + // when the cover is embedded and has no external file. + result := []AlbumImageInfo{{ + CoverArt: model.NewArtworkID(model.KindAlbumArtwork, albumID, &al.UpdatedAt).String(), + Type: "Front", + }} + for i, img := range images { + if img.Path == coverFile { + continue // already shown as slide 0 + } + id := model.NewArtworkID(model.KindAlbumArtwork, model.AlbumImageArtworkID(albumID, i), &al.UpdatedAt) + result = append(result, AlbumImageInfo{ + CoverArt: id.String(), + Type: img.Type, + Name: img.Name, + }) + } + return result, nil +} + type coverArtGetter interface { CoverArtID() model.ArtworkID } diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index c95371959..bbb52ec02 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -69,6 +69,82 @@ var _ = Describe("Artwork", func() { aw = NewArtwork(ds, cache, ffmpeg, nil).(*artwork) }) + Describe("AlbumImages", func() { + BeforeEach(func() { + conf.Server.CoverArtPriority = "cover.*, folder.*, front.*, embedded, external" + }) + + It("lists the primary cover plus recognized externals, deduping the cover file", func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "al1", LibraryID: 0, FolderIDs: []string{"f1"}}, + }) + folderRepo.result = []model.Folder{ + {ID: "f1", Path: "Artist", Name: "Album", ImageFiles: []string{"cover.jpg", "back.jpg", "booklet.jpg", "toto.jpg"}}, + } + + imgs, err := aw.AlbumImages(ctx, "al1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgs).To(HaveLen(3)) // primary + back + booklet; cover.jpg deduped, toto.jpg excluded + Expect(imgs[0].Type).To(Equal("Front")) + Expect(imgs[0].CoverArt).To(HavePrefix("al-al1_")) // primary id, no image index + Expect(imgs[1].Type).To(Equal("Back")) + Expect(imgs[1].Name).To(Equal("back.jpg")) + Expect(imgs[1].CoverArt).To(ContainSubstring("al-al1:1")) + Expect(imgs[2].Type).To(Equal("Booklet")) + Expect(imgs[2].CoverArt).To(ContainSubstring("al-al1:2")) + }) + + It("keeps external front images when the cover resolves to a non-file source", func() { + // art.jpg is Front-typed but matches no priority pattern โ†’ must not be dropped. + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "al2", LibraryID: 0, FolderIDs: []string{"f1"}}, + }) + folderRepo.result = []model.Folder{ + {ID: "f1", Path: "Artist", Name: "Album", ImageFiles: []string{"art.jpg", "back.jpg"}}, + } + + imgs, err := aw.AlbumImages(ctx, "al2") + + Expect(err).ToNot(HaveOccurred()) + names := make([]string, 0, len(imgs)-1) + for _, im := range imgs[1:] { + names = append(names, im.Name) + } + Expect(names).To(ConsistOf("art.jpg", "back.jpg")) + }) + + It("returns just the primary cover when there are no recognized images", func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "al3", LibraryID: 0, FolderIDs: []string{"f1"}}, + }) + folderRepo.result = []model.Folder{ + {ID: "f1", Path: "Artist", Name: "Album", ImageFiles: []string{"toto.jpg"}}, + } + + imgs, err := aw.AlbumImages(ctx, "al3") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgs).To(HaveLen(1)) + Expect(imgs[0].Type).To(Equal("Front")) + }) + }) + + Describe("indexed album image reader", func() { + It("returns ErrNotFound for an out-of-range image index", func() { + ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "al9", LibraryID: 0, FolderIDs: []string{"f1"}}, + }) + folderRepo.result = []model.Folder{ + {ID: "f1", Path: "Artist", Name: "Album", ImageFiles: []string{"cover.jpg"}}, + } + + _, err := newAlbumArtworkReader(ctx, aw, model.MustParseArtworkID("al-al9:99"), nil) + + Expect(err).To(MatchError(model.ErrNotFound)) + }) + }) + Describe("albumArtworkReader", func() { Context("ID not found", func() { It("returns ErrNotFound if album is not in the DB", func() { diff --git a/core/artwork/cache_warmer_test.go b/core/artwork/cache_warmer_test.go index a5da2004c..4112ddfb0 100644 --- a/core/artwork/cache_warmer_test.go +++ b/core/artwork/cache_warmer_test.go @@ -214,6 +214,10 @@ func (m *mockArtwork) GetOrPlaceholder(ctx context.Context, id string, size int, return m.Get(ctx, model.ArtworkID{}, size, square) } +func (m *mockArtwork) AlbumImages(context.Context, string) ([]AlbumImageInfo, error) { + return nil, nil +} + type mockFileCache struct { disabled atomic.Bool ready atomic.Bool diff --git a/core/artwork/reader_album.go b/core/artwork/reader_album.go index 73ba9b5ee..446bca898 100644 --- a/core/artwork/reader_album.go +++ b/core/artwork/reader_album.go @@ -24,16 +24,21 @@ import ( type albumArtworkReader struct { cacheKey - a *artwork - provider external.Provider - album model.Album - updatedAt *time.Time - imgFiles []string // library-relative, forward-slash, no leading slash - lib libraryView + a *artwork + provider external.Provider + album model.Album + updatedAt *time.Time + imgFiles []string // library-relative, forward-slash, no leading slash + lib libraryView + imageIndex int // -1 = use cover-art priority; >=0 = serve the Nth recognized image } func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*albumArtworkReader, error) { - al, err := artwork.ds.Album(ctx).Get(artID.ID) + albumID, imageIndex, err := model.ParseAlbumArtworkID(artID.ID) + if err != nil { + return nil, err + } + al, err := artwork.ds.Album(ctx).Get(albumID) if err != nil { return nil, err } @@ -41,17 +46,21 @@ func newAlbumArtworkReader(ctx context.Context, artwork *artwork, artID model.Ar if err != nil { return nil, err } + if imageIndex >= 0 && imageIndex >= len(recognizedAlbumImages(imgFiles)) { + return nil, model.ErrNotFound + } lib, err := loadLibraryView(ctx, artwork.ds, al.LibraryID) if err != nil { return nil, err } a := &albumArtworkReader{ - a: artwork, - provider: provider, - album: *al, - updatedAt: imagesUpdateAt, - imgFiles: imgFiles, - lib: lib, + a: artwork, + provider: provider, + album: *al, + updatedAt: imagesUpdateAt, + imgFiles: imgFiles, + lib: lib, + imageIndex: imageIndex, } a.cacheKey.artID = artID a.cacheKey.lastUpdate = utils.TimeNewest(al.UpdatedAt, al.ImportedAt) @@ -79,10 +88,29 @@ func (a *albumArtworkReader) LastUpdated() time.Time { } func (a *albumArtworkReader) Reader(ctx context.Context) (io.ReadCloser, string, error) { + if a.imageIndex >= 0 { + return selectImageReader(ctx, a.artID, a.fromImageIndex(ctx, a.imageIndex)) + } var ff = a.fromCoverArtPriority(ctx, a.a.ffmpeg, conf.Server.CoverArtPriority) return selectImageReader(ctx, a.artID, ff...) } +// fromImageIndex serves the Nth recognized external image (bypassing priority). +func (a *albumArtworkReader) fromImageIndex(ctx context.Context, index int) sourceFunc { + return func() (io.ReadCloser, string, error) { + images := recognizedAlbumImages(a.imgFiles) + if index < 0 || index >= len(images) { + return nil, "", fmt.Errorf("album image index %d out of range (%d images): %w", index, len(images), model.ErrNotFound) + } + file := images[index].Path + f, err := a.lib.FS.Open(file) + if err != nil { + return nil, "", err + } + return f, file, nil + } +} + func (a *albumArtworkReader) fromCoverArtPriority(ctx context.Context, ffmpeg ffmpeg.FFmpeg, priority string) []sourceFunc { var ff []sourceFunc for pattern := range strings.SplitSeq(strings.ToLower(priority), ",") { @@ -186,6 +214,118 @@ func commonParentFolder(folders []model.Folder, folderIDSet map[string]bool) str return parentID } +// albumImage is a recognized external image file with its inferred type. +type albumImage struct { + Path string // library-relative, forward-slash + Name string // base filename + Type string // official MusicBrainz CAA type (Front, Back, Booklet, Medium, ...) +} + +// albumImageTypes maps filename stems to the official MusicBrainz CAA type +// (https://musicbrainz.org/doc/Cover_Art/Types). Slice order is the gallery +// display order; non-Front types match before Front so "back cover" โ†’ Back. +var albumImageTypes = []struct { + Type string + stems []string +}{ + {"Front", []string{"front", "cover", "folder", "album", "albumart", "art"}}, + {"Back", []string{"back"}}, + {"Booklet", []string{"booklet", "leaflet", "inlay", "inside"}}, + {"Medium", []string{"medium", "media", "disc", "discart", "disque", "cd", "cdart"}}, + {"Tray", []string{"tray"}}, + {"Obi", []string{"obi"}}, + {"Spine", []string{"spine"}}, + {"Track", []string{"track"}}, + {"Liner", []string{"liner"}}, + {"Sticker", []string{"sticker"}}, + {"Poster", []string{"poster"}}, + {"Matrix/Runout", []string{"matrix", "runout"}}, + {"Top", []string{"top"}}, + {"Bottom", []string{"bottom"}}, + {"Panel", []string{"panel", "gatefold"}}, + {"Watermark", []string{"watermark"}}, + {"Raw/Unedited", []string{"raw", "unedited"}}, + {"Other", []string{"other"}}, +} + +// imageTypeRank maps each type to its display order, derived from albumImageTypes. +var imageTypeRank = func() map[string]int { + m := make(map[string]int, len(albumImageTypes)) + for i, t := range albumImageTypes { + m[t.Type] = i + } + return m +}() + +// imageTypeFromName infers the CAA type from a filename (numeric suffixes +// tolerated); returns "" for unrecognized names. +func imageTypeFromName(name string) string { + stem := strings.ToLower(strings.TrimSuffix(name, path.Ext(name))) + fields := strings.FieldsFunc(stem, func(r rune) bool { + return r == ' ' || r == '.' || r == '-' || r == '_' + }) + for i, f := range fields { + fields[i] = strings.TrimRight(f, "0123456789") + } + matches := func(stems []string) bool { + for _, f := range fields { + if slices.Contains(stems, f) { + return true + } + } + return false + } + // A specific (non-Front) type wins over a generic front token. + for _, t := range albumImageTypes { + if t.Type == "Front" { + continue + } + if matches(t.stems) { + return t.Type + } + } + if matches(albumImageTypes[0].stems) { // Front + return "Front" + } + return "" +} + +// resolveCoverFile returns the external file the primary cover (al-) resolves +// to, or "" if it comes from a non-file source. Lets AlbumImages skip that file. +func resolveCoverFile(imgFiles []string, priority string) string { + for pattern := range strings.SplitSeq(strings.ToLower(priority), ",") { + pattern = strings.TrimSpace(pattern) + if pattern == "" || pattern == "embedded" || pattern == "external" { + continue + } + for _, f := range imgFiles { + if ok, _ := path.Match(pattern, strings.ToLower(path.Base(f))); ok { + return f + } + } + } + return "" +} + +// recognizedAlbumImages returns the album's recognized-type images, ordered by +// type then filename. Single source of truth for the indexed fetch and listing. +func recognizedAlbumImages(imgFiles []string) []albumImage { + var images []albumImage + for _, f := range imgFiles { + name := path.Base(f) + if t := imageTypeFromName(name); t != "" { + images = append(images, albumImage{Path: f, Name: name, Type: t}) + } + } + slices.SortStableFunc(images, func(a, b albumImage) int { + return cmp.Or( + cmp.Compare(imageTypeRank[a.Type], imageTypeRank[b.Type]), + compareImageFiles(a.Path, b.Path), + ) + }) + return images +} + // compareImageFiles sorts image paths by: base filename (natural order), // then path depth (shallower first), then full path (stable tiebreaker). func compareImageFiles(a, b string) int { diff --git a/core/artwork/reader_album_test.go b/core/artwork/reader_album_test.go index 1cf039bee..8bc87529e 100644 --- a/core/artwork/reader_album_test.go +++ b/core/artwork/reader_album_test.go @@ -396,4 +396,75 @@ var _ = Describe("Album Artwork Reader", func() { Expect(repo.getCallCount).To(Equal(1)) }) }) + + Describe("imageTypeFromName", func() { + DescribeTable("infers the official MusicBrainz CAA image type from a filename", + func(name, expected string) { + Expect(imageTypeFromName(name)).To(Equal(expected)) + }, + Entry("cover.jpg", "cover.jpg", "Front"), + Entry("front.png", "front.png", "Front"), + Entry("folder.jpg", "folder.jpg", "Front"), + Entry("Front Cover.jpg", "Front Cover.jpg", "Front"), + Entry("numeric front cover.1.jpg", "cover.1.jpg", "Front"), + Entry("back.jpg", "back.jpg", "Back"), + Entry("Back Cover.jpg", "Back Cover.jpg", "Back"), + Entry("booklet.jpg", "booklet.jpg", "Booklet"), + Entry("booklet-01.jpg", "booklet-01.jpg", "Booklet"), + Entry("leaflet.png", "leaflet.png", "Booklet"), + Entry("disc.png", "disc.png", "Medium"), + Entry("cd1.jpg", "cd1.jpg", "Medium"), + Entry("discart.png", "discart.png", "Medium"), + Entry("medium.jpg", "medium.jpg", "Medium"), + Entry("tray.jpg", "tray.jpg", "Tray"), + Entry("obi.jpg", "obi.jpg", "Obi"), + Entry("spine.jpg", "spine.jpg", "Spine"), + Entry("sticker.png", "sticker.png", "Sticker"), + Entry("matrix.jpg", "matrix.jpg", "Matrix/Runout"), + Entry("case-insensitive BACK.JPG", "BACK.JPG", "Back"), + Entry("unrecognized toto.jpg", "toto.jpg", ""), + Entry("unrecognized scan001.jpg", "scan001.jpg", ""), + ) + }) + + Describe("resolveCoverFile", func() { + const prio = "cover.*, folder.*, front.*, embedded, external" + DescribeTable("returns the external file the primary cover resolves to", + func(files []string, expected string) { + Expect(resolveCoverFile(files, prio)).To(Equal(expected)) + }, + Entry("cover wins over folder/back", []string{"a/back.jpg", "a/cover.jpg", "a/folder.jpg"}, "a/cover.jpg"), + Entry("folder when no cover", []string{"a/folder.jpg", "a/back.jpg"}, "a/folder.jpg"), + Entry("front when no cover/folder", []string{"a/front.png", "a/back.jpg"}, "a/front.png"), + Entry("empty when only non-priority files", []string{"a/art.jpg", "a/back.jpg"}, ""), + Entry("empty when no files", []string{}, ""), + ) + }) + + Describe("recognizedAlbumImages", func() { + It("filters unrecognized names and orders by official type", func() { + imgFiles := []string{ + "Album/toto.jpg", + "Album/spine.jpg", + "Album/back.jpg", + "Album/disc.png", + "Album/cover.jpg", + "Album/booklet.jpg", + } + images := recognizedAlbumImages(imgFiles) + + Expect(images).To(HaveLen(5)) // toto.jpg excluded + types := make([]string, len(images)) + for i, img := range images { + types[i] = img.Type + } + Expect(types).To(Equal([]string{"Front", "Back", "Booklet", "Medium", "Spine"})) + Expect(images[0].Path).To(Equal("Album/cover.jpg")) + Expect(images[0].Name).To(Equal("cover.jpg")) + }) + + It("returns nil when no image is recognized", func() { + Expect(recognizedAlbumImages([]string{"Album/toto.jpg", "Album/random.png"})).To(BeEmpty()) + }) + }) }) diff --git a/model/artwork_id.go b/model/artwork_id.go index 1bd146c1f..9650afcc5 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -99,6 +99,28 @@ func DiscArtworkID(albumID string, discNumber int) string { return fmt.Sprintf("%s:%d", albumID, discNumber) } +// AlbumImageArtworkID builds the album-image ID portion ":" (mirrors DiscArtworkID). +func AlbumImageArtworkID(albumID string, index int) string { + return fmt.Sprintf("%s:%d", albumID, index) +} + +// ParseAlbumArtworkID splits "" or ":" into the album ID +// and image index (-1 when no index, i.e. use cover-art priority). +func ParseAlbumArtworkID(id string) (albumID string, index int, err error) { + albumID, idxStr, found := strings.Cut(id, ":") + if !found { + return id, -1, nil + } + index, err = strconv.Atoi(idxStr) + if err != nil { + return "", 0, fmt.Errorf("invalid image index in artwork id %q: %w", id, err) + } + if index < 0 { + return "", 0, fmt.Errorf("invalid image index in artwork id %q", id) + } + return albumID, index, nil +} + func ParseDiscArtworkID(id string) (albumID string, discNumber int, err error) { parts := strings.SplitN(id, ":", 2) if len(parts) != 2 || parts[1] == "" { diff --git a/model/artwork_id_test.go b/model/artwork_id_test.go index ad66f7bb5..07ecf4baf 100644 --- a/model/artwork_id_test.go +++ b/model/artwork_id_test.go @@ -61,6 +61,34 @@ var _ = Describe("ArtworkID", func() { ) }) + Describe("ParseAlbumArtworkID", func() { + DescribeTable("parses album artwork IDs with optional image index", + func(id string, expectedAlbum string, expectedIndex int, expectErr bool) { + albumID, index, err := model.ParseAlbumArtworkID(id) + if expectErr { + Expect(err).To(HaveOccurred()) + } else { + Expect(err).ToNot(HaveOccurred()) + Expect(albumID).To(Equal(expectedAlbum)) + Expect(index).To(Equal(expectedIndex)) + } + }, + Entry("no index", "albumid123", "albumid123", -1, false), + Entry("index 0", "albumid123:0", "albumid123", 0, false), + Entry("index 3", "albumid123:3", "albumid123", 3, false), + Entry("large index", "abc:10", "abc", 10, false), + Entry("non-numeric index", "abc:foo", "", 0, true), + Entry("negative index", "abc:-1", "", 0, true), + Entry("empty index", "abc:", "", 0, true), + ) + It("round-trips through AlbumImageArtworkID", func() { + albumID, index, err := model.ParseAlbumArtworkID(model.AlbumImageArtworkID("abc", 2)) + Expect(err).ToNot(HaveOccurred()) + Expect(albumID).To(Equal("abc")) + Expect(index).To(Equal(2)) + }) + }) + Describe("ParseArtworkID()", func() { It("parses album artwork ids", func() { id, err := model.ParseArtworkID("al-1234") diff --git a/server/e2e/e2e_suite_test.go b/server/e2e/e2e_suite_test.go index 7ce8de2e6..6113906a2 100644 --- a/server/e2e/e2e_suite_test.go +++ b/server/e2e/e2e_suite_test.go @@ -289,6 +289,10 @@ func (n noopArtwork) GetOrPlaceholder(_ context.Context, _ string, _ int, _ bool return io.NopCloser(io.LimitReader(nil, 0)), time.Time{}, nil } +func (n noopArtwork) AlbumImages(context.Context, string) ([]artwork.AlbumImageInfo, error) { + return nil, model.ErrNotFound +} + // spyStreamer captures the Request passed to NewStream for test assertions, // then returns a minimal fake Stream so the handler completes without error. type spyStreamer struct { diff --git a/server/nativeapi/album_images.go b/server/nativeapi/album_images.go new file mode 100644 index 000000000..99d0eebf9 --- /dev/null +++ b/server/nativeapi/album_images.go @@ -0,0 +1,41 @@ +package nativeapi + +import ( + "encoding/json" + "errors" + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" +) + +// albumImages serves an album's images (primary cover + recognized scans). Each +// coverArt is a getCoverArt id. +func albumImages(aw artwork.Artwork) http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + id := chi.URLParam(r, "id") + if id == "" { + http.Error(w, "missing id", http.StatusBadRequest) + return + } + + images, err := aw.AlbumImages(ctx, id) + if errors.Is(err, model.ErrNotFound) { + http.Error(w, "not found", http.StatusNotFound) + return + } + if err != nil { + log.Error(ctx, "Error listing album images", "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + w.Header().Set("Content-Type", "application/json") + if err := json.NewEncoder(w).Encode(images); err != nil { + log.Error(ctx, "Error sending album images response", "id", id, err) + } + } +} diff --git a/server/nativeapi/album_images_test.go b/server/nativeapi/album_images_test.go new file mode 100644 index 000000000..37f0439cf --- /dev/null +++ b/server/nativeapi/album_images_test.go @@ -0,0 +1,76 @@ +package nativeapi + +import ( + "context" + "io" + "net/http" + "net/http/httptest" + "time" + + "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/core/artwork" + "github.com/navidrome/navidrome/model" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +type fakeArtwork struct { + images []artwork.AlbumImageInfo + err error +} + +func (f *fakeArtwork) Get(context.Context, model.ArtworkID, int, bool) (io.ReadCloser, time.Time, error) { + return nil, time.Time{}, nil +} + +func (f *fakeArtwork) GetOrPlaceholder(context.Context, string, int, bool) (io.ReadCloser, time.Time, error) { + return nil, time.Time{}, nil +} + +func (f *fakeArtwork) AlbumImages(context.Context, string) ([]artwork.AlbumImageInfo, error) { + return f.images, f.err +} + +var _ = Describe("albumImages handler", func() { + var ( + router http.Handler + aw *fakeArtwork + ) + + BeforeEach(func() { + aw = &fakeArtwork{} + api := &Router{artwork: aw} + r := chi.NewRouter() + api.addAlbumImagesRoute(r) + router = r + }) + + doGet := func(path string) *httptest.ResponseRecorder { + w := httptest.NewRecorder() + router.ServeHTTP(w, httptest.NewRequest(http.MethodGet, path, nil)) + return w + } + + It("returns the list of album images as JSON", func() { + aw.images = []artwork.AlbumImageInfo{ + {CoverArt: "al-abc_0", Type: "Front"}, + {CoverArt: "al-abc:1_0", Type: "Back", Name: "back.jpg"}, + } + + w := doGet("/album/abc/images") + + Expect(w.Code).To(Equal(http.StatusOK)) + Expect(w.Header().Get("Content-Type")).To(ContainSubstring("application/json")) + Expect(w.Body.String()).To(ContainSubstring(`"coverArt":"al-abc:1_0"`)) + Expect(w.Body.String()).To(ContainSubstring(`"type":"Back"`)) + Expect(w.Body.String()).To(ContainSubstring(`"name":"back.jpg"`)) + }) + + It("returns 404 when the album is not found", func() { + aw.err = model.ErrNotFound + + w := doGet("/album/missing/images") + + Expect(w.Code).To(Equal(http.StatusNotFound)) + }) +}) diff --git a/server/nativeapi/config_test.go b/server/nativeapi/config_test.go index 4e6e9e89b..ffd8881c3 100644 --- a/server/nativeapi/config_test.go +++ b/server/nativeapi/config_test.go @@ -28,7 +28,7 @@ var _ = Describe("Config API", func() { conf.Server.DevUIShowConfig = true // Enable config endpoint for tests ds = &tests.MockDataStore{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil) router = server.JWTVerifier(nativeRouter) // Create test users diff --git a/server/nativeapi/library_test.go b/server/nativeapi/library_test.go index ed5564a41..1708a5aba 100644 --- a/server/nativeapi/library_test.go +++ b/server/nativeapi/library_test.go @@ -29,7 +29,7 @@ var _ = Describe("Library API", func() { DeferCleanup(configtest.SetupConfig()) ds = &tests.MockDataStore{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil) router = server.JWTVerifier(nativeRouter) // Create test users diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 669c4d7b5..8e6082d2f 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -13,6 +13,7 @@ import ( "github.com/go-chi/chi/v5/middleware" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/core/artwork" "github.com/navidrome/navidrome/core/metrics" playlistsvc "github.com/navidrome/navidrome/core/playlists" "github.com/navidrome/navidrome/log" @@ -45,10 +46,11 @@ type Router struct { maintenance core.Maintenance pluginManager PluginManager imgUpload core.ImageUploadService + artwork artwork.Artwork } -func New(ds model.DataStore, share core.Share, playlists playlistsvc.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager, imgUpload core.ImageUploadService) *Router { - r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, users: userService, maintenance: maintenance, pluginManager: pluginManager, imgUpload: imgUpload} +func New(ds model.DataStore, share core.Share, playlists playlistsvc.Playlists, insights metrics.Insights, libraryService core.Library, userService core.User, maintenance core.Maintenance, pluginManager PluginManager, imgUpload core.ImageUploadService, artwork artwork.Artwork) *Router { + r := &Router{ds: ds, share: share, playlists: playlists, insights: insights, libs: libraryService, users: userService, maintenance: maintenance, pluginManager: pluginManager, imgUpload: imgUpload, artwork: artwork} r.Handler = r.routes() return r } @@ -80,6 +82,7 @@ func (api *Router) routes() http.Handler { api.addPlaylistRoute(r) api.addPlaylistTrackRoute(r) api.addSongPlaylistsRoute(r) + api.addAlbumImagesRoute(r) api.addQueueRoute(r) api.addMissingFilesRoute(r) api.addKeepAliveRoute(r) @@ -175,6 +178,10 @@ func (api *Router) addPlaylistTrackRoute(r chi.Router) { }) } +func (api *Router) addAlbumImagesRoute(r chi.Router) { + r.Get("/album/{id}/images", albumImages(api.artwork)) +} + func (api *Router) addSongPlaylistsRoute(r chi.Router) { r.With(server.URLParamsMiddleware).Get("/song/{id}/playlists", func(w http.ResponseWriter, r *http.Request) { getSongPlaylists(api.playlists)(w, r) diff --git a/server/nativeapi/native_api_song_test.go b/server/nativeapi/native_api_song_test.go index f0ee50ebb..e1e02d7ee 100644 --- a/server/nativeapi/native_api_song_test.go +++ b/server/nativeapi/native_api_song_test.go @@ -94,7 +94,7 @@ var _ = Describe("Song Endpoints", func() { mfRepo.SetData(testSongs) // Create the native API router and wrap it with the JWTVerifier middleware - nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil) router = server.JWTVerifier(nativeRouter) w = httptest.NewRecorder() }) diff --git a/server/nativeapi/playlists_test.go b/server/nativeapi/playlists_test.go index e1c933709..b7da533b5 100644 --- a/server/nativeapi/playlists_test.go +++ b/server/nativeapi/playlists_test.go @@ -98,7 +98,7 @@ var _ = Describe("Playlist Tracks Endpoint", func() { err := userRepo.Put(&testUser) Expect(err).ToNot(HaveOccurred()) - nativeRouter := New(ds, nil, plsSvc, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + nativeRouter := New(ds, nil, plsSvc, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil, nil) router = server.JWTVerifier(nativeRouter) w = httptest.NewRecorder() }) diff --git a/server/nativeapi/plugin_test.go b/server/nativeapi/plugin_test.go index 8fc88e09c..fe73e7a5e 100644 --- a/server/nativeapi/plugin_test.go +++ b/server/nativeapi/plugin_test.go @@ -33,7 +33,7 @@ var _ = Describe("Plugin API", func() { ds = &tests.MockDataStore{} mockManager = &tests.MockPluginManager{} auth.Init(ds) - nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, mockManager, nil) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, mockManager, nil, nil) router = server.JWTVerifier(nativeRouter) // Create test users From c8f0f6f9dd8457fa7090b339e0d4f2081f89b1e9 Mon Sep 17 00:00:00 2001 From: paddy <106592312+BorisLord@users.noreply.github.com> Date: Thu, 11 Jun 2026 23:06:04 +0200 Subject: [PATCH 2/3] feat(ui): add album image gallery carousel --- ui/src/album/AlbumDetails.jsx | 59 ++++++++++++++++++++-- ui/src/dataProvider/wrapperDataProvider.js | 5 ++ ui/src/subsonic/index.js | 7 +++ ui/src/subsonic/index.test.js | 31 ++++++++++++ 4 files changed, 97 insertions(+), 5 deletions(-) diff --git a/ui/src/album/AlbumDetails.jsx b/ui/src/album/AlbumDetails.jsx index cec66eb8b..de5869856 100644 --- a/ui/src/album/AlbumDetails.jsx +++ b/ui/src/album/AlbumDetails.jsx @@ -14,6 +14,7 @@ import { ChipField, Link, SingleFieldList, + useDataProvider, useRecordContext, useTranslate, } from 'react-admin' @@ -216,6 +217,9 @@ export const Details = (props) => { return <>{intersperse(details, ' ยท ')} } +// Bounded lightbox size: avoids transferring/caching multi-MB originals. +const GALLERY_IMAGE_SIZE = 1920 + const AlbumDetails = (props) => { const record = useRecordContext(props) const isXsmall = useMediaQuery((theme) => theme.breakpoints.down('xs')) @@ -233,6 +237,10 @@ const AlbumDetails = (props) => { handleCloseLightbox, } = useImageLoadingState(record.id) + const dataProvider = useDataProvider() + const [images, setImages] = useState([]) + const [photoIndex, setPhotoIndex] = useState(0) + let notes = albumInfo?.notes || record.notes if (notes) { @@ -255,7 +263,30 @@ const AlbumDetails = (props) => { }, [record]) const imageUrl = subsonic.getCoverArtUrl(record, config.uiCoverArtSize) - const fullImageUrl = subsonic.getCoverArtUrl(record) + const fullImageUrl = subsonic.getCoverArtUrl(record, GALLERY_IMAGE_SIZE) + + const galleryCount = images.length || 1 + const imageSrcFor = (i) => + images.length + ? subsonic.getImageCoverArtUrl(images[i].coverArt, GALLERY_IMAGE_SIZE) + : fullImageUrl + + const openGallery = () => { + if (imageError) return + dataProvider + .getAlbumImages(record.id) + .then(({ data }) => setImages(Array.isArray(data) ? data : [])) + .catch(() => setImages([])) + .finally(() => { + setPhotoIndex(0) + handleOpenLightbox() + }) + } + + const closeGallery = () => { + handleCloseLightbox() + setPhotoIndex(0) + } return ( @@ -268,7 +299,7 @@ const AlbumDetails = (props) => { width="400" height="400" className={`${classes.cover} ${imageLoading ? classes.coverLoading : ''}`} - onClick={handleOpenLightbox} + onClick={openGallery} onLoad={handleImageLoad} onError={handleImageError} title={record.name} @@ -367,9 +398,27 @@ const AlbumDetails = (props) => { 1 + ? imageSrcFor((photoIndex + 1) % galleryCount) + : undefined + } + prevSrc={ + galleryCount > 1 + ? imageSrcFor((photoIndex + galleryCount - 1) % galleryCount) + : undefined + } + onMoveNextRequest={() => setPhotoIndex((p) => (p + 1) % galleryCount)} + onMovePrevRequest={() => + setPhotoIndex((p) => (p + galleryCount - 1) % galleryCount) + } + onCloseRequest={closeGallery} /> )} diff --git a/ui/src/dataProvider/wrapperDataProvider.js b/ui/src/dataProvider/wrapperDataProvider.js index 268d3668d..5dd065479 100644 --- a/ui/src/dataProvider/wrapperDataProvider.js +++ b/ui/src/dataProvider/wrapperDataProvider.js @@ -220,6 +220,11 @@ const wrapperDataProvider = { data: json, })) }, + getAlbumImages: (albumId) => { + return httpClient(`${REST_URL}/album/${albumId}/images`).then( + ({ json }) => ({ data: json }), + ) + }, } export default wrapperDataProvider diff --git a/ui/src/subsonic/index.js b/ui/src/subsonic/index.js index 7d93972e0..2c0976fa7 100644 --- a/ui/src/subsonic/index.js +++ b/ui/src/subsonic/index.js @@ -113,6 +113,12 @@ const getDiscCoverArtUrl = (albumId, discNumber, updatedAt, size) => { ) } +// Builds a getCoverArt URL from a ready-made coverArt id (from /album/{id}/images). +const getImageCoverArtUrl = (coverArtId, size) => { + const options = { ...(size && { size }) } + return baseUrl(url('getCoverArt', coverArtId, options)) +} + const getArtistInfo = (id) => { return httpClient(url('getArtistInfo', id)) } @@ -152,6 +158,7 @@ export default { getNowPlaying, getCoverArtUrl, getDiscCoverArtUrl, + getImageCoverArtUrl, getAvatarUrl, streamUrl, getAlbumInfo, diff --git a/ui/src/subsonic/index.test.js b/ui/src/subsonic/index.test.js index ad4764c24..05e6e2dcd 100644 --- a/ui/src/subsonic/index.test.js +++ b/ui/src/subsonic/index.test.js @@ -172,6 +172,37 @@ describe('getDiscCoverArtUrl', () => { }) }) +describe('getImageCoverArtUrl', () => { + 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('builds a getCoverArt URL from a fully-formed indexed coverArt id', () => { + const url = subsonic.getImageCoverArtUrl('al-album-123:1_0', 300) + + expect(url).toContain('getCoverArt') + expect(url).toContain('id=al-album-123%3A1_0') + expect(url).toContain('size=300') + }) + + it('omits size when not provided', () => { + const url = subsonic.getImageCoverArtUrl('al-album-123_0') + + expect(url).toContain('id=al-album-123_0') + expect(url).not.toContain('size=') + }) +}) + describe('getAvatarUrl', () => { beforeEach(() => { // Mock localStorage values required by subsonic From 6dec905eab7613ccc854effdb6b05be6e515a480 Mon Sep 17 00:00:00 2001 From: paddy <106592312+BorisLord@users.noreply.github.com> Date: Fri, 12 Jun 2026 00:31:26 +0200 Subject: [PATCH 3/3] fix(ui): open gallery lightbox immediately --- ui/src/album/AlbumDetails.jsx | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/ui/src/album/AlbumDetails.jsx b/ui/src/album/AlbumDetails.jsx index de5869856..742232473 100644 --- a/ui/src/album/AlbumDetails.jsx +++ b/ui/src/album/AlbumDetails.jsx @@ -273,14 +273,13 @@ const AlbumDetails = (props) => { const openGallery = () => { if (imageError) return + setImages([]) + setPhotoIndex(0) + handleOpenLightbox() dataProvider .getAlbumImages(record.id) .then(({ data }) => setImages(Array.isArray(data) ? data : [])) .catch(() => setImages([])) - .finally(() => { - setPhotoIndex(0) - handleOpenLightbox() - }) } const closeGallery = () => {