From 83ae195c101497d34a7510b60ee373792ce0eb4d Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 17 Jul 2026 23:16:10 -0400 Subject: [PATCH] fix(album): don't delete cover files still shared by another album row After an album-ID copy (e.g. a partial track move), two album rows can reference the same uploaded image file. Replacing or deleting the cover on one of them removed the shared file out from under the other. Add AlbumRepository.CountByImage (unfiltered by library) and skip the file removal in the upload/delete handlers when the filename is referenced by more than one row; the purge GC reaps the file when the last reference goes. --- model/album.go | 1 + persistence/album_repository.go | 9 +++++++++ persistence/album_repository_test.go | 6 ++++++ server/nativeapi/albums.go | 29 ++++++++++++++++++++++++++-- tests/mock_album_repo.go | 13 +++++++++++++ 5 files changed, 56 insertions(+), 2 deletions(-) diff --git a/model/album.go b/model/album.go index 888338df3..f0454752f 100644 --- a/model/album.go +++ b/model/album.go @@ -147,6 +147,7 @@ type AlbumRepository interface { Put(*Album) error UpdateExternalInfo(*Album) error UpdateImage(id, filename string) error + CountByImage(filename string) (int64, error) Get(id string) (*Album, error) GetAll(...QueryOptions) (Albums, error) GetCursor(...QueryOptions) (AlbumCursor, error) diff --git a/persistence/album_repository.go b/persistence/album_repository.go index 89ce7fb88..3add4e90e 100644 --- a/persistence/album_repository.go +++ b/persistence/album_repository.go @@ -238,6 +238,15 @@ func (r *albumRepository) UpdateImage(id, filename string) error { return nil } +// CountByImage counts album rows referencing an uploaded image filename, unfiltered by +// library — CopyAttributes can leave two rows sharing one file across an album-ID change. +func (r *albumRepository) CountByImage(filename string) (int64, error) { + if filename == "" { + return 0, nil + } + return r.count(Select(), model.QueryOptions{Filters: Eq{"uploaded_image": filename}}) +} + func (r *albumRepository) selectAlbum(options ...model.QueryOptions) SelectBuilder { sql := r.newSelect(options...).Columns("album.*", "library.path as library_path", "library.name as library_name"). LeftJoin("library on album.library_id = library.id") diff --git a/persistence/album_repository_test.go b/persistence/album_repository_test.go index a3883cb1a..a08425f85 100644 --- a/persistence/album_repository_test.go +++ b/persistence/album_repository_test.go @@ -73,6 +73,12 @@ var _ = Describe("AlbumRepository", func() { It("returns ErrNotFound for a missing album", func() { Expect(albumRepo.UpdateImage("does-not-exist", "x.jpg")).To(MatchError(model.ErrNotFound)) }) + It("counts rows sharing an image filename, ignoring library filters", func() { + Expect(albumRepo.CountByImage("img-1_cover.jpg")).To(Equal(int64(0))) + Expect(albumRepo.UpdateImage("img-1", "img-1_cover.jpg")).To(Succeed()) + Expect(albumRepo.CountByImage("img-1_cover.jpg")).To(Equal(int64(1))) + Expect(albumRepo.CountByImage("")).To(Equal(int64(0))) + }) It("bumps cover_art_updated_at without touching updated_at", func() { before, err := albumRepo.Get("img-1") Expect(err).ToNot(HaveOccurred()) diff --git a/server/nativeapi/albums.go b/server/nativeapi/albums.go index ed63e5a3b..7608d94cf 100644 --- a/server/nativeapi/albums.go +++ b/server/nativeapi/albums.go @@ -38,7 +38,11 @@ func (api *Router) uploadAlbumImage() http.HandlerFunc { } return err } - filename, err := api.imgUpload.SetImage(ctx, consts.EntityAlbum, al.ID, al.Name, al.UploadedImagePath(), reader, ext) + oldPath, err := api.albumImagePathToRemove(ctx, al) + if err != nil { + return err + } + filename, err := api.imgUpload.SetImage(ctx, consts.EntityAlbum, al.ID, al.Name, oldPath, reader, ext) if err != nil { return err } @@ -46,6 +50,23 @@ func (api *Router) uploadAlbumImage() http.HandlerFunc { }) } +// albumImagePathToRemove returns the album's current image path, or "" when the file is +// shared with another album row (post album-ID copy) and must be left on disk. +func (api *Router) albumImagePathToRemove(ctx context.Context, al *model.Album) (string, error) { + path := al.UploadedImagePath() + if path == "" { + return "", nil + } + refs, err := api.ds.Album(ctx).CountByImage(al.UploadedImage) + if err != nil { + return "", err + } + if refs > 1 { + return "", nil + } + return path, nil +} + func (api *Router) deleteAlbumImage() http.HandlerFunc { return handleImageDelete(func(ctx context.Context) error { albumID := chi.URLParamFromCtx(ctx, "id") @@ -56,7 +77,11 @@ func (api *Router) deleteAlbumImage() http.HandlerFunc { } return err } - if err := api.imgUpload.RemoveImage(ctx, al.UploadedImagePath()); err != nil { + oldPath, err := api.albumImagePathToRemove(ctx, al) + if err != nil { + return err + } + if err := api.imgUpload.RemoveImage(ctx, oldPath); err != nil { return err } return api.ds.Album(ctx).UpdateImage(al.ID, "") diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index 843552fc6..f54805ad3 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -147,6 +147,19 @@ func (m *MockAlbumRepo) UpdateImage(id, filename string) error { return model.ErrNotFound } +func (m *MockAlbumRepo) CountByImage(filename string) (int64, error) { + if m.Err { + return 0, errors.New("unexpected error") + } + var n int64 + for _, al := range m.Data { + if filename != "" && al.UploadedImage == filename { + n++ + } + } + return n, nil +} + func (m *MockAlbumRepo) Search(q string, options ...model.QueryOptions) (model.Albums, error) { m.SearchQuery = q if len(options) > 0 {