nik/navidrome
1
0
Fork 0
mirror of https://github.com/navidrome/navidrome.git synced 2026-10-18 07:30:29 +00:00
Code Issues Packages Projects Releases Wiki Activity

refactor(artwork): split the repository's deletes from its purges by name

Browse Source 
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.
This commit is contained in:
Deluan 2026-08-01 21:10:01 -04:00
parent cdb9f44735
commit d704587eb5
6 changed files with 18 additions and 34 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
core/artwork/housekeeping.go
View File

@ -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}

4
core/artwork/prune.go
View File

@ -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
}

10
model/artwork.go
View File

@ -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 {

8
persistence/artwork_repository.go
View File

@ -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 {

10
persistence/artwork_repository_test.go
View File

@ -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))
})

18
tests/mock_artwork_repo.go
View File

@ -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()