From d704587eb51e190c83dc1ae21f4b20b6583c3105 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 1 Aug 2026 21:10:01 -0400 Subject: [PATCH] refactor(artwork): split the repository's deletes from its purges by name Six removal methods across the two artwork repositories used Delete and Purge interchangeably, so neither prefix told you anything. There is a real distinction underneath: Delete methods are caller-directed and take the rows to remove, while Purge methods are garbage collection - the repository finds rows whose referent is gone by joining against the owning table, and returns a count because the caller cannot know one. The signatures already followed that split; only the names did not. DeleteOrphans was the odd one out, taking a grace cutoff rather than a target and returning a count, so it becomes PurgeOrphans. PurgeDanglingItemArtwork loses the Artwork it repeats from its own interface and keeps the Items that says which of that interface's two tables it means. DeleteForItem was DeleteForItems with one id - squirrel emits = for a scalar and IN for a slice against the same predicate - so its one caller passes a slice instead. Dangling and Orphan stay distinct: dangling is a row whose entity is gone, orphan is an image no state row references. --- core/artwork/housekeeping.go | 2 +- core/artwork/prune.go | 4 ++-- model/artwork.go | 10 +++++----- persistence/artwork_repository.go | 8 ++------ persistence/artwork_repository_test.go | 10 +++++----- tests/mock_artwork_repo.go | 18 +++--------------- 6 files changed, 18 insertions(+), 34 deletions(-) diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index cc4bbbf1c..ce72a6c8e 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -118,7 +118,7 @@ func enqueueMissingAll(ctx context.Context, ds model.DataStore) error { // Refresh drops an item's resolved artwork state and re-queues it at Bump priority. func Refresh(ctx context.Context, ds model.DataStore, kind model.Kind, id string) error { - if err := ds.Artwork(ctx).DeleteForItem(kind, id); err != nil { + if err := ds.Artwork(ctx).DeleteForItems(kind, []string{id}); err != nil { return fmt.Errorf("clearing artwork state: %w", err) } item := model.ArtworkQueueItem{ItemKind: kind.Prefix(), ItemID: id, ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBump} diff --git a/core/artwork/prune.go b/core/artwork/prune.go index 6c78c8c17..9d00bdfec 100644 --- a/core/artwork/prune.go +++ b/core/artwork/prune.go @@ -16,7 +16,7 @@ func prune(ctx context.Context, ds model.DataStore, store *ImageStore) error { defer func() { log.Debug(ctx, "Artwork: Prune finished", "elapsed", time.Since(start)) }() repo := ds.Artwork(ctx) - purged, err := repo.PurgeDanglingItemArtwork() + purged, err := repo.PurgeDanglingItems() if err != nil { return err } @@ -35,7 +35,7 @@ func prune(ctx context.Context, ds model.DataStore, store *ImageStore) error { // Files younger than the grace window may belong to acquisitions whose rows aren't committed yet. cutoff := time.Now().Add(-pruneMinAge) - orphans, err := repo.DeleteOrphans(cutoff) + orphans, err := repo.PurgeOrphans(cutoff) if err != nil { return err } diff --git a/model/artwork.go b/model/artwork.go index f4cae956f..0513ebaf8 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -95,21 +95,21 @@ const ( ArtworkPriorityBump = 100 ) +// Delete* takes the rows to remove; Purge* finds them itself and reports how many went. type ArtworkRepository interface { GetImage(hash string) (*Artwork, error) PutImage(a *Artwork) error - // DeleteOrphans deletes rows referenced by no item_artwork row and older than cutoff. - DeleteOrphans(createdBefore time.Time) (int64, error) + // PurgeOrphans deletes rows referenced by no item_artwork row and older than cutoff. + PurgeOrphans(createdBefore time.Time) (int64, error) GetItemArtwork(kind Kind, id, imageType string) (*ItemArtwork, error) PutItemArtwork(ia *ItemArtwork) error - DeleteForItem(kind Kind, id string) error DeleteForItems(kind Kind, ids []string) error // GetInfoForItems hydrates a page in one batched query. GetInfoForItems(kind Kind, ids []string) (map[string]ItemArtworkInfo, error) // GetMimeByHash returns hash -> current mime for every stored artwork. GetMimeByHash() (map[string]string, error) - // PurgeDanglingItemArtwork removes state rows whose entity no longer exists. - PurgeDanglingItemArtwork() (int64, error) + // PurgeDanglingItems removes state rows whose entity no longer exists. + PurgeDanglingItems() (int64, error) } type ArtworkQueueRepository interface { diff --git a/persistence/artwork_repository.go b/persistence/artwork_repository.go index de97e604e..6c49e8057 100644 --- a/persistence/artwork_repository.go +++ b/persistence/artwork_repository.go @@ -72,7 +72,7 @@ func (r *artworkRepository) GetMimeByHash() (map[string]string, error) { return res, nil } -func (r *artworkRepository) DeleteOrphans(createdBefore time.Time) (int64, error) { +func (r *artworkRepository) PurgeOrphans(createdBefore time.Time) (int64, error) { del := Delete(r.tableName).Where(And{ Lt{"created_at": createdBefore}, Expr("hash NOT IN (SELECT hash FROM " + itemArtworkTable + " WHERE hash <> '')"), @@ -106,7 +106,7 @@ func purgeDangling(r sqlRepository) (int64, error) { return total, nil } -func (r *artworkRepository) PurgeDanglingItemArtwork() (int64, error) { +func (r *artworkRepository) PurgeDanglingItems() (int64, error) { return purgeDangling(r.items) } @@ -138,10 +138,6 @@ func (r *artworkRepository) PutItemArtwork(ia *model.ItemArtwork) error { return err } -func (r *artworkRepository) DeleteForItem(kind model.Kind, id string) error { - return r.items.delete(Eq{"item_kind": kind.Prefix(), "item_id": id}) -} - func (r *artworkRepository) DeleteForItems(kind model.Kind, ids []string) error { for chunk := range slices.Chunk(ids, artworkBatchSize) { if err := r.items.delete(Eq{"item_kind": kind.Prefix(), "item_id": chunk}); err != nil { diff --git a/persistence/artwork_repository_test.go b/persistence/artwork_repository_test.go index 145dcf980..c1527fcf8 100644 --- a/persistence/artwork_repository_test.go +++ b/persistence/artwork_repository_test.go @@ -101,7 +101,7 @@ var _ = Describe("ArtworkRepository", func() { Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: "a1", ImageType: model.ImageTypePrimary, Hash: "dref", Source: "folder"})).To(Succeed()) - Expect(repo.DeleteOrphans(time.Now().Add(time.Minute))).To(BeNumerically("==", 1)) + Expect(repo.PurgeOrphans(time.Now().Add(time.Minute))).To(BeNumerically("==", 1)) _, err := repo.GetImage("d1") Expect(err).To(MatchError(model.ErrNotFound)) @@ -111,7 +111,7 @@ var _ = Describe("ArtworkRepository", func() { It("spares an unreferenced row younger than the cutoff", func() { Expect(repo.PutImage(&model.Artwork{Hash: "young", Mime: "image/jpeg"})).To(Succeed()) - Expect(repo.DeleteOrphans(time.Now().Add(-time.Hour))).To(BeNumerically("==", 0)) + Expect(repo.PurgeOrphans(time.Now().Add(-time.Hour))).To(BeNumerically("==", 0)) _, err := repo.GetImage("young") Expect(err).ToNot(HaveOccurred()) }) @@ -130,7 +130,7 @@ var _ = Describe("ArtworkRepository", func() { Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: songDayInALife.ID, ImageType: model.ImageTypePrimary, Hash: "keepMf"})).To(Succeed()) Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "mf", ItemID: "no-such-mediafile", ImageType: model.ImageTypePrimary, Hash: "danglingMf"})).To(Succeed()) - purged, err := repo.PurgeDanglingItemArtwork() + purged, err := repo.PurgeDanglingItems() Expect(err).ToNot(HaveOccurred()) Expect(purged).To(Equal(int64(5))) @@ -212,9 +212,9 @@ var _ = Describe("ArtworkRepository", func() { Expect(unresolved).To(BeFalse()) }) - It("deletes all rows for an item", func() { + It("deletes all rows for a single item", func() { Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "pl", ItemID: "p1", ImageType: model.ImageTypePrimary, Hash: "h1"})).To(Succeed()) - Expect(repo.DeleteForItem(model.KindPlaylistArtwork, "p1")).To(Succeed()) + Expect(repo.DeleteForItems(model.KindPlaylistArtwork, []string{"p1"})).To(Succeed()) _, err := repo.GetItemArtwork(model.KindPlaylistArtwork, "p1", model.ImageTypePrimary) Expect(err).To(MatchError(model.ErrNotFound)) }) diff --git a/tests/mock_artwork_repo.go b/tests/mock_artwork_repo.go index 4c04553de..bec589314 100644 --- a/tests/mock_artwork_repo.go +++ b/tests/mock_artwork_repo.go @@ -16,7 +16,7 @@ type MockArtworkRepo struct { Data map[string]model.Artwork ItemData map[string]model.ItemArtwork // keyed by iaKey(kind, id, imageType) Err error - // ExistingIDs, keyed by item_kind, backs PurgeDanglingItemArtwork; nil map keeps everything. + // ExistingIDs, keyed by item_kind, backs PurgeDanglingItems; nil map keeps everything. ExistingIDs map[string]map[string]bool } @@ -63,7 +63,7 @@ func (m *MockArtworkRepo) GetMimeByHash() (map[string]string, error) { return mimes, nil } -func (m *MockArtworkRepo) PurgeDanglingItemArtwork() (int64, error) { +func (m *MockArtworkRepo) PurgeDanglingItems() (int64, error) { m.mu.Lock() defer m.mu.Unlock() if m.Err != nil { @@ -84,7 +84,7 @@ func (m *MockArtworkRepo) PurgeDanglingItemArtwork() (int64, error) { return purged, nil } -func (m *MockArtworkRepo) DeleteOrphans(createdBefore time.Time) (int64, error) { +func (m *MockArtworkRepo) PurgeOrphans(createdBefore time.Time) (int64, error) { m.mu.Lock() defer m.mu.Unlock() if m.Err != nil { @@ -139,18 +139,6 @@ func (m *MockArtworkRepo) PutItemArtwork(ia *model.ItemArtwork) error { return nil } -func (m *MockArtworkRepo) DeleteForItem(kind model.Kind, id string) error { - m.mu.Lock() - defer m.mu.Unlock() - if m.Err != nil { - return m.Err - } - maps.DeleteFunc(m.ItemData, func(_ string, ia model.ItemArtwork) bool { - return ia.ItemKind == kind.Prefix() && ia.ItemID == id - }) - return nil -} - func (m *MockArtworkRepo) DeleteForItems(kind model.Kind, ids []string) error { m.mu.Lock() defer m.mu.Unlock()