diff --git a/cmd/pls.go b/cmd/pls.go index 95cbe4eec..0203fe3a9 100644 --- a/cmd/pls.go +++ b/cmd/pls.go @@ -260,7 +260,7 @@ func runImport(ctx context.Context, files []string) { ctx = request.WithUser(ctx, *user) } - pls := playlists.NewPlaylists(ds, core.NewImageUploadService()) + pls := playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) for _, file := range files { absPath, err := filepath.Abs(file) diff --git a/cmd/scan.go b/cmd/scan.go index 320b401d4..c75a7aed4 100644 --- a/cmd/scan.go +++ b/cmd/scan.go @@ -82,7 +82,7 @@ func runScanner(ctx context.Context) { sqlDB := db.Db() defer db.Db().Close() ds := persistence.New(sqlDB) - pls := playlists.NewPlaylists(ds, core.NewImageUploadService()) + pls := playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) // Parse targets from command line or file var scanTargets []model.ScanTarget diff --git a/cmd/wire_gen.go b/cmd/wire_gen.go index cbd903e3c..82d0ecc89 100644 --- a/cmd/wire_gen.go +++ b/cmd/wire_gen.go @@ -65,7 +65,7 @@ func CreateNativeAPIRouter(ctx context.Context) *nativeapi.Router { sqlDB := db.Db() dataStore := persistence.New(sqlDB) share := core.NewShare(dataStore) - imageUploadService := core.NewImageUploadService() + imageUploadService := core.NewImageUploadService(dataStore) playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) insights := metrics.GetInstance(dataStore) broker := events.GetBroker() @@ -98,7 +98,7 @@ func CreateSubsonicAPIRouter(ctx context.Context) *subsonic.Router { agentsAgents := agents.GetAgents(dataStore, manager) matcherMatcher := matcher.New(dataStore) provider := external.NewProvider(dataStore, agentsAgents, matcherMatcher) - imageUploadService := core.NewImageUploadService() + imageUploadService := core.NewImageUploadService(dataStore) playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) modelScanner := scanner.New(ctx, dataStore, broker, playlistsPlaylists, metricsMetrics) playTracker := scrobbler.GetPlayTracker(dataStore, broker, manager) @@ -125,7 +125,7 @@ func CreateJellyfinAPIRouter(ctx context.Context) *jellyfin.Router { metricsMetrics := metrics.GetPrometheusInstance(dataStore) manager := plugins.GetManager(dataStore, broker, metricsMetrics) playTracker := scrobbler.GetPlayTracker(dataStore, broker, manager) - imageUploadService := core.NewImageUploadService() + imageUploadService := core.NewImageUploadService(dataStore) playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) agentsAgents := agents.GetAgents(dataStore, manager) matcherMatcher := matcher.New(dataStore) @@ -183,7 +183,7 @@ func CreateScanner(ctx context.Context) model.Scanner { sqlDB := db.Db() dataStore := persistence.New(sqlDB) broker := events.GetBroker() - imageUploadService := core.NewImageUploadService() + imageUploadService := core.NewImageUploadService(dataStore) playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) metricsMetrics := metrics.GetPrometheusInstance(dataStore) modelScanner := scanner.New(ctx, dataStore, broker, playlistsPlaylists, metricsMetrics) @@ -194,7 +194,7 @@ func CreateScanWatcher(ctx context.Context) scanner.Watcher { sqlDB := db.Db() dataStore := persistence.New(sqlDB) broker := events.GetBroker() - imageUploadService := core.NewImageUploadService() + imageUploadService := core.NewImageUploadService(dataStore) playlistsPlaylists := playlists.NewPlaylists(dataStore, imageUploadService) metricsMetrics := metrics.GetPrometheusInstance(dataStore) modelScanner := scanner.New(ctx, dataStore, broker, playlistsPlaylists, metricsMetrics) @@ -218,7 +218,8 @@ func CreateArtworkWorker() *artwork.Worker { manager := plugins.GetManager(dataStore, broker, metricsMetrics) agentsAgents := agents.GetAgents(dataStore, manager) fFmpeg := ffmpeg.New() - worker := artwork.NewWorker(dataStore, imageStore, agentsAgents, fFmpeg, broker) + fileCache := artwork.GetImageCache() + worker := artwork.NewWorker(dataStore, imageStore, agentsAgents, fFmpeg, broker, fileCache) return worker } diff --git a/core/artwork/processor.go b/core/artwork/processor.go index 28231b65e..4272d41b8 100644 --- a/core/artwork/processor.go +++ b/core/artwork/processor.go @@ -15,6 +15,7 @@ import ( "github.com/navidrome/navidrome/core/ffmpeg" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/cache" xdraw "golang.org/x/image/draw" ) @@ -49,6 +50,7 @@ type workerDeps struct { store *ImageStore agents *agents.Agents ffmpeg ffmpeg.FFmpeg + cache cache.FileCache gate gateFunc } diff --git a/core/artwork/serving.go b/core/artwork/serving.go index a2f9c57ef..38a474355 100644 --- a/core/artwork/serving.go +++ b/core/artwork/serving.go @@ -106,21 +106,14 @@ func (s *service) serveHash(ctx context.Context, artID model.ArtworkID, ia *mode } if size == 0 && !square { - rc, err := s.openOriginal(ia, art.Mime) + rc, err := openOriginal(ia, art.Mime, s.store) if err != nil { return s.dangling(ctx, artID) } return &Image{ReadCloser: rc, Hash: ia.Hash, LastUpdated: ia.UpdatedAt}, nil } - item := &resizedItem{ - hash: ia.Hash, - size: size, - square: square, - lastUpdate: ia.UpdatedAt, - ffmpeg: s.ffmpeg, - open: func() (io.ReadCloser, error) { return s.openOriginal(ia, art.Mime) }, - } + item := newResizedItem(ia, art.Mime, size, square, s.store, s.ffmpeg) stream, err := s.cache.Get(ctx, item) if err != nil { if errors.Is(err, context.Canceled) { @@ -133,7 +126,7 @@ func (s *service) serveHash(ctx context.Context, artID model.ArtworkID, ia *mode // openOriginal opens the full-resolution bytes for a found state row, enforcing the // mtime invariant: bytes are never served under a hash they no longer match. -func (s *service) openOriginal(ia *model.ItemArtwork, mime string) (io.ReadCloser, error) { +func openOriginal(ia *model.ItemArtwork, mime string, store *ImageStore) (io.ReadCloser, error) { if isFileBacked(ia.Source) { f, err := os.Open(ia.SourcePath) if err != nil { @@ -161,7 +154,20 @@ func (s *service) openOriginal(ia *model.ItemArtwork, mime string) (io.ReadClose return nil, errStaleSource } } - return s.store.Open(ia.Hash, mime) + return store.Open(ia.Hash, mime) +} + +// newResizedItem builds the resize-cache reader for a found state row's bytes; shared by +// the serving path and the worker's precache so both key the cache identically. +func newResizedItem(ia *model.ItemArtwork, mime string, size int, square bool, store *ImageStore, ffm ffmpeg.FFmpeg) *resizedItem { + return &resizedItem{ + hash: ia.Hash, + size: size, + square: square, + lastUpdate: ia.UpdatedAt, + ffmpeg: ffm, + open: func() (io.ReadCloser, error) { return openOriginal(ia, mime, store) }, + } } // provisional does a local-only read-through for an entity with no state row: it enqueues @@ -332,7 +338,7 @@ func unixMtime(mtime int64) time.Time { } // resizedItem is an artworkReader that resizes bytes opened by open() and caches the -// result under a hash-derived key, sharing the image cache with the legacy readers. +// result under a hash-derived key. type resizedItem struct { hash string size int diff --git a/core/artwork/worker.go b/core/artwork/worker.go index a71495441..01ff548de 100644 --- a/core/artwork/worker.go +++ b/core/artwork/worker.go @@ -16,6 +16,7 @@ import ( "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/events" + "github.com/navidrome/navidrome/utils/cache" "golang.org/x/time/rate" ) @@ -53,9 +54,9 @@ type Worker struct { inFlight map[string]struct{} } -func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker) *Worker { +func NewWorker(ds model.DataStore, store *ImageStore, ag *agents.Agents, ffmpeg ffmpeg.FFmpeg, broker events.Broker, imgCache cache.FileCache) *Worker { w := &Worker{ - deps: workerDeps{ds: ds, store: store, agents: ag, ffmpeg: ffmpeg}, + deps: workerDeps{ds: ds, store: store, agents: ag, ffmpeg: ffmpeg, cache: imgCache}, broker: broker, wake: make(chan struct{}, 1), runCtx: context.Background(), @@ -148,6 +149,9 @@ func (w *Worker) drain(ctx context.Context, concurrency int) (int, error) { foundMu.Lock() found = append(found, it) foundMu.Unlock() + // Post-outcome only: the queue row was already settled by process, so warming + // the resize cache here can never block or alter queue-op handling. + w.precache(ctx, it) } }(item) } @@ -215,6 +219,34 @@ func (w *Worker) process(ctx context.Context, item model.ArtworkQueueItem) outco return out } +// precache warms the resize cache for a newly-acquired image at the UI cover size, so the +// first UI request is a cache hit. Skipped when disabled; failures are debug-only. +func (w *Worker) precache(ctx context.Context, item model.ArtworkQueueItem) { + if !conf.Server.EnableArtworkPrecache || w.deps.cache == nil || w.deps.cache.Disabled(ctx) { + return + } + imageType := item.ImageType + if imageType == "" { + imageType = model.ImageTypePrimary + } + repo := w.deps.ds.Artwork(ctx) + ia, err := repo.GetItemArtwork(item.ItemKind, item.ItemID, imageType) + if err != nil || ia.Hash == "" { + return + } + art, err := repo.GetImage(ia.Hash) + if err != nil { + return + } + stream, err := w.deps.cache.Get(ctx, newResizedItem(ia, art.Mime, conf.Server.UICoverArtSize, false, w.deps.store, w.deps.ffmpeg)) + if err != nil { + log.Debug(ctx, "artwork: precache failed", "kind", item.ItemKind, "id", item.ItemID, err) + return + } + _, _ = io.Copy(io.Discard, stream) + _ = stream.Close() +} + // claim reserves items not already in flight, so a row appearing twice within a single // batch is processed once. func (w *Worker) claim(batch []model.ArtworkQueueItem) []model.ArtworkQueueItem { diff --git a/core/artwork/worker_test.go b/core/artwork/worker_test.go index 97bbc41c8..3917036c7 100644 --- a/core/artwork/worker_test.go +++ b/core/artwork/worker_test.go @@ -15,11 +15,38 @@ import ( "github.com/navidrome/navidrome/model" "github.com/navidrome/navidrome/server/events" "github.com/navidrome/navidrome/tests" + "github.com/navidrome/navidrome/utils/cache" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" "go.uber.org/goleak" ) +// recordingCache captures the keys passed to Get so precache warming can be asserted, +// and can be forced Disabled to exercise the skip path. +type recordingCache struct { + cache.FileCache + mu sync.Mutex + keys []string + disabled bool +} + +func (c *recordingCache) Disabled(ctx context.Context) bool { + return c.disabled || c.FileCache.Disabled(ctx) +} + +func (c *recordingCache) Get(ctx context.Context, arg cache.Item) (*cache.CachedStream, error) { + c.mu.Lock() + c.keys = append(c.keys, arg.Key()) + c.mu.Unlock() + return c.FileCache.Get(ctx, arg) +} + +func (c *recordingCache) getKeys() []string { + c.mu.Lock() + defer c.mu.Unlock() + return append([]string(nil), c.keys...) +} + // reenqueueOnDequeue simulates a concurrent scan Enqueue between DequeueBatch and the // worker's delete by bumping retry_at, so a DeleteIfUnchanged on the dequeued value no-ops. type reenqueueOnDequeue struct { @@ -88,6 +115,7 @@ var _ = Describe("Worker", func() { artRepo *tests.MockArtworkRepo queueRepo *tests.MockArtworkQueueRepo broker *fakeEventBroker + imgCache *recordingCache repoRoot string w *Worker ) @@ -98,6 +126,7 @@ var _ = Describe("Worker", func() { var err error repoRoot, err = os.Getwd() Expect(err).ToNot(HaveOccurred()) + conf.Server.CacheFolder = conf.NewDir(GinkgoT().TempDir()) folderRepo = &fakeFolderRepo{} libRepo = &tests.MockLibraryRepo{} @@ -117,7 +146,13 @@ var _ = Describe("Worker", func() { conf.Server.CoverArtPriority = "cover.jpg, embedded" conf.Server.ArtworkExternalMaxRPS = 1000 // keep the limiter out of the way of behavior tests broker = &fakeEventBroker{} - w = NewWorker(ds, store, ag, ffm, broker) + imgCache = &recordingCache{FileCache: cache.NewFileCache("WorkerTest", "100MB", "images", 0, + func(ctx context.Context, arg cache.Item) (io.Reader, error) { + r, _, err := arg.(artworkReader).Reader(ctx) + return r, err + })} + Eventually(func() bool { return imgCache.Available(ctx) }).Should(BeTrue()) + w = NewWorker(ds, store, ag, ffm, broker, imgCache) }) Describe("drain", func() { @@ -238,7 +273,7 @@ var _ = Describe("Worker", func() { }) racing := &reenqueueOnDequeue{MockArtworkQueueRepo: queueRepo} ds.MockedArtworkQueue = racing - w = NewWorker(ds, store, ag, ffm, broker) + w = NewWorker(ds, store, ag, ffm, broker, imgCache) Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ ItemKind: "al", ItemID: "al7", Priority: model.ArtworkPriorityScan, })).To(Succeed()) @@ -260,7 +295,7 @@ var _ = Describe("Worker", func() { imageAgents(&fakeImageAgent{name: "failAgent", err: errors.New("agent timed out")}) racing := &reenqueueOnDequeue{MockArtworkQueueRepo: queueRepo} ds.MockedArtworkQueue = racing - w = NewWorker(ds, store, ag, ffm, broker) + w = NewWorker(ds, store, ag, ffm, broker, imgCache) Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "al", ItemID: "al8"})).To(Succeed()) dequeued := findQueued(queueRepo, "al", "al8").RetryAt @@ -283,7 +318,7 @@ var _ = Describe("Worker", func() { private: model.Playlist{ID: "plPriv", OwnerID: "admin"}, tracks: &tests.MockPlaylistTrackRepo{}, } - w = NewWorker(vds, store, ag, ffm, broker) + w = NewWorker(vds, store, ag, ffm, broker, imgCache) Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ItemKind: "pl", ItemID: "plPriv"})).To(Succeed()) n, err := w.drain(ctx, 1) @@ -433,6 +468,42 @@ var _ = Describe("Worker", func() { }) }) + Describe("precache", func() { + BeforeEach(func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"cover.jpg"}, + }} + ds.MockedAlbum.(*tests.MockAlbumRepo).SetData(model.Albums{ + {ID: "alpc", Name: "Album", FolderIDs: []string{"f1"}}, + }) + conf.Server.UICoverArtSize = 300 + Expect(queueRepo.Enqueue(model.ArtworkQueueItem{ + ItemKind: "al", ItemID: "alpc", Priority: model.ArtworkPriorityScan, + })).To(Succeed()) + }) + + It("warms the resize cache at the UI cover size after a found acquisition", func() { + conf.Server.EnableArtworkPrecache = true + + n, err := w.drain(ctx, 1) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(1)) + + Expect(imgCache.getKeys()).To(ContainElement(ContainSubstring(".300.false."))) + }) + + It("skips warming when precache is disabled", func() { + conf.Server.EnableArtworkPrecache = false + + n, err := w.drain(ctx, 1) + Expect(err).ToNot(HaveOccurred()) + Expect(n).To(Equal(1)) + + Expect(imgCache.getKeys()).To(BeEmpty()) + }) + }) + Describe("RunPrune", func() { It("runs a prune under the worker mutex", func() { Expect(w.RunPrune(ctx)).To(Succeed()) @@ -456,7 +527,7 @@ var _ = Describe("Worker", func() { DeferCleanup(func() { goleak.VerifyNone(GinkgoT(), ignore) }) localDS := &tests.MockDataStore{MockedArtworkQueue: tests.CreateMockArtworkQueueRepo()} - lw := NewWorker(localDS, NewImageStore(GinkgoT().TempDir()), agents.GetAgents(localDS, nil), tests.NewMockFFmpeg(""), &fakeEventBroker{}) + lw := NewWorker(localDS, NewImageStore(GinkgoT().TempDir()), agents.GetAgents(localDS, nil), tests.NewMockFFmpeg(""), &fakeEventBroker{}, imgCache) runCtx, cancel := context.WithCancel(ctx) done := make(chan error, 1) diff --git a/core/artwork/worker_timing_test.go b/core/artwork/worker_timing_test.go index 159a53081..dbaed0a84 100644 --- a/core/artwork/worker_timing_test.go +++ b/core/artwork/worker_timing_test.go @@ -47,7 +47,7 @@ func TestArtworkGatePerAgentBreakerIsolation(t *testing.T) { synctest.Test(t, func(t *testing.T) { g := NewWithT(t) w := NewWorker(&tests.MockDataStore{}, NewImageStore(t.TempDir()), - agents.GetAgents(&tests.MockDataStore{}, nil), tests.NewMockFFmpeg(""), &fakeEventBroker{}) + agents.GetAgents(&tests.MockDataStore{}, nil), tests.NewMockFFmpeg(""), &fakeEventBroker{}, nil) fail := func() (io.ReadCloser, string, error) { return nil, "", errors.New("boom") } for range breakerThreshold { diff --git a/core/image_upload.go b/core/image_upload.go index eb61b225a..6f5ef418a 100644 --- a/core/image_upload.go +++ b/core/image_upload.go @@ -30,10 +30,20 @@ func MaxImageUploadSize() int64 { return int64(size) } -type imageUploadService struct{} +// uploadEntityKind maps an upload's entity type to its artwork kind prefix, so a +// successful upload can clear and re-queue that item's artwork state. +var uploadEntityKind = map[string]string{ + consts.EntityArtist: model.KindArtistArtwork.Prefix(), + consts.EntityPlaylist: model.KindPlaylistArtwork.Prefix(), + consts.EntityRadio: model.KindRadioArtwork.Prefix(), +} -func NewImageUploadService() ImageUploadService { - return &imageUploadService{} +type imageUploadService struct { + ds model.DataStore +} + +func NewImageUploadService(ds model.DataStore) ImageUploadService { + return &imageUploadService{ds: ds} } func (s *imageUploadService) SetImage(ctx context.Context, entityType string, entityID string, name string, oldPath string, reader io.Reader, ext string) (string, error) { @@ -62,9 +72,27 @@ func (s *imageUploadService) SetImage(ctx context.Context, entityType string, en return "", fmt.Errorf("writing image file: %w", err) } + s.bumpArtwork(ctx, entityType, entityID) return filename, nil } +// bumpArtwork clears the item's resolved state and re-queues it at Bump priority: the +// upload is now the top-priority source, so the worker re-resolves and the UI swaps. +func (s *imageUploadService) bumpArtwork(ctx context.Context, entityType, id string) { + kind, ok := uploadEntityKind[entityType] + if !ok { + return + } + if err := s.ds.Artwork(ctx).DeleteForItem(kind, id); err != nil { + log.Warn(ctx, "Could not clear artwork state after upload", "kind", kind, "id", id, err) + } + item := model.ArtworkQueueItem{ItemKind: kind, ItemID: id, ImageType: model.ImageTypePrimary, + Priority: model.ArtworkPriorityBump} + if err := s.ds.ArtworkQueue(ctx).Enqueue(item); err != nil { + log.Warn(ctx, "Could not enqueue artwork after upload", "kind", kind, "id", id, err) + } +} + func (s *imageUploadService) RemoveImage(ctx context.Context, path string) error { if path == "" { return nil diff --git a/core/image_upload_test.go b/core/image_upload_test.go index e7648df34..1b63b3052 100644 --- a/core/image_upload_test.go +++ b/core/image_upload_test.go @@ -10,6 +10,8 @@ import ( "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/consts" "github.com/navidrome/navidrome/core" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/tests" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) @@ -17,12 +19,17 @@ import ( var _ = Describe("ImageUploadService", func() { var svc core.ImageUploadService var tmpDir string + var artRepo *tests.MockArtworkRepo + var queueRepo *tests.MockArtworkQueueRepo BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) tmpDir = GinkgoT().TempDir() conf.Server.DataFolder = conf.NewDir(tmpDir) - svc = core.NewImageUploadService() + artRepo = tests.CreateMockArtworkRepo() + queueRepo = tests.CreateMockArtworkQueueRepo() + ds := &tests.MockDataStore{MockedArtwork: artRepo, MockedArtworkQueue: queueRepo} + svc = core.NewImageUploadService(ds) }) Describe("SetImage", func() { @@ -69,6 +76,27 @@ var _ = Describe("ImageUploadService", func() { _, err := svc.SetImage(ctx, consts.EntityArtist, "ar-1", "Name", "/nonexistent/path.jpg", reader, ".jpg") Expect(err).ToNot(HaveOccurred()) }) + + It("clears artwork state and enqueues a Bump on success", func() { + ctx := context.Background() + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: "ar", ItemID: "ar-1", Hash: "oldhash", Source: "external", + })).To(Succeed()) + + _, err := svc.SetImage(ctx, consts.EntityArtist, "ar-1", "Pink Floyd", "", strings.NewReader("img"), ".jpg") + Expect(err).ToNot(HaveOccurred()) + + _, err = artRepo.GetItemArtwork("ar", "ar-1", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + + queued, err := queueRepo.DequeueBatch(1000) + Expect(err).ToNot(HaveOccurred()) + Expect(queued).To(ContainElement(SatisfyAll( + HaveField("ItemKind", "ar"), + HaveField("ItemID", "ar-1"), + HaveField("Priority", model.ArtworkPriorityBump), + ))) + }) }) Describe("RemoveImage", func() { diff --git a/core/playlists/import_test.go b/core/playlists/import_test.go index f2866fb60..730af20cd 100644 --- a/core/playlists/import_test.go +++ b/core/playlists/import_test.go @@ -43,7 +43,7 @@ var _ = Describe("Playlists - Import", func() { var folder *model.Folder BeforeEach(func() { DeferCleanup(configtest.SetupConfig()) - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) ds.MockedMediaFile = &mockedMediaFileRepo{} libPath, _ := os.Getwd() // Set up library with the actual library path that matches the folder @@ -118,7 +118,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3", "test.ogg"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -136,7 +136,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -155,7 +155,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -174,7 +174,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -192,7 +192,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -209,7 +209,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -226,7 +226,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -244,7 +244,7 @@ var _ = Describe("Playlists - Import", func() { mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFolder := &model.Folder{ID: "1", LibraryID: 1, LibraryPath: tmpDir, Path: "", Name: ""} pls, err := ps.ImportFromFolder(ctx, plsFolder, "test.m3u") @@ -258,7 +258,7 @@ var _ = Describe("Playlists - Import", func() { tmpDir := GinkgoT().TempDir() mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) m3u := "#EXTALBUMARTURL:https://example.com/new-cover.jpg\ntest.mp3\n" plsFile := filepath.Join(tmpDir, "test.m3u") @@ -285,7 +285,7 @@ var _ = Describe("Playlists - Import", func() { tmpDir := GinkgoT().TempDir() mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFile := filepath.Join(tmpDir, "test.m3u") Expect(os.WriteFile(plsFile, []byte("test.mp3\n"), 0600)).To(Succeed()) @@ -311,7 +311,7 @@ var _ = Describe("Playlists - Import", func() { tmpDir := GinkgoT().TempDir() mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{"test.mp3"}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) m3u := "test.mp3\n" plsFile := filepath.Join(tmpDir, "test.m3u") @@ -388,7 +388,7 @@ var _ = Describe("Playlists - Import", func() { tmpDir := GinkgoT().TempDir() mockLibRepo.SetData([]model.Library{{ID: 1, Path: tmpDir}}) ds.MockedMediaFile = &mockedMediaFileFromListRepo{data: []string{}} - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) // Create the playlist file on disk with the filesystem's normalization form plsFile := tmpDir + "/" + filesystemName + ".m3u" @@ -448,7 +448,7 @@ var _ = Describe("Playlists - Import", func() { "def.mp3", // This is playlists/def.mp3 relative to plsDir }, } - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("handles relative paths that reference files in other libraries", func() { @@ -604,7 +604,7 @@ var _ = Describe("Playlists - Import", func() { }, } // Recreate playlists service to pick up new mock - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) // Create playlist in music library that references both tracks plsContent := "#PLAYLIST:Same Path Test\nalbum/track.mp3\n../classical/album/track.mp3" @@ -662,7 +662,7 @@ var _ = Describe("Playlists - Import", func() { }, } ds.MockedFolder = mockFolderRepo - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsContent := "#PLAYLIST:My Playlist\ntest.mp3\ntest.ogg\n" plsFile := filepath.Join(tmpDir, "my-playlist.m3u") @@ -681,7 +681,7 @@ var _ = Describe("Playlists - Import", func() { libDir := filepath.Join(tmpDir, "music") Expect(os.Mkdir(libDir, 0755)).To(Succeed()) mockLibRepo.SetData([]model.Library{{ID: 1, Path: libDir}}) - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsContent := "#PLAYLIST:External Playlist\n" + libDir + "/test.mp3\n" plsFile := filepath.Join(tmpDir, "external.m3u") @@ -704,7 +704,7 @@ var _ = Describe("Playlists - Import", func() { }, } ds.MockedFolder = mockFolderRepo - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFile := filepath.Join(tmpDir, "test.m3u") Expect(os.WriteFile(plsFile, []byte("test.mp3\n"), 0600)).To(Succeed()) @@ -724,7 +724,7 @@ var _ = Describe("Playlists - Import", func() { }, } ds.MockedFolder = mockFolderRepo - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFile := filepath.Join(tmpDir, "test.m3u") Expect(os.WriteFile(plsFile, []byte("test.mp3\n"), 0600)).To(Succeed()) @@ -744,7 +744,7 @@ var _ = Describe("Playlists - Import", func() { }, } ds.MockedFolder = mockFolderRepo - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) plsFile := filepath.Join(tmpDir, "test.m3u") Expect(os.WriteFile(plsFile, []byte("test.mp3\n"), 0600)).To(Succeed()) @@ -767,7 +767,7 @@ var _ = Describe("Playlists - Import", func() { BeforeEach(func() { repo = &mockedMediaFileFromListRepo{} ds.MockedMediaFile = repo - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) mockLibRepo.SetData([]model.Library{{ID: 1, Path: "/music"}, {ID: 2, Path: "/new"}}) ctx = request.WithUser(ctx, model.User{ID: "123"}) }) diff --git a/core/playlists/playlists_test.go b/core/playlists/playlists_test.go index 0c9674bed..4120fd2f5 100644 --- a/core/playlists/playlists_test.go +++ b/core/playlists/playlists_test.go @@ -42,7 +42,7 @@ var _ = Describe("Playlists", func() { "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("allows owner to delete their playlist", func() { @@ -82,7 +82,7 @@ var _ = Describe("Playlists", func() { "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("returns the playlist's track repository", func() { @@ -103,7 +103,7 @@ var _ = Describe("Playlists", func() { "pls-smart": {ID: "pls-smart", Name: "Smart", OwnerID: "user-1", Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}}, } - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("creates a new playlist with owner set from context", func() { @@ -161,7 +161,7 @@ var _ = Describe("Playlists", func() { Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("allows owner to update their playlist", func() { @@ -219,7 +219,7 @@ var _ = Describe("Playlists", func() { "pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("allows owner to add tracks", func() { @@ -267,7 +267,7 @@ var _ = Describe("Playlists", func() { Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("allows owner to remove tracks", func() { @@ -301,7 +301,7 @@ var _ = Describe("Playlists", func() { Rules: &criteria.Criteria{Expression: criteria.Contains{"title": "test"}}}, } mockPlsRepo.TracksRepo = mockTracks - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("allows owner to reorder", func() { @@ -330,7 +330,7 @@ var _ = Describe("Playlists", func() { "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"}, "pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"}, } - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("saves image file and updates UploadedImage", func() { @@ -400,7 +400,7 @@ var _ = Describe("Playlists", func() { "pls-empty": {ID: "pls-empty", Name: "No Cover", OwnerID: "user-1"}, "pls-other": {ID: "pls-other", Name: "Other's", OwnerID: "other-user"}, } - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) It("removes file and clears UploadedImage", func() { diff --git a/core/playlists/rest_adapter_test.go b/core/playlists/rest_adapter_test.go index 58a327bde..0c18c416b 100644 --- a/core/playlists/rest_adapter_test.go +++ b/core/playlists/rest_adapter_test.go @@ -37,7 +37,7 @@ var _ = Describe("REST Adapter", func() { mockPlsRepo.Data = map[string]*model.Playlist{ "pls-1": {ID: "pls-1", Name: "My Playlist", OwnerID: "user-1"}, } - ps = playlists.NewPlaylists(ds, core.NewImageUploadService()) + ps = playlists.NewPlaylists(ds, core.NewImageUploadService(ds)) }) Describe("Save", func() { diff --git a/persistence/e2e/e2e_suite_test.go b/persistence/e2e/e2e_suite_test.go index ac7668218..29291f66e 100644 --- a/persistence/e2e/e2e_suite_test.go +++ b/persistence/e2e/e2e_suite_test.go @@ -275,7 +275,7 @@ var _ = BeforeSuite(func() { buildTestFS() s := scanner.New(ctx, initDS, events.NoopBroker(), - playlists.NewPlaylists(initDS, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(initDS, core.NewImageUploadService(initDS)), metrics.NewNoopInstance()) _, err = s.ScanAll(ctx, true) Expect(err).ToNot(HaveOccurred()) diff --git a/persistence/radio_repository.go b/persistence/radio_repository.go index d48178977..ac3950005 100644 --- a/persistence/radio_repository.go +++ b/persistence/radio_repository.go @@ -106,9 +106,10 @@ func (r *radioRepository) Put(radio *model.Radio, colsToUpdate ...string) error if err != nil { return err } - // Enqueue artwork resolution for the created/updated radio. Never fails the save. + // Enqueue artwork resolution for the created/updated radio at Bump priority so a new + // radio's cover resolves proactively. Never fails the save. item := model.ArtworkQueueItem{ItemKind: "ra", ItemID: radio.ID, ImageType: model.ImageTypePrimary, - Priority: model.ArtworkPriorityScan} + Priority: model.ArtworkPriorityBump} if err := NewArtworkQueueRepository(r.ctx, r.db).Enqueue(item); err != nil { log.Warn(r.ctx, "could not enqueue radio artwork", "id", radio.ID, err) } diff --git a/persistence/radio_repository_test.go b/persistence/radio_repository_test.go index 4b243c142..1ea8b971c 100644 --- a/persistence/radio_repository_test.go +++ b/persistence/radio_repository_test.go @@ -140,7 +140,7 @@ var _ = Describe("RadioRepository", func() { Expect(queued).To(ContainElement(SatisfyAll( HaveField("ItemKind", "ra"), HaveField("ItemID", created.ID), - HaveField("Priority", model.ArtworkPriorityScan), + HaveField("Priority", model.ArtworkPriorityBump), ))) }) }) diff --git a/scanner/controller_test.go b/scanner/controller_test.go index 921841a2d..3dbb36eeb 100644 --- a/scanner/controller_test.go +++ b/scanner/controller_test.go @@ -31,7 +31,7 @@ var _ = Describe("Controller", func() { DeferCleanup(configtest.SetupConfig()) ds = &tests.MockDataStore{RealDS: persistence.New(db.Db())} ds.MockedProperty = &tests.MockedPropertyRepo{} - ctrl = scanner.New(ctx, ds, events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + ctrl = scanner.New(ctx, ds, events.NoopBroker(), playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) }) It("includes last scan error", func() { diff --git a/scanner/scanner_benchmark_test.go b/scanner/scanner_benchmark_test.go index 6252b272a..0abac5f40 100644 --- a/scanner/scanner_benchmark_test.go +++ b/scanner/scanner_benchmark_test.go @@ -40,7 +40,7 @@ func BenchmarkScan(b *testing.B) { ds := persistence.New(db.Db()) conf.Server.DevExternalScanner = false s := scanner.New(context.Background(), ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) fs := storagetest.FakeFS{} storagetest.Register("fake", &fs) diff --git a/scanner/scanner_multilibrary_test.go b/scanner/scanner_multilibrary_test.go index 83e174444..e6650f39d 100644 --- a/scanner/scanner_multilibrary_test.go +++ b/scanner/scanner_multilibrary_test.go @@ -78,7 +78,7 @@ var _ = Describe("Scanner - Multi-Library", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) // Create two test libraries (let DB auto-assign IDs) lib1 = model.Library{Name: "Rock Collection", Path: "rock:///music"} diff --git a/scanner/scanner_selective_test.go b/scanner/scanner_selective_test.go index 4eec23e89..ca87a5f6b 100644 --- a/scanner/scanner_selective_test.go +++ b/scanner/scanner_selective_test.go @@ -66,7 +66,7 @@ var _ = Describe("ScanFolders", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) lib = model.Library{ID: 1, Name: "Fake Library", Path: "fake:///music"} Expect(ds.Library(ctx).Put(&lib)).To(Succeed()) diff --git a/scanner/scanner_test.go b/scanner/scanner_test.go index 46c45e522..48eb2fedd 100644 --- a/scanner/scanner_test.go +++ b/scanner/scanner_test.go @@ -86,7 +86,7 @@ var _ = Describe("Scanner", Ordered, func() { Expect(ds.User(ctx).Put(&adminUser)).To(Succeed()) s = scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) lib = model.Library{ID: 1, Name: "Fake Library", Path: "fake:///music"} Expect(ds.Library(ctx).Put(&lib)).To(Succeed()) diff --git a/server/jellyfin/e2e/e2e_suite_test.go b/server/jellyfin/e2e/e2e_suite_test.go index 1986a95ec..e7ea172e3 100644 --- a/server/jellyfin/e2e/e2e_suite_test.go +++ b/server/jellyfin/e2e/e2e_suite_test.go @@ -326,7 +326,7 @@ func setupTestDB() { decider, core.NewPlayers(ds), scrobbler.NewPlayTracker(ds, events.NoopBroker(), nil), - playlists.NewPlaylists(ds, core.NewImageUploadService()), + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), providerFake, sonicSvc, lyrics.NewLyrics(ds, nil), diff --git a/server/nativeapi/artwork.go b/server/nativeapi/artwork.go new file mode 100644 index 000000000..cefbccff4 --- /dev/null +++ b/server/nativeapi/artwork.go @@ -0,0 +1,49 @@ +package nativeapi + +import ( + "net/http" + + "github.com/go-chi/chi/v5" + "github.com/navidrome/navidrome/log" + "github.com/navidrome/navidrome/model" +) + +// refreshableArtworkKinds are the entity kinds a manual re-resolve accepts. +var refreshableArtworkKinds = map[string]bool{ + model.KindAlbumArtwork.Prefix(): true, + model.KindArtistArtwork.Prefix(): true, + model.KindPlaylistArtwork.Prefix(): true, + model.KindRadioArtwork.Prefix(): true, + model.KindMediaFileArtwork.Prefix(): true, +} + +func (api *Router) addArtworkRoute(r chi.Router) { + r.Post("/artwork/{kind}/{id}/refresh", api.refreshArtwork()) +} + +// refreshArtwork clears an item's resolved artwork state and re-queues it at Bump priority. +// State is deliberately cleared so a wrong pick disappears immediately (placeholder until re-resolved). +func (api *Router) refreshArtwork() http.HandlerFunc { + return func(w http.ResponseWriter, r *http.Request) { + ctx := r.Context() + kind := chi.URLParam(r, "kind") + id := chi.URLParam(r, "id") + if !refreshableArtworkKinds[kind] { + http.Error(w, "invalid artwork kind", http.StatusBadRequest) + return + } + if err := api.ds.Artwork(ctx).DeleteForItem(kind, id); err != nil { + log.Error(ctx, "Error clearing artwork state", "kind", kind, "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + item := model.ArtworkQueueItem{ItemKind: kind, ItemID: id, ImageType: model.ImageTypePrimary, + Priority: model.ArtworkPriorityBump} + if err := api.ds.ArtworkQueue(ctx).Enqueue(item); err != nil { + log.Error(ctx, "Error enqueuing artwork refresh", "kind", kind, "id", id, err) + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + w.WriteHeader(http.StatusNoContent) + } +} diff --git a/server/nativeapi/artwork_test.go b/server/nativeapi/artwork_test.go new file mode 100644 index 000000000..c84e83fdc --- /dev/null +++ b/server/nativeapi/artwork_test.go @@ -0,0 +1,95 @@ +package nativeapi + +import ( + "context" + "net/http" + "net/http/httptest" + + "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" + "github.com/navidrome/navidrome/core/auth" + "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/server" + "github.com/navidrome/navidrome/tests" + . "github.com/onsi/ginkgo/v2" + . "github.com/onsi/gomega" +) + +var _ = Describe("Artwork API", func() { + var ds *tests.MockDataStore + var artRepo *tests.MockArtworkRepo + var queueRepo *tests.MockArtworkQueueRepo + var router http.Handler + var adminToken, userToken string + + BeforeEach(func() { + DeferCleanup(configtest.SetupConfig()) + conf.Server.EnableSharing = false + artRepo = tests.CreateMockArtworkRepo() + queueRepo = tests.CreateMockArtworkQueueRepo() + ds = &tests.MockDataStore{MockedArtwork: artRepo, MockedArtworkQueue: queueRepo} + auth.Init(ds) + nativeRouter := New(ds, nil, nil, nil, tests.NewMockLibraryService(), tests.NewMockUserService(), nil, nil, nil) + router = server.JWTVerifier(nativeRouter) + + adminUser := model.User{ID: "admin-1", UserName: "admin", IsAdmin: true, NewPassword: "adminpass"} + regularUser := model.User{ID: "user-1", UserName: "regular", IsAdmin: false, NewPassword: "userpass"} + Expect(ds.User(context.TODO()).Put(&adminUser)).To(Succeed()) + Expect(ds.User(context.TODO()).Put(®ularUser)).To(Succeed()) + + var err error + adminToken, err = auth.CreateToken(&adminUser) + Expect(err).ToNot(HaveOccurred()) + userToken, err = auth.CreateToken(®ularUser) + Expect(err).ToNot(HaveOccurred()) + }) + + Describe("POST /api/artwork/{kind}/{id}/refresh", func() { + It("clears state and enqueues a Bump for admins", func() { + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: "al", ItemID: "al-1", Hash: "oldhash", Source: "external", + })).To(Succeed()) + + req := createAuthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil, adminToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusNoContent)) + + _, err := artRepo.GetItemArtwork("al", "al-1", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound)) + + queued, err := queueRepo.DequeueBatch(1000) + Expect(err).ToNot(HaveOccurred()) + Expect(queued).To(ContainElement(SatisfyAll( + HaveField("ItemKind", "al"), + HaveField("ItemID", "al-1"), + HaveField("Priority", model.ArtworkPriorityBump), + ))) + }) + + It("returns 400 for an invalid kind", func() { + req := createAuthenticatedRequest("POST", "/artwork/xx/id-1/refresh", nil, adminToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusBadRequest)) + }) + + It("denies access to regular users", func() { + req := createAuthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil, userToken) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusForbidden)) + }) + + It("denies access without authentication", func() { + req := createUnauthenticatedRequest("POST", "/artwork/al/al-1/refresh", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + Expect(w.Code).To(Equal(http.StatusUnauthorized)) + }) + }) +}) diff --git a/server/nativeapi/native_api.go b/server/nativeapi/native_api.go index 5a7023eb6..3f38e2fe5 100644 --- a/server/nativeapi/native_api.go +++ b/server/nativeapi/native_api.go @@ -91,6 +91,7 @@ func (api *Router) routes() http.Handler { api.addConfigRoute(r) api.addUserLibraryRoute(r) api.addPluginRoute(r) + api.addArtworkRoute(r) api.RX(r, "/library", api.libs.NewRepository, true) }) }) diff --git a/server/subsonic/e2e/e2e_suite_test.go b/server/subsonic/e2e/e2e_suite_test.go index c84680a11..af15f17df 100644 --- a/server/subsonic/e2e/e2e_suite_test.go +++ b/server/subsonic/e2e/e2e_suite_test.go @@ -409,7 +409,7 @@ func setupTestDB() { streamerSpy = &harness.SpyStreamer{} decider := stream.NewTranscodeDecider(ds, harness.NoopFFmpeg{}) s := scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) router = subsonic.New( ds, noopArtwork{}, @@ -419,7 +419,7 @@ func setupTestDB() { noopProvider{}, s, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), scrobbler.NewPlayTracker(ds, events.NoopBroker(), nil), core.NewShare(ds), playback.PlaybackServer(nil), diff --git a/server/subsonic/e2e/subsonic_multilibrary_test.go b/server/subsonic/e2e/subsonic_multilibrary_test.go index 2ecc3d863..18ce76b4a 100644 --- a/server/subsonic/e2e/subsonic_multilibrary_test.go +++ b/server/subsonic/e2e/subsonic_multilibrary_test.go @@ -53,7 +53,7 @@ var _ = Describe("Multi-Library Support", Ordered, func() { // Run incremental scan to import lib2 content (lib1 files unchanged → skipped) s := scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) _, err = s.ScanAll(ctx, false) Expect(err).ToNot(HaveOccurred()) diff --git a/server/subsonic/e2e/subsonic_sonic_similarity_test.go b/server/subsonic/e2e/subsonic_sonic_similarity_test.go index 775fefe89..672cdf1d3 100644 --- a/server/subsonic/e2e/subsonic_sonic_similarity_test.go +++ b/server/subsonic/e2e/subsonic_sonic_similarity_test.go @@ -43,7 +43,7 @@ func buildSonicRouter(provider sonic.Provider) *subsonic.Router { noopProvider{}, nil, // scanner events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), scrobbler.NewPlayTracker(ds, events.NoopBroker(), nil), core.NewShare(ds), playback.PlaybackServer(nil), diff --git a/tests/harness/harness.go b/tests/harness/harness.go index dabb6f0ea..9e9e9c4ed 100644 --- a/tests/harness/harness.go +++ b/tests/harness/harness.go @@ -74,7 +74,7 @@ func SetupDB(ctx context.Context, users ...*model.User) *DB { } s := scanner.New(ctx, ds, events.NoopBroker(), - playlists.NewPlaylists(ds, core.NewImageUploadService()), metrics.NewNoopInstance()) + playlists.NewPlaylists(ds, core.NewImageUploadService(ds)), metrics.NewNoopInstance()) _, err := s.ScanAll(ctx, true) Expect(err).ToNot(HaveOccurred())