diff --git a/model/artwork_id.go b/model/artwork_id.go index 15db720e5..e308cb676 100644 --- a/model/artwork_id.go +++ b/model/artwork_id.go @@ -17,6 +17,11 @@ func (k Kind) String() string { return k.name } +// Prefix is the short token used in artwork ids and the item_artwork.item_kind column. +func (k Kind) Prefix() string { + return k.prefix +} + var ( KindMediaFileArtwork = Kind{"mf", "media_file"} KindArtistArtwork = Kind{"ar", "artist"} diff --git a/persistence/album_repository.go b/persistence/album_repository.go index e7b155e6f..426048654 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -251,7 +251,21 @@ func (r *albumRepository) GetAll(options ...model.QueryOptions) (model.Albums, e if err != nil { return nil, err } - return res.toModels(), nil + albums := res.toModels() + r.hydrateArtwork(albums) + return albums, nil +} + +// hydrateArtwork fills each album's ImageHash/ImageAbsent from one batched item_artwork lookup. +func (r *albumRepository) hydrateArtwork(albums model.Albums) { + if len(albums) == 0 { + return + } + ids := slice.Map(albums, func(a model.Album) string { return a.ID }) + infos := hydrateItemImages(r.ctx, r.db, model.KindAlbumArtwork.Prefix(), ids) + for i := range albums { + applyItemImage(infos, albums[i].ID, &albums[i].ItemImage) + } } // GetAllIDs returns just the album IDs for the same row set as GetAll, skipping the @@ -414,7 +428,9 @@ func (r *albumRepository) Search(q string, options ...model.QueryOptions) (model if err != nil { return nil, fmt.Errorf("searching album %q: %w", q, err) } - return res.toModels(), nil + albums := res.toModels() + r.hydrateArtwork(albums) + return albums, nil } func (r *albumRepository) Count(options ...rest.QueryOptions) (int64, error) { diff --git a/persistence/artist_repository.go b/persistence/artist_repository.go index f26ade8ba..a78718de4 100644 --- a/persistence/artist_repository.go +++ b/persistence/artist_repository.go @@ -250,6 +250,7 @@ func (r *artistRepository) Get(id string) (*model.Artist, error) { return nil, model.ErrNotFound } res := dba.toModels() + r.hydrateArtwork(res) return &res[0], nil } @@ -261,6 +262,7 @@ func (r *artistRepository) GetAll(options ...model.QueryOptions) (model.Artists, return nil, err } res := dba.toModels() + r.hydrateArtwork(res) return res, err } @@ -273,6 +275,18 @@ func (r *artistRepository) GetAllIDs(options ...model.QueryOptions) ([]string, e return ids, err } +// hydrateArtwork fills each artist's ImageHash/ImageAbsent from one batched item_artwork lookup. +func (r *artistRepository) hydrateArtwork(artists model.Artists) { + if len(artists) == 0 { + return + } + ids := slice.Map(artists, func(a model.Artist) string { return a.ID }) + infos := hydrateItemImages(r.ctx, r.db, model.KindArtistArtwork.Prefix(), ids) + for i := range artists { + applyItemImage(infos, artists[i].ID, &artists[i].ItemImage) + } +} + func (r *artistRepository) GetCursor(options ...model.QueryOptions) (model.ArtistCursor, error) { sel := r.selectArtist(options...) cursor, err := queryWithStableResults[dbArtist](r.sqlRepository, sel) @@ -644,7 +658,9 @@ func (r *artistRepository) Search(q string, options ...model.QueryOptions) (mode if err != nil { return nil, fmt.Errorf("searching artist %q: %w", q, err) } - return res.toModels(), nil + artists := res.toModels() + r.hydrateArtwork(artists) + return artists, nil } // searchScope returns the library IDs the search must be restricted to, or nil to skip the filter diff --git a/persistence/artwork_hydration.go b/persistence/artwork_hydration.go new file mode 100644 index 000000000..bf1060055 --- /dev/null +++ b/persistence/artwork_hydration.go @@ -0,0 +1,31 @@ +package persistence + +import ( + "context" + + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/pocketbase/dbx" +) + +// hydrateItemImages returns per-item artwork info for a fetched page via one batched query per kind +// (never a join, see spec ยง6). On error it logs and returns an empty map so the page still renders. +func hydrateItemImages(ctx context.Context, db dbx.Builder, kind string, ids []string) map[string]model.ItemArtworkInfo { + if len(ids) == 0 { + return map[string]model.ItemArtworkInfo{} + } + infos, err := NewArtworkRepository(ctx, db).GetInfoForItems(kind, ids) + if err != nil { + log.Error(ctx, "Failed to hydrate artwork info onto page", "kind", kind, err) + return map[string]model.ItemArtworkInfo{} + } + return infos +} + +// applyItemImage copies a hydration entry onto img; a missing entry leaves it zero (unresolved). +func applyItemImage(infos map[string]model.ItemArtworkInfo, id string, img *model.ItemImage) { + if info, ok := infos[id]; ok { + img.ImageHash = info.Hash + img.ImageAbsent = info.Absent() + } +} diff --git a/persistence/artwork_hydration_test.go b/persistence/artwork_hydration_test.go new file mode 100644 index 000000000..6001221cb --- /dev/null +++ b/persistence/artwork_hydration_test.go @@ -0,0 +1,257 @@ +package persistence + +import ( + "context" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/model/request" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" + "github.com/pocketbase/dbx" +) + +var _ = Describe("Artwork hydration", func() { + var ctx context.Context + var aw model.ArtworkRepository + + putInfo := func(kind, id, hash string) { + Expect(aw.PutItemArtwork(&model.ItemArtwork{ + ItemKind: kind, ItemID: id, ImageType: model.ImageTypePrimary, Hash: hash, + })).To(Succeed()) + } + + BeforeEach(func() { + clearArtworkTables() + DeferCleanup(clearArtworkTables) + ctx = request.WithUser(log.NewContext(context.Background()), adminUser) + aw = NewArtworkRepository(ctx, GetDBXBuilder()) + }) + + Describe("albums", func() { + var repo model.AlbumRepository + BeforeEach(func() { repo = NewAlbumRepository(ctx, GetDBXBuilder()) }) + + It("hydrates the found / known-absent / unresolved states", func() { + putInfo("al", albumSgtPeppers.ID, "althash11111111") + putInfo("al", albumAbbeyRoad.ID, "") + // albumRadioactivity: no row -> unresolved + + byID := map[string]model.Album{} + all, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + for _, a := range all { + byID[a.ID] = a + } + + Expect(byID[albumSgtPeppers.ID].ImageHash).To(Equal("althash11111111")) + Expect(byID[albumSgtPeppers.ID].ImageAbsent).To(BeFalse()) + Expect(byID[albumAbbeyRoad.ID].ImageHash).To(BeEmpty()) + Expect(byID[albumAbbeyRoad.ID].ImageAbsent).To(BeTrue()) + Expect(byID[albumRadioactivity.ID].ImageHash).To(BeEmpty()) + Expect(byID[albumRadioactivity.ID].ImageAbsent).To(BeFalse()) + }) + + It("hydrates Get", func() { + putInfo("al", albumSgtPeppers.ID, "gethash22222222") + got, err := repo.Get(albumSgtPeppers.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.ImageHash).To(Equal("gethash22222222")) + }) + + It("hydrates Search", func() { + putInfo("al", albumSgtPeppers.ID, "srchash33333333") + res, err := repo.Search("Peppers", model.QueryOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeEmpty()) + Expect(res[0].ImageHash).To(Equal("srchash33333333")) + }) + + It("does not persist ImageHash/ImageAbsent on Put", func() { + al := albumSgtPeppers + al.ImageHash = "shouldnotpersist" + al.ImageAbsent = true + Expect(repo.(*albumRepository).Put(&al)).To(Succeed()) + + // No item_artwork rows exist, so a fresh read must observe zero values. + got, err := repo.Get(al.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.ImageHash).To(BeEmpty()) + Expect(got.ImageAbsent).To(BeFalse()) + }) + }) + + Describe("artists", func() { + var repo model.ArtistRepository + BeforeEach(func() { repo = NewArtistRepository(ctx, GetDBXBuilder()) }) + + It("hydrates the found / known-absent / unresolved states", func() { + putInfo("ar", artistBeatles.ID, "arhash444444444") + putInfo("ar", artistKraftwerk.ID, "") + // artistCJK: no row -> unresolved + + byID := map[string]model.Artist{} + all, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + for _, a := range all { + byID[a.ID] = a + } + + Expect(byID[artistBeatles.ID].ImageHash).To(Equal("arhash444444444")) + Expect(byID[artistBeatles.ID].ImageAbsent).To(BeFalse()) + Expect(byID[artistKraftwerk.ID].ImageHash).To(BeEmpty()) + Expect(byID[artistKraftwerk.ID].ImageAbsent).To(BeTrue()) + Expect(byID[artistCJK.ID].ImageHash).To(BeEmpty()) + Expect(byID[artistCJK.ID].ImageAbsent).To(BeFalse()) + }) + + It("hydrates Get", func() { + putInfo("ar", artistBeatles.ID, "arget5555555555") + got, err := repo.Get(artistBeatles.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.ImageHash).To(Equal("arget5555555555")) + }) + + It("hydrates Search", func() { + putInfo("ar", artistBeatles.ID, "arsrch666666666") + res, err := repo.Search("Beatles", model.QueryOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeEmpty()) + Expect(res[0].ImageHash).To(Equal("arsrch666666666")) + }) + }) + + Describe("playlists", func() { + var repo model.PlaylistRepository + BeforeEach(func() { repo = NewPlaylistRepository(ctx, GetDBXBuilder()) }) + + It("hydrates the found / known-absent states", func() { + putInfo("pl", plsBest.ID, "plhash777777777") + putInfo("pl", plsCool.ID, "") + + byID := map[string]model.Playlist{} + all, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + for _, p := range all { + byID[p.ID] = p + } + + Expect(byID[plsBest.ID].ImageHash).To(Equal("plhash777777777")) + Expect(byID[plsBest.ID].ImageAbsent).To(BeFalse()) + Expect(byID[plsCool.ID].ImageHash).To(BeEmpty()) + Expect(byID[plsCool.ID].ImageAbsent).To(BeTrue()) + }) + + It("hydrates Get", func() { + putInfo("pl", plsBest.ID, "plget8888888888") + got, err := repo.Get(plsBest.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.ImageHash).To(Equal("plget8888888888")) + }) + }) + + Describe("radios", func() { + var repo model.RadioRepository + BeforeEach(func() { repo = NewRadioRepository(ctx, GetDBXBuilder()) }) + + It("hydrates the found / known-absent states", func() { + putInfo("ra", radioWithHomePage.ID, "rahash999999999") + putInfo("ra", radioWithoutHomePage.ID, "") + + byID := map[string]model.Radio{} + all, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + for _, rd := range all { + byID[rd.ID] = rd + } + + Expect(byID[radioWithHomePage.ID].ImageHash).To(Equal("rahash999999999")) + Expect(byID[radioWithHomePage.ID].ImageAbsent).To(BeFalse()) + Expect(byID[radioWithoutHomePage.ID].ImageHash).To(BeEmpty()) + Expect(byID[radioWithoutHomePage.ID].ImageAbsent).To(BeTrue()) + }) + + It("hydrates Get", func() { + putInfo("ra", radioWithHomePage.ID, "ragetaaaaaaaaaa") + got, err := repo.Get(radioWithHomePage.ID) + Expect(err).ToNot(HaveOccurred()) + Expect(got.ImageHash).To(Equal("ragetaaaaaaaaaa")) + }) + }) + + Describe("mediafiles", func() { + var repo model.MediaFileRepository + + setCover := func(id string, v bool) { + _, err := GetDBXBuilder().NewQuery("UPDATE media_file SET has_cover_art={:v} WHERE id={:id}"). + Bind(dbx.Params{"v": v, "id": id}).Execute() + Expect(err).ToNot(HaveOccurred()) + } + + getByID := func() map[string]model.MediaFile { + byID := map[string]model.MediaFile{} + all, err := repo.GetAll() + Expect(err).ToNot(HaveOccurred()) + for _, mf := range all { + byID[mf.ID] = mf + } + return byID + } + + BeforeEach(func() { + repo = NewMediaFileRepository(ctx, GetDBXBuilder()) + DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableMediaFileCoverArt = true + }) + + It("resolves the embedded-eligible fallback matrix", func() { + setCover("1001", true) // eligible, own hash + setCover("1002", true) // eligible, but embedded art absent -> album + DeferCleanup(func() { setCover("1001", false); setCover("1002", false) }) + + putInfo("al", "101", "alh101xxxxxxxxxx") // song 1001's album (found) + putInfo("al", "102", "alh102xxxxxxxxxx") // song 1002's album (found) + putInfo("al", "103", "") // songs 1003/1004 album known-absent + putInfo("mf", "1001", "mfh1001xxxxxxxx") + putInfo("mf", "1002", "") // embedded resolved absent + + byID := getByID() + + // eligible + own hash -> own hash + Expect(byID["1001"].ImageHash).To(Equal("mfh1001xxxxxxxx")) + Expect(byID["1001"].ImageAbsent).To(BeFalse()) + // eligible + embedded absent -> falls through to album 102 info + Expect(byID["1002"].ImageHash).To(Equal("alh102xxxxxxxxxx")) + Expect(byID["1002"].ImageAbsent).To(BeFalse()) + // not eligible (no embedded cover) -> album 103 info (known-absent) + Expect(byID["1003"].ImageHash).To(BeEmpty()) + Expect(byID["1003"].ImageAbsent).To(BeTrue()) + // not eligible, album has no row -> zero values (unresolved) + Expect(byID["2002"].ImageHash).To(BeEmpty()) + Expect(byID["2002"].ImageAbsent).To(BeFalse()) + }) + + It("uses album info for an eligible file when EnableMediaFileCoverArt is off", func() { + conf.Server.EnableMediaFileCoverArt = false + setCover("1001", true) + DeferCleanup(func() { setCover("1001", false) }) + + putInfo("al", "101", "alh101offxxxxxxx") + putInfo("mf", "1001", "mfh1001offxxxxx") + + byID := getByID() + Expect(byID["1001"].ImageHash).To(Equal("alh101offxxxxxxx")) + Expect(byID["1001"].ImageAbsent).To(BeFalse()) + }) + + It("hydrates Search", func() { + putInfo("al", "101", "alsrchhhhhhhhhhh") + res, err := repo.Search("A Day In A Life", model.QueryOptions{}) + Expect(err).ToNot(HaveOccurred()) + Expect(res).ToNot(BeEmpty()) + Expect(res[0].ImageHash).To(Equal("alsrchhhhhhhhhhh")) + }) + }) +}) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 0a6ddefdc..7ccef59ef 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -19,6 +19,7 @@ var _ = Describe("ArtworkQueueRepository", func() { BeforeEach(func() { clearArtworkTables() + DeferCleanup(clearArtworkTables) repo = NewArtworkQueueRepository(context.Background(), GetDBXBuilder()) }) diff --git a/persistence/artwork_repository_test.go b/persistence/artwork_repository_test.go index ae6752def..ae9f15fda 100644 --- a/persistence/artwork_repository_test.go +++ b/persistence/artwork_repository_test.go @@ -25,6 +25,7 @@ var _ = Describe("ArtworkRepository", func() { BeforeEach(func() { clearArtworkTables() + DeferCleanup(clearArtworkTables) repo = NewArtworkRepository(context.Background(), GetDBXBuilder()) }) diff --git a/persistence/mediafile_repository.go b/persistence/mediafile_repository.go index ace61610c..9ba9d3825 100644 --- a/persistence/mediafile_repository.go +++ b/persistence/mediafile_repository.go @@ -218,7 +218,37 @@ func (r *mediaFileRepository) GetAll(options ...model.QueryOptions) (model.Media if err != nil { return nil, err } - return res.toModels(), nil + mfs := res.toModels() + r.hydrateArtwork(mfs) + return mfs, nil +} + +// hydrateArtwork mirrors MediaFile.CoverArtID: an embedded-eligible file with resolved own art uses +// it, else it falls back to the album's. Two batched item_artwork lookups per page, never a join. +func (r *mediaFileRepository) hydrateArtwork(mfs model.MediaFiles) { + if len(mfs) == 0 { + return + } + albumIDs := make([]string, len(mfs)) + var eligibleIDs []string + for i := range mfs { + albumIDs[i] = mfs[i].AlbumID + if mfs[i].HasCoverArt && conf.Server.EnableMediaFileCoverArt { + eligibleIDs = append(eligibleIDs, mfs[i].ID) + } + } + albumInfos := hydrateItemImages(r.ctx, r.db, model.KindAlbumArtwork.Prefix(), albumIDs) + mfInfos := hydrateItemImages(r.ctx, r.db, model.KindMediaFileArtwork.Prefix(), eligibleIDs) + for i := range mfs { + if mfs[i].HasCoverArt && conf.Server.EnableMediaFileCoverArt { + if info, ok := mfInfos[mfs[i].ID]; ok && !info.Absent() { + mfs[i].ImageHash = info.Hash + mfs[i].ImageAbsent = false + continue + } + } + applyItemImage(albumInfos, mfs[i].AlbumID, &mfs[i].ItemImage) + } } // GetRandom uses two passes so the random sort runs over a narrow rowid index instead of the @@ -252,7 +282,9 @@ func (r *mediaFileRepository) GetRandom(options ...model.QueryOptions) (model.Me if err := r.queryAll(sq, &res); err != nil { return nil, err } - return res.toModels(), nil + mfs := res.toModels() + r.hydrateArtwork(mfs) + return mfs, nil } func (r *mediaFileRepository) GetAllByTags(tag model.TagName, values []string, options ...model.QueryOptions) (model.MediaFiles, error) { @@ -487,7 +519,9 @@ func (r *mediaFileRepository) Search(q string, options ...model.QueryOptions) (m if err != nil { return nil, fmt.Errorf("searching media_file %q: %w", q, err) } - return res.toModels(), nil + mfs := res.toModels() + r.hydrateArtwork(mfs) + return mfs, nil } func (r *mediaFileRepository) Count(options ...rest.QueryOptions) (int64, error) { diff --git a/persistence/playlist_repository.go b/persistence/playlist_repository.go index 0abf53d93..e984b8852 100644 --- a/persistence/playlist_repository.go +++ b/persistence/playlist_repository.go @@ -14,6 +14,7 @@ import ( "github.com/deluan/rest" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" "github.com/pocketbase/dbx" ) @@ -172,7 +173,21 @@ func (r *playlistRepository) findBy(sql Sqlizer) (*model.Playlist, error) { return nil, model.ErrNotFound } - return &pls[0].Playlist, nil + list := model.Playlists{pls[0].Playlist} + r.hydrateArtwork(list) + return &list[0], nil +} + +// hydrateArtwork fills each playlist's ImageHash/ImageAbsent from one batched item_artwork lookup. +func (r *playlistRepository) hydrateArtwork(playlists model.Playlists) { + if len(playlists) == 0 { + return + } + ids := slice.Map(playlists, func(p model.Playlist) string { return p.ID }) + infos := hydrateItemImages(r.ctx, r.db, model.KindPlaylistArtwork.Prefix(), ids) + for i := range playlists { + applyItemImage(infos, playlists[i].ID, &playlists[i].ItemImage) + } } func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playlists, error) { @@ -186,6 +201,7 @@ func (r *playlistRepository) GetAll(options ...model.QueryOptions) (model.Playli for i, p := range res { playlists[i] = p.Playlist } + r.hydrateArtwork(playlists) return playlists, err } @@ -230,6 +246,7 @@ func (r *playlistRepository) GetPlaylists(mediaFileId string) (model.Playlists, for i, p := range res { playlists[i] = p.Playlist } + r.hydrateArtwork(playlists) return playlists, nil } diff --git a/persistence/radio_repository.go b/persistence/radio_repository.go index 3ef900920..d48178977 100644 --- a/persistence/radio_repository.go +++ b/persistence/radio_repository.go @@ -10,6 +10,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/model/id" + "github.com/navidrome/navidrome/utils/slice" "github.com/pocketbase/dbx" ) @@ -49,14 +50,35 @@ func (r *radioRepository) Get(id string) (*model.Radio, error) { sel := r.newSelect().Where(Eq{"id": id}).Columns("*") res := model.Radio{} err := r.queryOne(sel, &res) - return &res, err + if err != nil { + return &res, err + } + list := model.Radios{res} + r.hydrateArtwork(list) + return &list[0], nil } func (r *radioRepository) GetAll(options ...model.QueryOptions) (model.Radios, error) { sel := r.newSelect(options...).Columns("*") res := model.Radios{} err := r.queryAll(sel, &res) - return res, err + if err != nil { + return res, err + } + r.hydrateArtwork(res) + return res, nil +} + +// hydrateArtwork fills each radio's ImageHash/ImageAbsent from one batched item_artwork lookup. +func (r *radioRepository) hydrateArtwork(radios model.Radios) { + if len(radios) == 0 { + return + } + ids := slice.Map(radios, func(rd model.Radio) string { return rd.ID }) + infos := hydrateItemImages(r.ctx, r.db, model.KindRadioArtwork.Prefix(), ids) + for i := range radios { + applyItemImage(infos, radios[i].ID, &radios[i].ItemImage) + } } // GetAllIDs returns just the radio IDs. Used by bulk enumeration (artwork backfill).