From fd5aca76826962e3b94787bf89009930baa398e4 Mon Sep 17 00:00:00 2001 From: Deluan Date: Fri, 14 Aug 2026 13:47:09 -0400 Subject: [PATCH] feat(cli): add artwork reprocess Bulk re-enqueues artwork by kind and/or by the source an item currently resolves from, previewing the matched count and confirming before queueing. The preview counts with CountBySource (rows matched) and reports separately what EnqueueBySource inserted: its DO NOTHING conflict policy leaves an already-queued row untouched, so the two numbers differ and the output must not claim the skipped rows were re-queued. An unknown --source is rejected against the sources present in item_artwork, rather than silently matching nothing and printing a reassuring 0. --- cmd/artwork.go | 197 +++++++++++++++++++ cmd/artwork_test.go | 164 +++++++++++++++ model/artwork.go | 2 + persistence/artwork_queue_repository.go | 11 ++ persistence/artwork_queue_repository_test.go | 6 + tests/mock_artwork_queue_repo.go | 15 ++ 6 files changed, 395 insertions(+) diff --git a/cmd/artwork.go b/cmd/artwork.go index 507bf8b42..f69f02ddc 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -7,6 +7,7 @@ import ( "fmt" "io" "os" + "slices" "strings" "text/tabwriter" "time" @@ -16,6 +17,7 @@ import ( "github.com/navidrome/navidrome/db" "github.com/navidrome/navidrome/log" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" "github.com/spf13/cobra" ) @@ -26,11 +28,28 @@ var artworkKinds = []model.Kind{ var explainLive bool +var ( + reprocessKinds []string + reprocessSources []string + reprocessAll bool + reprocessDryRun bool + reprocessYes bool +) + func init() { artworkExplainCmd.Flags().BoolVar(&explainLive, "live", false, "perform real external lookups instead of reporting what would be tried") + artworkReprocessCmd.Flags().StringSliceVar(&reprocessKinds, "kind", nil, + "kinds to reprocess (ar, al, pl, ra); repeatable") + artworkReprocessCmd.Flags().StringSliceVar(&reprocessSources, "source", nil, + "only items currently resolved from these sources (e.g. folder, external:deezer, absent)") + artworkReprocessCmd.Flags().BoolVar(&reprocessAll, "all", false, "reprocess every kind") + artworkReprocessCmd.Flags().BoolVar(&reprocessDryRun, "dry-run", false, + "report what would be queued and exit without queueing") + artworkReprocessCmd.Flags().BoolVarP(&reprocessYes, "yes", "y", false, "skip the confirmation prompt") artworkCmd.AddCommand(artworkExplainCmd) artworkCmd.AddCommand(artworkRefreshCmd) + artworkCmd.AddCommand(artworkReprocessCmd) rootCmd.AddCommand(artworkCmd) } @@ -65,6 +84,184 @@ var artworkRefreshCmd = &cobra.Command{ }, } +var artworkReprocessCmd = &cobra.Command{ + Use: "reprocess", + Short: "Re-enqueue artwork in bulk, by kind and/or by the source it currently resolves from", + Args: cobra.NoArgs, + Run: func(cmd *cobra.Command, args []string) { + runReprocess(cmd.Context()) + }, +} + +func runReprocess(ctx context.Context) { + // A source filter alone is a complete selection, so it targets every kind. + all := reprocessAll || (len(reprocessKinds) == 0 && len(reprocessSources) > 0) + kinds, err := selectedKinds(reprocessKinds, all) + if err != nil { + log.Fatal(ctx, err) + } + + defer db.Init(ctx)() + ds, ctx := getAdminContext(ctx) + + confirm := promptConfirm(os.Stdin) + if reprocessYes { + confirm = func(io.Writer, int64) bool { return true } + } + if err := reprocessArtwork(ctx, ds, kinds, repositorySources(reprocessSources), + reprocessDryRun, confirm, os.Stdout); err != nil { + log.Fatal(ctx, err) + } +} + +func selectedKinds(kinds []string, all bool) ([]model.Kind, error) { + if all { + return artworkKinds, nil + } + if len(kinds) == 0 { + return nil, fmt.Errorf("no selector given: pass --kind, --source or --all") + } + out := make([]model.Kind, 0, len(kinds)) + for _, k := range kinds { + kind, err := parseArtworkKind(k) + if err != nil { + return nil, err + } + // A repeated kind would be counted twice, overstating the cost the operator confirms. + if !slices.Contains(out, kind) { + out = append(out, kind) + } + } + return out, nil +} + +// absentSource is how the stored empty source — resolved, no image — is spelled on the CLI. +const absentSource = "absent" + +func repositorySources(sources []string) []string { + return slice.Map(sources, func(s string) string { + if s == absentSource { + return "" + } + return s + }) +} + +func displaySource(s string) string { return cmp.Or(s, absentSource) } + +// confirmFunc reports whether the operator accepted queueing total items. +type confirmFunc func(out io.Writer, total int64) bool + +func promptConfirm(in io.Reader) confirmFunc { + return func(out io.Writer, total int64) bool { + fmt.Fprintf(out, "\nThis will re-resolve %d items, requiring up to %d external lookups. Continue? [y/N] ", + total, total) + var answer string + if _, err := fmt.Fscanln(in, &answer); err != nil { + return false + } + answer = strings.ToLower(strings.TrimSpace(answer)) + return answer == "y" || answer == "yes" + } +} + +// validateSources rejects a source no selected item resolves from: silently matching nothing reads +// as "nothing to do" when it actually means the filter was wrong. +func validateSources(q model.ArtworkQueueRepository, kinds []model.Kind, sources []string) error { + if len(sources) == 0 { + return nil + } + var inUse []string + for _, k := range kinds { + found, err := q.SourcesInUse(k) + if err != nil { + return fmt.Errorf("listing the sources in use by %s artwork: %w", k, err) + } + for _, s := range found { + if !slices.Contains(inUse, s) { + inUse = append(inUse, s) + } + } + } + var unknown []string + for _, s := range sources { + if !slices.Contains(inUse, s) { + unknown = append(unknown, displaySource(s)) + } + } + if len(unknown) == 0 { + return nil + } + valid := slice.Map(inUse, displaySource) + slices.Sort(valid) + return fmt.Errorf("no selected artwork resolves from %s; sources in use: %s", + strings.Join(unknown, ", "), cmp.Or(strings.Join(valid, ", "), "(none)")) +} + +// reprocessArtwork previews from CountBySource — rows matched — then reports what EnqueueBySource +// actually inserted; the two differ because an already-queued row is left untouched. +func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kind, sources []string, + dryRun bool, confirm confirmFunc, out io.Writer) error { + q := ds.ArtworkQueue(ctx) + if err := validateSources(q, kinds, sources); err != nil { + return err + } + + matched := make([]int64, len(kinds)) + var total int64 + for i, k := range kinds { + n, err := q.CountBySource(k, sources) + if err != nil { + return fmt.Errorf("counting %s artwork: %w", k, err) + } + matched[i] = n + total += n + } + printReprocessPreview(out, kinds, matched, total, sources) + + switch { + case total == 0: + fmt.Fprintln(out, "\nNothing matches this selection; nothing was queued.") + return nil + case dryRun: + fmt.Fprintln(out, "\nDry run: nothing was queued.") + return nil + case !confirm(out, total): + fmt.Fprintln(out, "Aborted: nothing was queued.") + return nil + } + + var queued int64 + for i, k := range kinds { + if matched[i] == 0 { + continue + } + n, err := q.EnqueueBySource(k, sources, model.ArtworkPriorityRecheck) + if err != nil { + return fmt.Errorf("queueing %s artwork: %w", k, err) + } + queued += n + fmt.Fprintf(out, "%s: %d queued\n", k, n) + } + fmt.Fprintf(out, "Queued %d of %d matched items.\n", queued, total) + if skipped := total - queued; skipped > 0 { + fmt.Fprintf(out, "Already queued, left unchanged: %d (priority and retry backoff untouched).\n", skipped) + } + return nil +} + +func printReprocessPreview(out io.Writer, kinds []model.Kind, matched []int64, total int64, sources []string) { + w := tabwriter.NewWriter(out, 0, 4, 2, ' ', 0) + shown := slice.Map(sources, displaySource) + fmt.Fprintf(w, "Sources:\t%s\n\n", cmp.Or(strings.Join(shown, ", "), "(any)")) + fmt.Fprintln(w, "KIND\tMATCHED") + for i, k := range kinds { + fmt.Fprintf(w, "%s\t%d\n", k, matched[i]) + } + fmt.Fprintf(w, "TOTAL\t%d\n", total) + w.Flush() +} + func runRefresh(ctx context.Context, kind model.Kind, ids []string) { defer db.Init(ctx)() ds, ctx := getAdminContext(ctx) diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index bee2e4637..805c0071a 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -3,6 +3,7 @@ package cmd import ( "context" "errors" + "io" "strings" "time" @@ -192,6 +193,169 @@ var _ = Describe("artwork refresh command", func() { }) }) +var _ = Describe("artwork reprocess selection", func() { + It("errors when no selector is given", func() { + _, err := selectedKinds(nil, false) + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("--all")) + }) + + It("returns every kind for --all", func() { + ks, err := selectedKinds(nil, true) + Expect(err).ToNot(HaveOccurred()) + Expect(ks).To(ConsistOf(artworkKinds)) + }) + + It("returns only the named kinds", func() { + ks, err := selectedKinds([]string{"ar"}, false) + Expect(err).ToNot(HaveOccurred()) + Expect(ks).To(Equal([]model.Kind{model.KindArtistArtwork})) + }) + + It("rejects an unknown kind", func() { + _, err := selectedKinds([]string{"zz"}, false) + Expect(err).To(HaveOccurred()) + }) + + It("counts a repeated kind once", func() { + ks, err := selectedKinds([]string{"ar", "ar"}, false) + Expect(err).ToNot(HaveOccurred()) + Expect(ks).To(Equal([]model.Kind{model.KindArtistArtwork})) + }) +}) + +var _ = Describe("repositorySources", func() { + It("maps the user-facing absent name onto the stored empty source", func() { + Expect(repositorySources([]string{"absent", "folder"})).To(Equal([]string{"", "folder"})) + }) + + It("keeps an empty selection empty, meaning every source", func() { + Expect(repositorySources(nil)).To(BeEmpty()) + }) +}) + +var _ = Describe("promptConfirm", func() { + var out strings.Builder + + BeforeEach(func() { out.Reset() }) + + It("states the external cost and accepts an explicit yes", func() { + Expect(promptConfirm(strings.NewReader("y\n"))(&out, 42)).To(BeTrue()) + Expect(out.String()).To(ContainSubstring("re-resolve 42 items")) + Expect(out.String()).To(ContainSubstring("42 external lookups")) + }) + + It("defaults to no on anything else", func() { + Expect(promptConfirm(strings.NewReader("\n"))(&out, 1)).To(BeFalse()) + Expect(promptConfirm(strings.NewReader("nope\n"))(&out, 1)).To(BeFalse()) + Expect(promptConfirm(strings.NewReader(""))(&out, 1)).To(BeFalse()) + }) +}) + +var _ = Describe("reprocessArtwork", func() { + var ds *tests.MockDataStore + var art *tests.MockArtworkRepo + var queue *tests.MockArtworkQueueRepo + var out strings.Builder + ctx := context.Background() + kinds := []model.Kind{model.KindArtistArtwork, model.KindAlbumArtwork} + accept := func(io.Writer, int64) bool { return true } + decline := func(io.Writer, int64) bool { return false } + + put := func(kind model.Kind, id, source string) { + Expect(art.PutItemArtwork(&model.ItemArtwork{ItemKind: kind.Prefix(), ItemID: id, + ImageType: model.ImageTypePrimary, Hash: "h" + id, Source: source})).To(Succeed()) + } + + BeforeEach(func() { + ds = &tests.MockDataStore{} + art = ds.Artwork(ctx).(*tests.MockArtworkRepo) + queue = ds.ArtworkQueue(ctx).(*tests.MockArtworkQueueRepo) + out.Reset() + put(model.KindArtistArtwork, "ar-1", "external:deezer") + put(model.KindArtistArtwork, "ar-2", "") + put(model.KindAlbumArtwork, "al-1", "external:deezer") + put(model.KindAlbumArtwork, "al-2", "folder") + }) + + It("previews the per-kind breakdown and queues nothing on a dry run", func() { + Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, true, accept, &out)).To(Succeed()) + + Expect(out.String()).To(ContainSubstring("external:deezer")) + Expect(out.String()).To(ContainSubstring("artist")) + Expect(out.String()).To(ContainSubstring("album")) + Expect(out.String()).To(ContainSubstring("TOTAL")) + Expect(out.String()).To(ContainSubstring("Dry run")) + Expect(queue.Count()).To(BeZero()) + }) + + It("queues nothing when the operator declines", func() { + Expect(reprocessArtwork(ctx, ds, kinds, nil, false, decline, &out)).To(Succeed()) + + Expect(out.String()).To(ContainSubstring("Aborted")) + Expect(queue.Count()).To(BeZero()) + }) + + It("queues the matching items at recheck priority, leaving their artwork state alone", func() { + Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, false, accept, &out)).To(Succeed()) + + Expect(queue.Count()).To(Equal(int64(2))) + queued, err := queue.Get(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(queued.Priority).To(Equal(model.ArtworkPriorityRecheck)) + _, err = queue.Get(model.KindAlbumArtwork, "al-2", model.ImageTypePrimary) + Expect(err).To(MatchError(model.ErrNotFound), "a non-matching source must not be queued") + + stored, err := art.GetItemArtwork(model.KindAlbumArtwork, "al-1", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(stored.Hash).To(Equal("hal-1"), "bulk reprocessing must not blank the current artwork") + }) + + It("targets the absent state", func() { + Expect(reprocessArtwork(ctx, ds, kinds, []string{""}, false, accept, &out)).To(Succeed()) + + Expect(queue.Count()).To(Equal(int64(1))) + _, err := queue.Get(model.KindArtistArtwork, "ar-2", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + }) + + It("reports matched and queued separately when part of the set is already queued", func() { + Expect(queue.Enqueue(model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar-1", + ImageType: model.ImageTypePrimary, Priority: model.ArtworkPriorityBump})).To(Succeed()) + + Expect(reprocessArtwork(ctx, ds, kinds, []string{"external:deezer"}, false, accept, &out)).To(Succeed()) + + Expect(out.String()).To(ContainSubstring("Queued 1 of 2 matched items")) + Expect(out.String()).To(ContainSubstring("Already queued, left unchanged: 1")) + queued, err := queue.Get(model.KindArtistArtwork, "ar-1", model.ImageTypePrimary) + Expect(err).ToNot(HaveOccurred()) + Expect(queued.Priority).To(Equal(model.ArtworkPriorityBump), + "an already-queued row keeps its priority and backoff") + }) + + It("stops at a selection that matches nothing instead of prompting", func() { + Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, false, + func(io.Writer, int64) bool { + Fail("must not prompt when there is nothing to queue") + return true + }, &out)).To(Succeed()) + + Expect(out.String()).To(ContainSubstring("Nothing")) + Expect(queue.Count()).To(BeZero()) + }) + + It("rejects an unknown source and names the ones in use", func() { + err := reprocessArtwork(ctx, ds, kinds, []string{"externa:deezer"}, true, accept, &out) + + Expect(err).To(HaveOccurred()) + Expect(err.Error()).To(ContainSubstring("externa:deezer")) + Expect(err.Error()).To(ContainSubstring("external:deezer")) + Expect(err.Error()).To(ContainSubstring("folder")) + Expect(err.Error()).To(ContainSubstring("absent"), "the empty source prints under its user-facing name") + Expect(queue.Count()).To(BeZero()) + }) +}) + var _ = Describe("refreshItems", func() { var ds *tests.MockDataStore var queue *tests.MockArtworkQueueRepo diff --git a/model/artwork.go b/model/artwork.go index 67a5cf45d..196c26e77 100644 --- a/model/artwork.go +++ b/model/artwork.go @@ -135,6 +135,8 @@ type ArtworkQueueRepository interface { // CountBySource reports how many items of a kind currently resolve from the given sources. // An empty sources slice means every source; "" matches absent state. CountBySource(kind Kind, sources []string) (int64, error) + // SourcesInUse lists the distinct sources items of a kind currently resolve from, "" included. + SourcesInUse(kind Kind) ([]string, error) // EnqueueBySource inserts queue rows for items of a kind whose current source matches. // It does not clear existing artwork state: the current image stays until it is replaced. EnqueueBySource(kind Kind, sources []string, priority int) (int64, error) diff --git a/persistence/artwork_queue_repository.go b/persistence/artwork_queue_repository.go index 22385d448..9b2aa1114 100644 --- a/persistence/artwork_queue_repository.go +++ b/persistence/artwork_queue_repository.go @@ -10,6 +10,7 @@ import ( . "github.com/Masterminds/squirrel" "github.com/navidrome/navidrome/model" + "github.com/navidrome/navidrome/utils/slice" "github.com/pocketbase/dbx" ) @@ -122,6 +123,16 @@ func (r *artworkQueueRepository) CountBySource(kind model.Kind, sources []string return res.Count, err } +func (r *artworkQueueRepository) SourcesInUse(kind model.Kind) ([]string, error) { + var res []struct{ Source string } + err := r.queryAll(Select("distinct source").From(itemArtworkTable). + Where(Eq{"item_kind": kind.Prefix()}), &res) + if err != nil { + return nil, err + } + return slice.Map(res, func(s struct{ Source string }) string { return s.Source }), nil +} + // EnqueueBySource deliberately leaves item_artwork alone: clearing state in bulk would blank the // library's artwork until every item is resolved again. func (r *artworkQueueRepository) EnqueueBySource(kind model.Kind, sources []string, priority int) (int64, error) { diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index 8ac5407bb..5b556f2da 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -356,6 +356,12 @@ var _ = Describe("ArtworkQueueRepository", func() { Expect(repo.CountBySource(model.KindArtistArtwork, []string{""})).To(Equal(int64(1))) Expect(repo.CountBySource(model.KindArtistArtwork, nil)).To(Equal(int64(3))) }) + + It("lists the distinct sources in use by a kind", func() { + Expect(repo.SourcesInUse(model.KindArtistArtwork)).To(ConsistOf("", "external:deezer", "external:lastfm")) + Expect(repo.SourcesInUse(model.KindAlbumArtwork)).To(ConsistOf("external:deezer")) + Expect(repo.SourcesInUse(model.KindRadioArtwork)).To(BeEmpty()) + }) }) It("does not disturb an already-queued entity when enqueueing missing rows", func() { diff --git a/tests/mock_artwork_queue_repo.go b/tests/mock_artwork_queue_repo.go index 458db6b9f..5f26a0247 100644 --- a/tests/mock_artwork_queue_repo.go +++ b/tests/mock_artwork_queue_repo.go @@ -252,6 +252,21 @@ func (m *MockArtworkQueueRepo) CountBySource(kind model.Kind, sources []string) return int64(len(m.matchingSource(kind, sources))), nil } +func (m *MockArtworkQueueRepo) SourcesInUse(kind model.Kind) ([]string, error) { + m.mu.Lock() + defer m.mu.Unlock() + if m.Err != nil { + return nil, m.Err + } + var res []string + for _, ia := range m.matchingSource(kind, nil) { + if !slices.Contains(res, ia.Source) { + res = append(res, ia.Source) + } + } + return res, nil +} + func (m *MockArtworkQueueRepo) EnqueueBySource(kind model.Kind, sources []string, priority int) (int64, error) { m.mu.Lock() defer m.mu.Unlock()