diff --git a/core/artwork/agent_images.go b/core/artwork/agent_images.go index 1c5c5844d..35e3c666a 100644 --- a/core/artwork/agent_images.go +++ b/core/artwork/agent_images.go @@ -47,13 +47,13 @@ func fetchArtistImage(ctx context.Context, ag *agents.Agents, gate gateFunc, ar // Synthetic artists would otherwise get an unrelated agent result assigned to them. switch ar.ID { case consts.UnknownArtistID, consts.VariousArtistsID: - traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"}) + traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "synthetic artist"}) return nil, "", false } name := externalName(ar.Name) imageAgents := ag.ArtistImageAgents() if len(imageAgents) == 0 { - traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, + traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "no enabled agent provides artist images"}) return nil, "", false } @@ -85,7 +85,7 @@ func fetchAlbumImage(ctx context.Context, ag *agents.Agents, gate gateFunc, al m name, artist := externalName(al.Name), externalName(al.AlbumArtist) imageAgents := ag.AlbumImageAgents() if len(imageAgents) == 0 { - traceFrom(ctx).add(traceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, + traceFrom(ctx).add(TraceStep{Candidate: ExternalCandidate, Outcome: OutcomeSkipped, Detail: "no enabled agent provides album images"}) return nil, "", false } diff --git a/core/artwork/agent_images_test.go b/core/artwork/agent_images_test.go index 88e0b47b5..60a34352d 100644 --- a/core/artwork/agent_images_test.go +++ b/core/artwork/agent_images_test.go @@ -175,12 +175,12 @@ var _ = Describe("agent images", func() { It("records a skipped external candidate when no agent provides artist images", func() { ag := imageAgents() - t := &chainTrace{} + t := &ChainTrace{} r, _, extErr := fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: "ar1"}) Expect(r).To(BeNil()) Expect(extErr).To(BeFalse()) - Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "external", Outcome: OutcomeSkipped, + Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped, Detail: "no enabled agent provides artist images"}}), "a configured external token must never be silently absent from the chain") }) @@ -188,7 +188,7 @@ var _ = Describe("agent images", func() { It("records a skipped external candidate for synthetic artists", func() { a := &fakeImageAgent{name: "agentA", imgs: []agents.ExternalImage{img("/a", 100)}} ag := imageAgents(a) - t := &chainTrace{} + t := &ChainTrace{} _, _, _ = fetchArtistImage(withTrace(ctx, t), ag, passthroughGate, model.Artist{ID: consts.VariousArtistsID, Name: "Various Artists"}) @@ -257,12 +257,12 @@ var _ = Describe("agent images", func() { It("records a skipped external candidate when no agent provides album images", func() { ag := imageAgents() - t := &chainTrace{} + t := &ChainTrace{} r, _, extErr := fetchAlbumImage(withTrace(ctx, t), ag, passthroughGate, model.Album{Name: "Album"}) Expect(r).To(BeNil()) Expect(extErr).To(BeFalse()) - Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "external", Outcome: OutcomeSkipped, + Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "external", Outcome: OutcomeSkipped, Detail: "no enabled agent provides album images"}}), "a configured external token must never be silently absent from the chain") }) diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index 6254e0cf8..e9943f99c 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -387,9 +387,6 @@ func (s *service) parseArtworkID(ctx context.Context, id string) (model.ArtworkI return model.ArtworkID{}, model.ErrNotFound } -type ChainTrace = chainTrace -type TraceStep = traceStep - // Resolver is the CLI's read-only view of resolution: it walks the priority chain, records // the walk and reports the winning source, without ever writing artwork state. type Resolver struct { diff --git a/core/artwork/resolve.go b/core/artwork/resolve.go index cc15c4a81..cc5bbf1b5 100644 --- a/core/artwork/resolve.go +++ b/core/artwork/resolve.go @@ -36,7 +36,7 @@ type resolution struct { // transient external failure still retries; localErr is dropped, as the scanner re-lists changes. type chainState struct { extErr, localErr bool - trace *chainTrace // nil unless the CLI asked for a trace + trace *ChainTrace // nil unless the CLI asked for a trace } // try stamps the accumulated external failure onto a hit, and records the miss otherwise. @@ -56,7 +56,7 @@ func (c *chainState) try(candidate string, res resolution, ok bool) (resolution, } func (c *chainState) record(candidate string, out Outcome, detail string) { - c.trace.add(traceStep{Candidate: candidate, Outcome: out, Detail: detail}) + c.trace.add(TraceStep{Candidate: candidate, Outcome: out, Detail: detail}) } // exhausted is the outcome when no source in the chain yielded an image. diff --git a/core/artwork/trace.go b/core/artwork/trace.go index 6cca9408d..26e5c7b99 100644 --- a/core/artwork/trace.go +++ b/core/artwork/trace.go @@ -28,21 +28,21 @@ const ( ExternalPrefix = ExternalCandidate + ":" ) -// traceStep is one candidate the priority chain considered. -type traceStep struct { +// TraceStep is one candidate the priority chain considered. +type TraceStep struct { Candidate string Outcome Outcome Detail string } -// chainTrace collects the walk of a single resolution. The artwork worker never attaches +// ChainTrace collects the walk of a single resolution. The artwork worker never attaches // one; only the CLI does, so resolution stays allocation-free in the hot path. -type chainTrace struct { +type ChainTrace struct { mu sync.Mutex - steps []traceStep + steps []TraceStep } -func (t *chainTrace) add(step traceStep) { +func (t *ChainTrace) add(step TraceStep) { if t == nil { return } @@ -51,7 +51,7 @@ func (t *chainTrace) add(step traceStep) { t.steps = append(t.steps, step) } -func (t *chainTrace) Steps() []traceStep { +func (t *ChainTrace) Steps() []TraceStep { if t == nil { return nil } @@ -62,29 +62,29 @@ func (t *chainTrace) Steps() []traceStep { type traceCtxKey struct{} -func withTrace(ctx context.Context, t *chainTrace) context.Context { +func withTrace(ctx context.Context, t *ChainTrace) context.Context { return context.WithValue(ctx, traceCtxKey{}, t) } -func traceFrom(ctx context.Context) *chainTrace { - t, _ := ctx.Value(traceCtxKey{}).(*chainTrace) +func traceFrom(ctx context.Context) *ChainTrace { + t, _ := ctx.Value(traceCtxKey{}).(*ChainTrace) return t } var errOfflineSkipped = errors.New("artwork: external lookup skipped (offline)") // tracingGate records each external agent's outcome without changing what the gate returns. -func tracingGate(t *chainTrace, inner gateFunc) gateFunc { +func tracingGate(t *ChainTrace, inner gateFunc) gateFunc { return func(name string, f func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) { r, path, err := inner(name, f) candidate := ExternalPrefix + name switch { case r != nil: - t.add(traceStep{Candidate: candidate, Outcome: OutcomeHit, Detail: path}) + t.add(TraceStep{Candidate: candidate, Outcome: OutcomeHit, Detail: path}) case isTransientExternal(err): - t.add(traceStep{Candidate: candidate, Outcome: OutcomeError, Detail: err.Error()}) + t.add(TraceStep{Candidate: candidate, Outcome: OutcomeError, Detail: err.Error()}) default: - t.add(traceStep{Candidate: candidate, Outcome: OutcomeMiss}) + t.add(TraceStep{Candidate: candidate, Outcome: OutcomeMiss}) } return r, path, err } @@ -92,9 +92,9 @@ func tracingGate(t *chainTrace, inner gateFunc) gateFunc { // offlineGate reports which agents would be asked without asking them, so a diagnostic // command cannot add load to a provider that is already rate-limiting us. -func offlineGate(t *chainTrace) gateFunc { +func offlineGate(t *ChainTrace) gateFunc { return func(name string, _ func() (io.ReadCloser, string, error)) (io.ReadCloser, string, error) { - t.add(traceStep{Candidate: ExternalPrefix + name, Outcome: OutcomeWouldTry}) + t.add(TraceStep{Candidate: ExternalPrefix + name, Outcome: OutcomeWouldTry}) return nil, "", errOfflineSkipped } } diff --git a/core/artwork/trace_test.go b/core/artwork/trace_test.go index 524c02ff3..24c32a4b0 100644 --- a/core/artwork/trace_test.go +++ b/core/artwork/trace_test.go @@ -37,13 +37,13 @@ var _ = Describe("chainTrace", func() { }) It("collects steps in order", func() { - t := &chainTrace{} + t := &ChainTrace{} ctx := withTrace(context.Background(), t) - traceFrom(ctx).add(traceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) - traceFrom(ctx).add(traceStep{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"}) + traceFrom(ctx).add(TraceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) + traceFrom(ctx).add(TraceStep{Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"}) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "cover.*", Outcome: OutcomeMiss}, {Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"}, })) @@ -54,18 +54,18 @@ var _ = Describe("chainTrace", func() { }) It("does not panic when the trace is nil", func() { - var t *chainTrace - Expect(func() { t.add(traceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) }).ToNot(Panic()) + var t *ChainTrace + Expect(func() { t.add(TraceStep{Candidate: "cover.*", Outcome: OutcomeMiss}) }).ToNot(Panic()) Expect(t.Steps()).To(BeEmpty(), "a nil trace collects nothing, so reading it must be as safe as writing it") }) It("is safe to use concurrently", func() { - t := &chainTrace{} + t := &ChainTrace{} done := make(chan struct{}) for range 10 { go func() { defer GinkgoRecover() - t.add(traceStep{Candidate: "x", Outcome: OutcomeMiss}) + t.add(TraceStep{Candidate: "x", Outcome: OutcomeMiss}) done <- struct{}{} }() } @@ -78,17 +78,17 @@ var _ = Describe("chainTrace", func() { var _ = Describe("chainState tracing", func() { It("records a miss when the candidate was absent", func() { - t := &chainTrace{} + t := &ChainTrace{} c := chainState{trace: t} _, ok := c.try("cover.*", resolution{}, false) Expect(ok).To(BeFalse()) - Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "cover.*", Outcome: OutcomeMiss}})) + Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "cover.*", Outcome: OutcomeMiss}})) }) It("records unreadable when the candidate existed but could not be read", func() { - t := &chainTrace{} + t := &ChainTrace{} c := chainState{trace: t} _, ok := c.try("cover.*", resolution{localError: true}, false) @@ -100,14 +100,14 @@ var _ = Describe("chainState tracing", func() { }) It("records a hit with the backing path", func() { - t := &chainTrace{} + t := &ChainTrace{} c := chainState{trace: t} res, ok := c.try("embedded", resolution{reader: nil, source: "embedded", sourcePath: "/music/a.flac"}, true) Expect(ok).To(BeTrue()) Expect(res.source).To(Equal("embedded")) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "embedded", Outcome: OutcomeHit, Detail: "/music/a.flac"}, })) }) @@ -126,26 +126,26 @@ var _ = Describe("external gate tracing", func() { boom := func() (io.ReadCloser, string, error) { return nil, "", errors.New("returned status 429") } It("records a hit with the image path", func() { - t := &chainTrace{} + t := &ChainTrace{} g := tracingGate(t, passthroughGate) r, _, err := g("deezer", hit) Expect(err).ToNot(HaveOccurred()) Expect(r).ToNot(BeNil()) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "external:deezer", Outcome: OutcomeHit, Detail: "http://img"}, })) }) It("records a miss for a not-found", func() { - t := &chainTrace{} + t := &ChainTrace{} _, _, _ = tracingGate(t, passthroughGate)("deezer", miss) Expect(t.Steps()[0].Outcome).To(Equal(OutcomeMiss)) }) It("records a miss for a model not-found", func() { - t := &chainTrace{} + t := &ChainTrace{} notFound := func() (io.ReadCloser, string, error) { return nil, "", model.ErrNotFound } _, _, _ = tracingGate(t, passthroughGate)("deezer", notFound) Expect(t.Steps()[0].Outcome).To(Equal(OutcomeMiss), @@ -153,14 +153,14 @@ var _ = Describe("external gate tracing", func() { }) It("records an error with its reason", func() { - t := &chainTrace{} + t := &ChainTrace{} _, _, _ = tracingGate(t, passthroughGate)("apple-music", boom) Expect(t.Steps()[0].Outcome).To(Equal(OutcomeError)) Expect(t.Steps()[0].Detail).To(ContainSubstring("429")) }) It("never calls the agent in offline mode", func() { - t := &chainTrace{} + t := &ChainTrace{} called := false counting := func() (io.ReadCloser, string, error) { called = true @@ -171,7 +171,7 @@ var _ = Describe("external gate tracing", func() { Expect(called).To(BeFalse(), "offline mode must not perform external requests") Expect(err).To(MatchError(errOfflineSkipped)) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "external:deezer", Outcome: OutcomeWouldTry}, })) }) @@ -185,7 +185,7 @@ var _ = Describe("resolveAlbum tracing", func() { folderRepo *fakeFolderRepo ffm *tests.MockFFmpeg ag *agents.Agents - t *chainTrace + t *ChainTrace ) BeforeEach(func() { @@ -204,7 +204,7 @@ var _ = Describe("resolveAlbum tracing", func() { } ffm = tests.NewMockFFmpeg("") ag = agents.GetAgents(&tests.MockDataStore{}, nil) - t = &chainTrace{} + t = &ChainTrace{} ctx = withTrace(context.Background(), t) }) @@ -218,7 +218,7 @@ var _ = Describe("resolveAlbum tracing", func() { Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() Expect(t.Steps()).To(HaveLen(2), "a configured pattern must appear even when the chain never evaluated it") - Expect(t.Steps()[0]).To(Equal(traceStep{ + Expect(t.Steps()[0]).To(Equal(TraceStep{ Candidate: "cover.jpg", Outcome: OutcomeSkipped, Detail: "no images in album folder", })) Expect(t.Steps()[1].Candidate).To(Equal("embedded")) @@ -238,7 +238,7 @@ var _ = Describe("resolveAlbum tracing", func() { Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() - Expect(t.Steps()[0]).To(Equal(traceStep{Candidate: "cover.jpg", Outcome: OutcomeMiss}), + Expect(t.Steps()[0]).To(Equal(TraceStep{Candidate: "cover.jpg", Outcome: OutcomeMiss}), "the folder was searched and held no cover.jpg, which is not the same as never looking") }) @@ -248,7 +248,7 @@ var _ = Describe("resolveAlbum tracing", func() { _, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "al", ItemID: "al2"}) Expect(err).ToNot(HaveOccurred()) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "cover.jpg", Outcome: OutcomeSkipped, Detail: "no images in album folder"}, })) }) @@ -263,7 +263,7 @@ var _ = Describe("resolveArtist tracing", func() { folderRepo *fakeFolderRepo ffm *tests.MockFFmpeg ag *agents.Agents - t *chainTrace + t *ChainTrace repoRoot string ) @@ -294,7 +294,7 @@ var _ = Describe("resolveArtist tracing", func() { } ffm = tests.NewMockFFmpeg("") ag = agents.GetAgents(&tests.MockDataStore{}, nil) - t = &chainTrace{} + t = &ChainTrace{} ctx = withTrace(context.Background(), t) }) @@ -306,7 +306,7 @@ var _ = Describe("resolveArtist tracing", func() { Expect(err).ToNot(HaveOccurred()) Expect(res.reader).ToNot(BeNil()) defer res.reader.Close() - Expect(t.Steps()).To(Equal([]traceStep{{Candidate: "upload", Outcome: OutcomeHit, Detail: path}})) + Expect(t.Steps()).To(Equal([]TraceStep{{Candidate: "upload", Outcome: OutcomeHit, Detail: path}})) }) It("records an upload miss before walking the chain", func() { @@ -314,7 +314,7 @@ var _ = Describe("resolveArtist tracing", func() { _, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar2"}) Expect(err).ToNot(HaveOccurred()) - Expect(t.Steps()[0]).To(Equal(traceStep{Candidate: "upload", Outcome: OutcomeMiss})) + Expect(t.Steps()[0]).To(Equal(TraceStep{Candidate: "upload", Outcome: OutcomeMiss})) }) It("labels each step with the configured priority token", func() { @@ -342,7 +342,7 @@ var _ = Describe("resolveArtist tracing", func() { _, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar5"}) Expect(err).ToNot(HaveOccurred()) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "upload", Outcome: OutcomeMiss}, {Candidate: "album/artist.*", Outcome: OutcomeSkipped, Detail: "artist has no albums"}, }), "a configured pattern that was never evaluated must still appear, and say why") @@ -355,7 +355,7 @@ var _ = Describe("resolveArtist tracing", func() { _, err := newResolver(ds, ag, ffm, nil).resolve(ctx, model.ArtworkQueueItem{ItemKind: "ar", ItemID: "ar6"}) Expect(err).ToNot(HaveOccurred()) - Expect(t.Steps()).To(Equal([]traceStep{ + Expect(t.Steps()).To(Equal([]TraceStep{ {Candidate: "upload", Outcome: OutcomeMiss}, {Candidate: "artist.*", Outcome: OutcomeSkipped, Detail: "no artist folder"}, }))