mirror of
https://github.com/navidrome/navidrome.git
synced 2026-10-11 20:07:11 +02:00
fix(cli): cover the reprocess --yes guard and preview the external cost
Mutating the --yes check to `if true` left the suite green, so the one bypass of the confirmation was unverified. The choice is now reprocessConfirm(yes, in), covered in both directions. The external estimate only reached the operator through the prompt, which --dry-run skips — hiding the number in the one mode that exists to show it before committing. The preview now carries it, and the prompt drops the clause when no lookup will be made. An empty selection says so again under --dry-run.
This commit is contained in:
parent
bd7b46c91c
commit
482ed50d7d
2 changed files with 65 additions and 10 deletions
|
|
@ -102,12 +102,8 @@ func runReprocess(ctx context.Context) {
|
|||
defer db.Init(ctx)()
|
||||
ds, ctx := getAdminContext(ctx)
|
||||
|
||||
confirm := promptConfirm(os.Stdin)
|
||||
if reprocessYes {
|
||||
confirm = func(io.Writer, int64, int64) bool { return true }
|
||||
}
|
||||
if err := reprocessArtwork(ctx, ds, kinds, repositorySources(reprocessSources),
|
||||
reprocessDryRun, confirm, os.Stdout); err != nil {
|
||||
reprocessDryRun, reprocessConfirm(reprocessYes, os.Stdin), os.Stdout); err != nil {
|
||||
log.Fatal(ctx, err)
|
||||
}
|
||||
}
|
||||
|
|
@ -157,10 +153,21 @@ func displaySource(s string) string { return cmp.Or(s, absentSource) }
|
|||
// reach an external agent.
|
||||
type confirmFunc func(out io.Writer, total, external int64) bool
|
||||
|
||||
// reprocessConfirm is the only bypass of the confirmation: --yes, for scripted use.
|
||||
func reprocessConfirm(yes bool, in io.Reader) confirmFunc {
|
||||
if yes {
|
||||
return func(io.Writer, int64, int64) bool { return true }
|
||||
}
|
||||
return promptConfirm(in)
|
||||
}
|
||||
|
||||
func promptConfirm(in io.Reader) confirmFunc {
|
||||
return func(out io.Writer, total, external int64) bool {
|
||||
fmt.Fprintf(out, "\nThis will re-resolve %d items, requiring up to %d external lookups. Continue? [y/N] ",
|
||||
total, external)
|
||||
cost := fmt.Sprintf(", requiring up to %d external lookups", external)
|
||||
if external == 0 {
|
||||
cost = ""
|
||||
}
|
||||
fmt.Fprintf(out, "\nThis will re-resolve %d items%s. Continue? [y/N] ", total, cost)
|
||||
var answer string
|
||||
if _, err := fmt.Fscanln(in, &answer); err != nil {
|
||||
return false
|
||||
|
|
@ -225,14 +232,17 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
|
|||
external += n
|
||||
}
|
||||
}
|
||||
printReprocessPreview(out, kinds, matched, total, sources)
|
||||
printReprocessPreview(out, kinds, matched, total, external, sources)
|
||||
|
||||
if total == 0 {
|
||||
fmt.Fprintln(out, "\nNothing matches this selection.")
|
||||
}
|
||||
switch {
|
||||
case dryRun:
|
||||
fmt.Fprintln(out, "\nDry run: nothing was queued.")
|
||||
return nil
|
||||
case total == 0:
|
||||
fmt.Fprintln(out, "\nNothing matches this selection; nothing was queued.")
|
||||
fmt.Fprintln(out, "Nothing was queued.")
|
||||
return nil
|
||||
case !confirm(out, total, external):
|
||||
fmt.Fprintln(out, "Aborted: nothing was queued.")
|
||||
|
|
@ -258,7 +268,9 @@ func reprocessArtwork(ctx context.Context, ds model.DataStore, kinds []model.Kin
|
|||
return nil
|
||||
}
|
||||
|
||||
func printReprocessPreview(out io.Writer, kinds []model.Kind, matched []int64, total int64, sources []string) {
|
||||
// printReprocessPreview also states the external estimate, which --dry-run must show precisely
|
||||
// because it skips the prompt that would otherwise carry it.
|
||||
func printReprocessPreview(out io.Writer, kinds []model.Kind, matched []int64, total, external 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)"))
|
||||
|
|
@ -268,6 +280,12 @@ func printReprocessPreview(out io.Writer, kinds []model.Kind, matched []int64, t
|
|||
}
|
||||
fmt.Fprintf(w, "TOTAL\t%d\n", total)
|
||||
w.Flush()
|
||||
|
||||
estimate := fmt.Sprintf("up to %d", external)
|
||||
if external == 0 {
|
||||
estimate = "none"
|
||||
}
|
||||
fmt.Fprintf(out, "\nExternal lookups: %s\n", estimate)
|
||||
}
|
||||
|
||||
func runRefresh(ctx context.Context, kind model.Kind, ids []string) {
|
||||
|
|
|
|||
|
|
@ -270,6 +270,28 @@ var _ = Describe("promptConfirm", func() {
|
|||
Expect(promptConfirm(strings.NewReader("nope\n"))(&out, 1, 1)).To(BeFalse())
|
||||
Expect(promptConfirm(strings.NewReader(""))(&out, 1, 1)).To(BeFalse())
|
||||
})
|
||||
|
||||
It("drops the external clause when no lookup will be made", func() {
|
||||
Expect(promptConfirm(strings.NewReader("y\n"))(&out, 3, 0)).To(BeTrue())
|
||||
Expect(out.String()).To(ContainSubstring("re-resolve 3 items."))
|
||||
Expect(out.String()).ToNot(ContainSubstring("external lookups"))
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("reprocessConfirm", func() {
|
||||
var out strings.Builder
|
||||
|
||||
BeforeEach(func() { out.Reset() })
|
||||
|
||||
It("prompts when --yes was not given", func() {
|
||||
Expect(reprocessConfirm(false, strings.NewReader("n\n"))(&out, 5, 5)).To(BeFalse())
|
||||
Expect(out.String()).To(ContainSubstring("Continue?"))
|
||||
})
|
||||
|
||||
It("bypasses the prompt only for --yes", func() {
|
||||
Expect(reprocessConfirm(true, strings.NewReader(""))(&out, 5, 5)).To(BeTrue())
|
||||
Expect(out.String()).To(BeEmpty(), "--yes must not print a prompt it never reads")
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("reprocessArtwork", func() {
|
||||
|
|
@ -367,9 +389,24 @@ var _ = Describe("reprocessArtwork", func() {
|
|||
It("reports an empty selection as a dry run when one was asked for", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindRadioArtwork}, nil, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("Nothing matches"))
|
||||
Expect(out.String()).To(ContainSubstring("Dry run"))
|
||||
})
|
||||
|
||||
It("shows the external estimate on a dry run, which never reaches the prompt", func() {
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindAlbumArtwork}, nil, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: up to 2"))
|
||||
})
|
||||
|
||||
It("says so when the selection needs no external lookup", func() {
|
||||
put(model.KindPlaylistArtwork, "pl-1", "playlist")
|
||||
|
||||
Expect(reprocessArtwork(ctx, ds, []model.Kind{model.KindPlaylistArtwork}, nil, true, accept, &out)).To(Succeed())
|
||||
|
||||
Expect(out.String()).To(ContainSubstring("External lookups: none"))
|
||||
})
|
||||
|
||||
It("counts only the kinds that call an external agent as external cost", func() {
|
||||
put(model.KindPlaylistArtwork, "pl-1", "playlist")
|
||||
var total, external int64
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue