diff --git a/cmd/artwork.go b/cmd/artwork.go index 44606e165..afea5ebf6 100644 --- a/cmd/artwork.go +++ b/cmd/artwork.go @@ -132,11 +132,11 @@ type statusReport struct { queue []model.ArtworkQueueStat sources []sourceCount absent []absentCount + inputs []artwork.FingerprintInput stored string current string } -// backfillQueued counts what a backfill still has to drain, the cost a config change is charging now. func (r statusReport) backfillQueued() int64 { var n int64 for _, s := range r.queue { @@ -179,7 +179,7 @@ func collectStatus(ctx context.Context, ds model.DataStore) (statusReport, error rep.absent = append(rep.absent, absentCount{kind: k, ArtworkAbsentStat: stat}) } - rep.current = artwork.ConfigFingerprint() + rep.current, rep.inputs = artwork.ConfigFingerprint(), artwork.FingerprintInputs() if rep.stored, err = ds.Property(ctx).DefaultGet(consts.ArtConfFingerprintPropertyKey, ""); err != nil { return rep, fmt.Errorf("reading the stored artwork fingerprint: %w", err) } @@ -215,25 +215,34 @@ func formatStatus(rep statusReport) string { fmt.Fprintf(w, " (rechecked once the last attempt is older than %gh)\n", artwork.StaleAbsentAge.Hours()) fmt.Fprintln(w, "\nBackfill") + fmt.Fprintf(w, " State:\t%s\n", backfillState(rep)) fmt.Fprintf(w, " Stored fingerprint:\t%s\n", cmp.Or(rep.stored, "(none)")) fmt.Fprintf(w, " Current fingerprint:\t%s\n", rep.current) - fmt.Fprintf(w, " State:\t%s\n", backfillState(rep)) + if len(rep.inputs) > 0 { + fmt.Fprintln(w, " Fingerprint inputs (changing any of these re-resolves the whole library):") + for _, in := range rep.inputs { + fmt.Fprintf(w, " %s:\t%s\n", in.Name, in.Value) + } + } w.Flush() return sb.String() } -// backfillState turns "why is my server re-resolving everything?" into a line: a stored fingerprint -// that differs is a pending re-resolve of the whole library, and backfill rows are one already running. +// backfillState leads with the queued backlog: by the time anyone runs this, backfill has usually +// already stored the new fingerprint, and "up to date" would bury the flood it is still working through. func backfillState(rep statusReport) string { - state := "up to date" + changed := "fingerprint up to date" if rep.stored != rep.current { - state = "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup" + changed = "fingerprint changed" } if n := rep.backfillQueued(); n > 0 { - state += fmt.Sprintf("; %d items still queued at backfill priority", n) + return fmt.Sprintf("backfill running: %d items queued (%s)", n, changed) } - return state + if rep.stored != rep.current { + return "fingerprint changed — every artist, album, playlist and radio will be re-enqueued on the next startup" + } + return "up to date" } func kindName(prefix string) string { @@ -569,7 +578,7 @@ func formatExplain(rep explainReport) string { if rep.queued == nil { fmt.Fprintln(w, " (not queued)") } else { - fmt.Fprintf(w, " Priority:\t%d\n", rep.queued.Priority) + fmt.Fprintf(w, " Priority:\t%s (%d)\n", priorityName(rep.queued.Priority), rep.queued.Priority) fmt.Fprintf(w, " Attempts:\t%d\n", rep.queued.Attempts) fmt.Fprintf(w, " Retry at:\t%s\n", formatTime(rep.queued.RetryAt)) } diff --git a/cmd/artwork_test.go b/cmd/artwork_test.go index 6e090026e..1a3d5b880 100644 --- a/cmd/artwork_test.go +++ b/cmd/artwork_test.go @@ -151,7 +151,7 @@ var _ = Describe("formatExplain", func() { Expect(out).To(ContainSubstring("abc123")) Expect(out).To(ContainSubstring("/music/cover.jpg")) Expect(out).To(ContainSubstring("2026-08-13T10:00:00Z")) - Expect(out).To(ContainSubstring("50")) + Expect(out).To(ContainSubstring("scan (50)"), "a bare 50 makes the operator look the priority up") Expect(out).To(ContainSubstring("resolved from folder")) }) @@ -502,6 +502,12 @@ var _ = Describe("collectStatus", func() { Expect(rep.stored).To(BeEmpty()) }) + It("collects the config inputs the fingerprint is computed from", func() { + rep, err := collectStatus(ctx, ds) + Expect(err).ToNot(HaveOccurred()) + Expect(rep.inputs).To(Equal(artwork.FingerprintInputs())) + }) + It("queues nothing", func() { _, err := collectStatus(ctx, ds) Expect(err).ToNot(HaveOccurred()) @@ -526,38 +532,74 @@ var _ = Describe("formatStatus", func() { absent: []absentCount{ {kind: model.KindArtistArtwork, ArtworkAbsentStat: model.ArtworkAbsentStat{Total: 2, Stale: 1}}, }, + inputs: []artwork.FingerprintInput{{Name: "Agents", Value: "deezer,lastfm"}}, stored: "abc123", current: "abc123", } }) + // block isolates one section, so an assertion cannot be satisfied by a coincidence elsewhere. + block := func(out, header string) string { + GinkgoHelper() + _, after, found := strings.Cut(out, header+"\n") + Expect(found).To(BeTrue(), "the %q block must be printed", header) + body, _, _ := strings.Cut(after, "\n\n") + return body + } + It("prints every block", func() { out := formatStatus(rep) - for _, block := range []string{"Queue", "Sources", "Absent", "Backfill"} { - Expect(out).To(ContainSubstring(block)) + for _, header := range []string{"Queue", "Sources", "Absent", "Backfill"} { + Expect(out).To(ContainSubstring(header)) } - Expect(out).To(ContainSubstring("backfill")) - Expect(out).To(ContainSubstring("external:deezer")) Expect(out).To(ContainSubstring("abc123")) }) - It("names the empty source as absent", func() { - Expect(formatStatus(rep)).To(ContainSubstring("absent")) + It("names the kind and the priority of every queued row", func() { + queue := block(formatStatus(rep), "Queue") + Expect(queue).To(MatchRegexp(`artist\s+backfill\s+2`)) + Expect(queue).To(MatchRegexp(`album\s+scan\s+1`)) + }) + + It("totals the queue", func() { + Expect(block(formatStatus(rep), "Queue")).To(MatchRegexp(`TOTAL\s+3`)) + }) + + It("counts each source, naming the empty one absent", func() { + sources := block(formatStatus(rep), "Sources") + Expect(sources).To(MatchRegexp(`artist\s+external:deezer\s+5`)) + Expect(sources).To(MatchRegexp(`artist\s+absent\s+2`)) + }) + + It("prints the absent total and how many are due for recheck", func() { + absent := block(formatStatus(rep), "Absent (resolved, no image found)") + Expect(absent).To(MatchRegexp(`artist\s+2\s+1`)) }) It("states the recheck window the absent counts are bucketed against", func() { Expect(formatStatus(rep)).To(ContainSubstring("24h")) }) - It("reports a matching fingerprint as up to date, with the backfill still draining", func() { - out := formatStatus(rep) - Expect(out).To(ContainSubstring("up to date")) - Expect(out).To(ContainSubstring("2 items still queued at backfill priority"), - "a drained-down backfill is the signal that a config change flooded the queue") + It("leads with the queued backlog, which is the finding, not with the fingerprint verdict", func() { + out := block(formatStatus(rep), "Backfill") + Expect(out).To(MatchRegexp(`State:\s+backfill running: 2 items queued`), + "an operator scanning for trouble must not read 'up to date' while 2 items churn") + Expect(out).To(ContainSubstring("fingerprint up to date")) + }) + + It("reports up to date only once the backfill has drained", func() { + rep.queue = []model.ArtworkQueueStat{{ItemKind: "al", Priority: model.ArtworkPriorityScan, Count: 1}} + + Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`State:\s+up to date`)) + }) + + It("echoes the config inputs a fingerprint change would have come from", func() { + Expect(block(formatStatus(rep), "Backfill")).To(MatchRegexp(`Agents:\s+deezer,lastfm`)) }) It("reports a changed fingerprint as a pending re-resolve of everything", func() { rep.stored = "older" + rep.queue, rep.queueTotal = nil, 0 out := formatStatus(rep) Expect(out).To(ContainSubstring("fingerprint changed")) diff --git a/core/artwork/housekeeping.go b/core/artwork/housekeeping.go index c58deff28..9cfad32bc 100644 --- a/core/artwork/housekeeping.go +++ b/core/artwork/housekeeping.go @@ -6,6 +6,8 @@ import ( "encoding/hex" "fmt" "slices" + "strconv" + "strings" "time" "github.com/navidrome/navidrome/conf" @@ -33,12 +35,30 @@ func hasRecheckPath(prefix string) bool { // artworkEpoch invalidates all resolution state when bumped; bump it whenever resolution semantics change. const artworkEpoch = 1 +// FingerprintInput is one config value the fingerprint covers, named after the setting it came from. +type FingerprintInput struct { + Name string + Value string +} + +// FingerprintInputs is the single listing of what ConfigFingerprint hashes, so a caller can report +// which setting a fingerprint change came from without keeping a second copy of the list. +func FingerprintInputs() []FingerprintInput { + return []FingerprintInput{ + {"CoverArtPriority", conf.Server.CoverArtPriority}, + {"ArtistArtPriority", conf.Server.ArtistArtPriority}, + {"ArtistImageFolder", conf.Server.ArtistImageFolder}, + {"Agents", conf.Server.Agents}, + {"EnableExternalServices", strconv.FormatBool(conf.Server.EnableExternalServices)}, + {"EnableM3UExternalAlbumArt", strconv.FormatBool(conf.Server.EnableM3UExternalAlbumArt)}, + } +} + // ConfigFingerprint covers the inputs that affect resolution outcomes; a change invalidates stored state. // Exported so the CLI reports the value backfill compares instead of computing one that can drift. func ConfigFingerprint() string { - raw := fmt.Sprintf("%s|%s|%s|%s|%t|%t|%d", - conf.Server.CoverArtPriority, conf.Server.ArtistArtPriority, conf.Server.ArtistImageFolder, - conf.Server.Agents, conf.Server.EnableExternalServices, conf.Server.EnableM3UExternalAlbumArt, artworkEpoch) + values := slice.Map(FingerprintInputs(), func(i FingerprintInput) string { return i.Value }) + raw := fmt.Sprintf("%s|%d", strings.Join(values, "|"), artworkEpoch) sum := md5.Sum([]byte(raw)) //nolint:gosec // fingerprint, not security-sensitive return hex.EncodeToString(sum[:]) } diff --git a/core/artwork/housekeeping_test.go b/core/artwork/housekeeping_test.go index e875eb9ca..45fbdb4f9 100644 --- a/core/artwork/housekeeping_test.go +++ b/core/artwork/housekeeping_test.go @@ -113,6 +113,28 @@ var _ = Describe("Housekeeping", func() { Expect(ConfigFingerprint()).NotTo(Equal(f1)) }) + // Pinned: a changed formula re-resolves every library on upgrade, flooding external providers. + It("hashes a given config to a stable value", func() { + conf.Server.CoverArtPriority = "cover.*, embedded" + conf.Server.ArtistArtPriority = "artist.*, external" + conf.Server.ArtistImageFolder = "" + conf.Server.Agents = "lastfm,spotify" + conf.Server.EnableExternalServices = true + conf.Server.EnableM3UExternalAlbumArt = false + + Expect(ConfigFingerprint()).To(Equal("7e537a22febc07d3d5ca40546e88da54")) + }) + + It("reports the config inputs it hashes, so a change can be traced to a setting", func() { + conf.Server.Agents = "lastfm,spotify" + conf.Server.CoverArtPriority = "cover.*, embedded" + + Expect(FingerprintInputs()).To(ContainElements( + FingerprintInput{Name: "Agents", Value: "lastfm,spotify"}, + FingerprintInput{Name: "CoverArtPriority", Value: "cover.*, embedded"}, + )) + }) + It("does not change when the server version changes", func() { original := consts.Version DeferCleanup(func() { consts.Version = original }) diff --git a/persistence/artwork_queue_repository_test.go b/persistence/artwork_queue_repository_test.go index e1ec0f200..cbea6a078 100644 --- a/persistence/artwork_queue_repository_test.go +++ b/persistence/artwork_queue_repository_test.go @@ -415,6 +415,23 @@ var _ = Describe("ArtworkQueueRepository", func() { To(Equal(model.ArtworkAbsentStat{Total: 2, Stale: 1})) }) + // status prints both under one "absent" heading, so a writer that stops clearing hash and + // source together would make the two numbers disagree with no other warning. + It("agrees with the source-based absent count", func() { + awRepo := NewArtworkRepository(context.Background(), GetDBXBuilder()) + for _, ia := range []model.ItemArtwork{ + {ItemKind: "ar", ItemID: "absent1", ImageType: model.ImageTypePrimary, Hash: "", Source: "", AttemptedAt: time.Now()}, + {ItemKind: "ar", ItemID: "absent2", ImageType: model.ImageTypePrimary, Hash: "", Source: "", AttemptedAt: time.Now()}, + {ItemKind: "ar", ItemID: "found1", ImageType: model.ImageTypePrimary, Hash: "hX", Source: "folder", AttemptedAt: time.Now()}, + } { + Expect(awRepo.PutItemArtwork(&ia)).To(Succeed()) + } + + stat, err := repo.CountAbsent(model.KindArtistArtwork, time.Now()) + Expect(err).ToNot(HaveOccurred()) + Expect(repo.CountBySource(model.KindArtistArtwork, []string{""})).To(Equal(stat.Total)) + }) + It("reports a kind with no absent state as zero, not as an error", func() { Expect(repo.CountAbsent(model.KindRadioArtwork, time.Now())).To(Equal(model.ArtworkAbsentStat{})) })