From 66d3d231490156b94897a7df161c976246e62fd5 Mon Sep 17 00:00:00 2001 From: Deluan Date: Thu, 23 Jul 2026 00:43:04 -0400 Subject: [PATCH] fix(artwork): enqueue uploaded artwork only after the filename is persisted SetImage cleared state and enqueued the bump before the caller stored the new filename, so a worker drain in that window could resolve against the old (already deleted) file and settle absent, leaving the upload unused until a later scan. Move the invalidate+enqueue into EnqueueArtwork, which each caller now invokes after the entity Put. --- core/image_upload.go | 9 +++++---- core/image_upload_test.go | 24 +++++++++++++++++++++++- core/playlists/playlists.go | 7 ++++++- server/nativeapi/artists.go | 6 +++++- server/nativeapi/radios.go | 6 +++++- 5 files changed, 44 insertions(+), 8 deletions(-) diff --git a/core/image_upload.go b/core/image_upload.go index 6f5ef418a..ce32f8258 100644 --- a/core/image_upload.go +++ b/core/image_upload.go @@ -18,6 +18,9 @@ import ( type ImageUploadService interface { SetImage(ctx context.Context, entityType string, entityID string, name string, oldPath string, reader io.Reader, ext string) (filename string, err error) RemoveImage(ctx context.Context, path string) error + // EnqueueArtwork clears an item's resolved state and re-queues it at Bump priority. Callers + // must invoke it AFTER persisting the new filename, so the worker never resolves the old one. + EnqueueArtwork(ctx context.Context, entityType, entityID string) } // MaxImageUploadSize returns the configured MaxImageUploadSize in bytes, or the built-in default @@ -71,14 +74,12 @@ func (s *imageUploadService) SetImage(ctx context.Context, entityType string, en if _, err := io.Copy(f, reader); err != nil { 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 +// EnqueueArtwork 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) { +func (s *imageUploadService) EnqueueArtwork(ctx context.Context, entityType, id string) { kind, ok := uploadEntityKind[entityType] if !ok { return diff --git a/core/image_upload_test.go b/core/image_upload_test.go index 1b63b3052..29ce69298 100644 --- a/core/image_upload_test.go +++ b/core/image_upload_test.go @@ -77,7 +77,7 @@ var _ = Describe("ImageUploadService", func() { Expect(err).ToNot(HaveOccurred()) }) - It("clears artwork state and enqueues a Bump on success", func() { + It("does not touch artwork state or the queue (that is EnqueueArtwork's job, post-Put)", func() { ctx := context.Background() Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ ItemKind: "ar", ItemID: "ar-1", Hash: "oldhash", Source: "external", @@ -86,7 +86,24 @@ var _ = Describe("ImageUploadService", func() { _, err := svc.SetImage(ctx, consts.EntityArtist, "ar-1", "Pink Floyd", "", strings.NewReader("img"), ".jpg") Expect(err).ToNot(HaveOccurred()) + // SetImage only writes the file; the state row survives and nothing is queued until + // the caller has persisted the new filename and called EnqueueArtwork. _, err = artRepo.GetItemArtwork("ar", "ar-1", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(queueRepo.DequeueBatch(1000)).To(BeEmpty()) + }) + }) + + Describe("EnqueueArtwork", func() { + It("clears artwork state and enqueues a Bump", func() { + ctx := context.Background() + Expect(artRepo.PutItemArtwork(&model.ItemArtwork{ + ItemKind: "ar", ItemID: "ar-1", Hash: "oldhash", Source: "external", + })).To(Succeed()) + + svc.EnqueueArtwork(ctx, consts.EntityArtist, "ar-1") + + _, err := artRepo.GetItemArtwork("ar", "ar-1", model.ImageTypePrimary) Expect(err).To(MatchError(model.ErrNotFound)) queued, err := queueRepo.DequeueBatch(1000) @@ -97,6 +114,11 @@ var _ = Describe("ImageUploadService", func() { HaveField("Priority", model.ArtworkPriorityBump), ))) }) + + It("is a no-op for an unknown entity type", func() { + svc.EnqueueArtwork(context.Background(), "unknown", "x-1") + Expect(queueRepo.DequeueBatch(1000)).To(BeEmpty()) + }) }) Describe("RemoveImage", func() { diff --git a/core/playlists/playlists.go b/core/playlists/playlists.go index 1ef083bbb..a4a26c0f5 100644 --- a/core/playlists/playlists.go +++ b/core/playlists/playlists.go @@ -57,6 +57,7 @@ type Playlists interface { type ImageUploadService interface { SetImage(ctx context.Context, entityType string, entityID string, name string, oldPath string, reader io.Reader, ext string) (filename string, err error) RemoveImage(ctx context.Context, path string) error + EnqueueArtwork(ctx context.Context, entityType, entityID string) } type playlists struct { @@ -320,7 +321,11 @@ func (s *playlists) SetImage(ctx context.Context, playlistID string, reader io.R } pls.UploadedImage = filename - return s.ds.Playlist(ctx).Put(pls) + if err := s.ds.Playlist(ctx).Put(pls); err != nil { + return err + } + s.imgUpload.EnqueueArtwork(ctx, consts.EntityPlaylist, pls.ID) + return nil } func (s *playlists) RemoveImage(ctx context.Context, playlistID string) error { diff --git a/server/nativeapi/artists.go b/server/nativeapi/artists.go index daa918d00..90d4c0d27 100644 --- a/server/nativeapi/artists.go +++ b/server/nativeapi/artists.go @@ -46,7 +46,11 @@ func (api *Router) uploadArtistImage() http.HandlerFunc { } ar.UploadedImage = filename ar.UpdatedAt = new(time.Now()) - return api.ds.Artist(ctx).Put(ar, "uploaded_image", "updated_at") + if err := api.ds.Artist(ctx).Put(ar, "uploaded_image", "updated_at"); err != nil { + return err + } + api.imgUpload.EnqueueArtwork(ctx, consts.EntityArtist, ar.ID) + return nil }) } diff --git a/server/nativeapi/radios.go b/server/nativeapi/radios.go index 701c6c926..3868a3501 100644 --- a/server/nativeapi/radios.go +++ b/server/nativeapi/radios.go @@ -47,7 +47,11 @@ func (api *Router) uploadRadioImage() http.HandlerFunc { return err } radio.UploadedImage = filename - return api.ds.Radio(ctx).Put(radio, "UploadedImage") + if err := api.ds.Radio(ctx).Put(radio, "UploadedImage"); err != nil { + return err + } + api.imgUpload.EnqueueArtwork(ctx, consts.EntityRadio, radio.ID) + return nil }) }