mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
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.
This commit is contained in:
parent
f4e14e9c1a
commit
66d3d23149
@ -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
|
||||
|
||||
@ -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() {
|
||||
|
||||
@ -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 {
|
||||
|
||||
@ -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
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
@ -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
|
||||
})
|
||||
}
|
||||
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user