feat(artwork): enqueue artwork resolution from scan and CRUD paths

This commit is contained in:
Deluan 2026-07-22 10:21:48 -04:00
parent 57c64e386a
commit 25a05fd017
12 changed files with 211 additions and 1 deletions

View File

@ -13,6 +13,15 @@ const pruneMinAge = time.Hour
func Prune(ctx context.Context, ds model.DataStore, store *ImageStore) error {
repo := ds.Artwork(ctx)
purged, err := repo.PurgeDanglingItemArtwork()
if err != nil {
return err
}
if purged > 0 {
log.Info(ctx, "Prune: purged dangling item artwork state", "count", purged)
}
// One grace cutoff for both the DB orphan check and the file sweep: files younger
// than the window may belong to acquisitions whose rows aren't committed yet.
cutoff := time.Now().Add(-pruneMinAge)

View File

@ -39,6 +39,25 @@ var _ = Describe("Prune", func() {
awRepo.Data[h] = a
}
It("purges dangling item_artwork state for gone entities, summed across kinds", func() {
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: "gone-album", ImageType: model.ImageTypePrimary})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "gone-artist", ImageType: model.ImageTypePrimary})).To(Succeed())
Expect(awRepo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "live-artist", ImageType: model.ImageTypePrimary})).To(Succeed())
awRepo.ExistingIDs = map[string]map[string]bool{
"al": {},
"ar": {"live-artist": true},
}
Expect(Prune(context.Background(), ds, store)).To(Succeed())
_, err := awRepo.GetItemArtwork("al", "gone-album", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound))
_, err = awRepo.GetItemArtwork("ar", "gone-artist", model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound))
_, err = awRepo.GetItemArtwork("ar", "live-artist", model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
})
It("deletes orphan rows and their store files, keeps referenced ones", func() {
data := []byte("orphan-bytes")
h, _ := HashImage(bytes.NewReader(data))

View File

@ -77,6 +77,8 @@ type ArtworkRepository interface {
GetInfoForItems(kind string, ids []string) (map[string]ItemArtworkInfo, error)
// GetAllMimes returns hash -> current mime for every stored artwork, for sweep retention checks.
GetAllMimes() (map[string]string, error)
// PurgeDanglingItemArtwork removes state rows whose entity no longer exists.
PurgeDanglingItemArtwork() (int64, error)
}
type ArtworkQueueRepository interface {

View File

@ -114,6 +114,30 @@ func (r *artworkRepository) DeleteOrphans(createdBefore time.Time, hashes []stri
return nil
}
// danglingItemArtworkKinds maps item_kind prefixes to the table that owns the entity.
var danglingItemArtworkKinds = map[string]string{
"al": "album",
"ar": "artist",
"pl": "playlist",
"ra": "radio",
}
func (r *artworkRepository) PurgeDanglingItemArtwork() (int64, error) {
var total int64
for kind, table := range danglingItemArtworkKinds {
del := Delete(itemArtworkTable).Where(And{
Eq{"item_kind": kind},
Expr("item_id NOT IN (SELECT id FROM " + table + ")"),
})
c, err := r.items.executeSQL(del)
if err != nil {
return total, err
}
total += c
}
return total, nil
}
func (r *artworkRepository) GetItemArtwork(kind, id, imageType string) (*model.ItemArtwork, error) {
sel := Select("*").From(itemArtworkTable).
Where(Eq{"item_kind": kind, "item_id": id, "image_type": imageType})

View File

@ -136,6 +136,42 @@ var _ = Describe("ArtworkRepository", func() {
})
})
Context("dangling state cleanup", func() {
It("purges item_artwork rows per kind whose entity no longer exists, summing counts", func() {
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: albumSgtPeppers.ID, ImageType: model.ImageTypePrimary, Hash: "keepAl"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "al", ItemID: "no-such-album", ImageType: model.ImageTypePrimary, Hash: "danglingAl"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: artistKraftwerk.ID, ImageType: model.ImageTypePrimary, Hash: "keepAr"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ar", ItemID: "no-such-artist", ImageType: model.ImageTypePrimary, Hash: "danglingAr"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "pl", ItemID: plsBest.ID, ImageType: model.ImageTypePrimary, Hash: "keepPl"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "pl", ItemID: "no-such-playlist", ImageType: model.ImageTypePrimary, Hash: "danglingPl"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ra", ItemID: radioWithHomePage.ID, ImageType: model.ImageTypePrimary, Hash: "keepRa"})).To(Succeed())
Expect(repo.PutItemArtwork(&model.ItemArtwork{ItemKind: "ra", ItemID: "no-such-radio", ImageType: model.ImageTypePrimary, Hash: "danglingRa"})).To(Succeed())
purged, err := repo.PurgeDanglingItemArtwork()
Expect(err).ToNot(HaveOccurred())
Expect(purged).To(Equal(int64(4)))
for _, kept := range []model.ItemArtwork{
{ItemKind: "al", ItemID: albumSgtPeppers.ID},
{ItemKind: "ar", ItemID: artistKraftwerk.ID},
{ItemKind: "pl", ItemID: plsBest.ID},
{ItemKind: "ra", ItemID: radioWithHomePage.ID},
} {
_, err := repo.GetItemArtwork(kept.ItemKind, kept.ItemID, model.ImageTypePrimary)
Expect(err).ToNot(HaveOccurred())
}
for _, gone := range []model.ItemArtwork{
{ItemKind: "al", ItemID: "no-such-album"},
{ItemKind: "ar", ItemID: "no-such-artist"},
{ItemKind: "pl", ItemID: "no-such-playlist"},
{ItemKind: "ra", ItemID: "no-such-radio"},
} {
_, err := repo.GetItemArtwork(gone.ItemKind, gone.ItemID, model.ImageTypePrimary)
Expect(err).To(MatchError(model.ErrNotFound))
}
})
})
Context("item state", func() {
It("upserts and reads state, including per-item provenance", func() {
ia := &model.ItemArtwork{ItemKind: "al", ItemID: "al1", ImageType: model.ImageTypePrimary,

View File

@ -7,6 +7,7 @@ import (
. "github.com/Masterminds/squirrel"
"github.com/deluan/rest"
"github.com/navidrome/navidrome/log"
"github.com/navidrome/navidrome/model"
"github.com/navidrome/navidrome/model/id"
"github.com/pocketbase/dbx"
@ -72,7 +73,16 @@ func (r *radioRepository) Put(radio *model.Radio, colsToUpdate ...string) error
colsToUpdate = append(colsToUpdate, "UpdatedAt")
}
_, err := r.put(radio.ID, radio, colsToUpdate...)
return err
if err != nil {
return err
}
// Enqueue artwork resolution for the created/updated radio. Never fails the save.
item := model.ArtworkQueueItem{ItemKind: "ra", ItemID: radio.ID, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityScan}
if err := NewArtworkQueueRepository(r.ctx, r.db).Enqueue(item); err != nil {
log.Warn(r.ctx, "could not enqueue radio artwork", "id", radio.ID, err)
}
return nil
}
func (r *radioRepository) Count(options ...rest.QueryOptions) (int64, error) {

View File

@ -107,6 +107,27 @@ var _ = Describe("RadioRepository", func() {
Expect(err).To(BeNil())
Expect(all[2].StreamUrl).To(Equal("https://example.com:4533/app"))
})
It("enqueues artwork resolution for the saved radio", func() {
err := repo.Put(&model.Radio{
Name: "Artwork radio",
StreamUrl: "https://example.com:4533/artwork",
})
Expect(err).To(BeNil())
all, err := repo.GetAll()
Expect(err).To(BeNil())
created := all[len(all)-1]
queueRepo := NewArtworkQueueRepository(context.Background(), GetDBXBuilder())
queued, err := queueRepo.DequeueBatch(1000)
Expect(err).To(BeNil())
Expect(queued).To(ContainElement(SatisfyAll(
HaveField("ItemKind", "ra"),
HaveField("ItemID", created.ID),
HaveField("Priority", model.ArtworkPriorityScan),
)))
})
})
})

View File

@ -332,6 +332,8 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error)
// Collect artwork IDs to pre-cache after the transaction commits
var artworkIDs []model.ArtworkID
// Collect artwork queue items for changed albums/artists, enqueued in the same transaction
var queueItems []model.ArtworkQueueItem
err := p.ds.WithTx(func(tx model.DataStore) error {
// Instantiate all repositories just once per folder
@ -372,6 +374,10 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error)
}
if entry.artists[i].Name != consts.UnknownArtist && entry.artists[i].Name != consts.VariousArtists {
artworkIDs = append(artworkIDs, entry.artists[i].CoverArtID())
queueItems = append(queueItems, model.ArtworkQueueItem{
ItemKind: "ar", ItemID: entry.artists[i].ID, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityScan,
})
}
}
@ -384,6 +390,10 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error)
}
if entry.albums[i].Name != consts.UnknownAlbum {
artworkIDs = append(artworkIDs, entry.albums[i].CoverArtID())
queueItems = append(queueItems, model.ArtworkQueueItem{
ItemKind: "al", ItemID: entry.albums[i].ID, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityScan,
})
}
}
@ -415,6 +425,13 @@ func (p *phaseFolders) persistChanges(entry *folderEntry) (*folderEntry, error)
return err
}
}
// Enqueue artwork resolution for changed albums/artists. Never fails the scan.
if len(queueItems) > 0 {
if err := tx.ArtworkQueue(p.ctx).Enqueue(queueItems...); err != nil {
log.Warn(p.ctx, "Scanner: could not enqueue artwork resolution", "folder", entry.path, err)
}
}
return nil
}, "scanner: persist changes")
if err != nil {

View File

@ -149,6 +149,11 @@ func (p *phasePlaylists) processPlaylistsInFolder(folder *model.Folder) (*model.
log.Debug("Scanner: Imported playlist", "name", pls.Name, "lastUpdated", pls.UpdatedAt, "path", pls.Path, "numTracks", len(pls.Tracks), "elapsed", time.Since(started))
}
p.cw.PreCache(pls.CoverArtID())
item := model.ArtworkQueueItem{ItemKind: "pl", ItemID: pls.ID, ImageType: model.ImageTypePrimary,
Priority: model.ArtworkPriorityScan}
if err := p.ds.ArtworkQueue(p.ctx).Enqueue(item); err != nil {
log.Warn(p.ctx, "Scanner: could not enqueue playlist artwork", "id", pls.ID, err)
}
p.refreshed.Add(1)
}
return folder, nil

View File

@ -193,6 +193,29 @@ var _ = Describe("phasePlaylists", func() {
Expect(phase.refreshed.Load()).To(Equal(uint32(2)))
})
It("enqueues artwork resolution for the imported playlist", func() {
libPath := GinkgoT().TempDir()
folder := &model.Folder{LibraryPath: libPath, Path: "path/to", Name: "folder"}
_ = os.MkdirAll(folder.AbsolutePath(), 0755)
file1 := filepath.Join(folder.AbsolutePath(), "playlist1.m3u")
_ = os.WriteFile(file1, []byte{}, 0600)
pls.On("ImportFromFolder", mock.Anything, folder, "playlist1.m3u").
Return(&model.Playlist{ID: "pl1"}, nil)
_, err := phase.processPlaylistsInFolder(folder)
Expect(err).ToNot(HaveOccurred())
queued, err := ds.ArtworkQueue(ctx).DequeueBatch(10)
Expect(err).ToNot(HaveOccurred())
Expect(queued).To(ContainElement(SatisfyAll(
HaveField("ItemKind", "pl"),
HaveField("ItemID", "pl1"),
HaveField("Priority", model.ArtworkPriorityScan),
)))
})
It("reports an error if there is an error reading files", func() {
tests.SkipOnWindows("relies on Unix /etc filesystem")
progress := make(chan *ProgressInfo)

View File

@ -152,6 +152,29 @@ var _ = Describe("Scanner", Ordered, func() {
HaveField("SongCount", Equal(4)),
))
})
It("should enqueue artwork resolution for the scanned albums and artists", func() {
Expect(runScanner(ctx, true)).To(Succeed())
albums, _ := ds.Album(ctx).GetAll()
artists, _ := ds.Artist(ctx).GetAll(model.QueryOptions{Filters: squirrel.NotEq{"name": consts.UnknownArtist}})
queued, err := ds.ArtworkQueue(ctx).DequeueBatch(1000)
Expect(err).ToNot(HaveOccurred())
for _, al := range albums {
Expect(queued).To(ContainElement(SatisfyAll(
HaveField("ItemKind", "al"),
HaveField("ItemID", al.ID),
HaveField("Priority", model.ArtworkPriorityScan),
)))
}
for _, ar := range artists {
Expect(queued).To(ContainElement(SatisfyAll(
HaveField("ItemKind", "ar"),
HaveField("ItemID", ar.ID),
HaveField("Priority", model.ArtworkPriorityScan),
)))
}
})
})
When("a file was changed", func() {
It("should update the media_file", func() {

View File

@ -12,6 +12,8 @@ type MockArtworkRepo struct {
ItemData map[string]model.ItemArtwork // keyed by iaKey(kind, id, imageType)
OrphanHashes []string
Err error
// ExistingIDs, keyed by item_kind, backs PurgeDanglingItemArtwork; nil map keeps everything.
ExistingIDs map[string]map[string]bool
}
func CreateMockArtworkRepo() *MockArtworkRepo {
@ -68,6 +70,25 @@ func (m *MockArtworkRepo) GetAllMimes() (map[string]string, error) {
return mimes, nil
}
func (m *MockArtworkRepo) PurgeDanglingItemArtwork() (int64, error) {
if m.Err != nil {
return 0, m.Err
}
var purged int64
for k, ia := range m.ItemData {
// A nil per-kind map means that kind isn't tracked by the test, so keep it.
existing := m.ExistingIDs[ia.ItemKind]
if existing == nil {
continue
}
if !existing[ia.ItemID] {
delete(m.ItemData, k)
purged++
}
}
return purged, nil
}
func (m *MockArtworkRepo) DeleteOrphans(createdBefore time.Time, hashes []string) error {
if m.Err != nil {
return m.Err