diff options
Diffstat (limited to 'internal')
| -rw-r--r-- | internal/engine/apply.go | 19 | ||||
| -rw-r--r-- | internal/engine/undo_identity_test.go | 51 | ||||
| -rw-r--r-- | internal/journal/read.go | 37 | ||||
| -rw-r--r-- | internal/journal/read_test.go | 48 |
4 files changed, 153 insertions, 2 deletions
diff --git a/internal/engine/apply.go b/internal/engine/apply.go index 94d65e4..e109aaf 100644 --- a/internal/engine/apply.go +++ b/internal/engine/apply.go @@ -343,6 +343,14 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) { return nil, fmt.Errorf("engine: plan undo: run %s is itself an undo and cannot be undone", runID) } + // Reversals an earlier undo of this same run already completed are not + // offered again (review M10): an undo that stopped part way can be + // finished by undoing the run once more. + reversed, err := journal.ReversedSteps(e.Config.LogFile(), runID) + if err != nil { + return nil, fmt.Errorf("engine: plan undo: %w", err) + } + // Entries are grouped by directory and file together (review M7): one run // spans every directory, and two directories can each hold a file of the // same name. @@ -362,7 +370,7 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) { up := &UndoPlan{Run: runID} for _, k := range order { - uf := planUndoFile(k.dir, k.file, byFile[k]) + uf := planUndoFile(k.dir, k.file, byFile[k], reversed) // Critical finding, Task 8's review: a file every one of whose // entries has Status != "ok" (declined by the ORIGINAL run's own // review, or skipped, or failed before anything happened) yields @@ -464,7 +472,7 @@ func isFileAffecting(action string) bool { // two steps of one file that touch the same path, and every un-contended // check keeps behaving exactly as before, since the projection only ever // overrides a real occupant that this same chain is itself about to clear. -func planUndoFile(dir, file string, ents []journal.Entry) UndoFile { +func planUndoFile(dir, file string, ents []journal.Entry, reversed map[journal.ReversedKey]int) UndoFile { uf := UndoFile{File: file, Dir: dir} proj := newUndoProjection() for i := len(ents) - 1; i >= 0; i-- { @@ -483,6 +491,13 @@ func planUndoFile(dir, file string, ents []journal.Entry) UndoFile { break } step := reverseStep(en) + if k := (journal.ReversedKey{Dir: dir, File: file, Action: step.Action, Src: step.Src}); reversed[k] > 0 { + // An earlier undo of this run already reversed this step: the + // disk already shows it, and it is not offered again. + reversed[k]-- + proj.record(step) + continue + } if (en.Action == "move" || en.Action == "rename") && proj.occupied[en.Dst] { // A reversal already queued for this same file puts it back at // en.Dst before this one runs, and that reversal was checked diff --git a/internal/engine/undo_identity_test.go b/internal/engine/undo_identity_test.go index 9c93b3b..d4169be 100644 --- a/internal/engine/undo_identity_test.go +++ b/internal/engine/undo_identity_test.go @@ -223,3 +223,54 @@ func TestUndoLeavesNoDirectoriesBehind(t *testing.T) { t.Errorf("dl/Out is left behind: %v", err) } } + +// TestUndoCanBeFinishedAfterAFailure: an undo whose last reversal failed +// (something took the original name) leaves the file part way back; once +// the obstacle is gone, undoing the same run again offers only the step +// that is left, and finishes it (review M10). +func TestUndoCanBeFinishedAfterAFailure(t *testing.T) { + e, run, h, logPath := appliedRun(t, map[string]map[string]string{"dl": {"a.pdf": "one"}}, + map[string]string{"dl": "(path \"~/dl\")\n(rule \"r\" (rename \"r-{name}\") (move \"Out\"))\n"}) + up, err := e.PlanUndo(run) + if err != nil { + t.Fatal(err) + } + blocker := filepath.Join(h, "dl", "a.pdf") + if err := os.WriteFile(blocker, []byte("in the way"), 0o644); err != nil { + t.Fatal(err) + } + undo := func(up *UndoPlan) *ApplyResult { + t.Helper() + j, err := journal.Open(logPath) + if err != nil { + t.Fatal(err) + } + defer j.Close() + res, err := e.ApplyUndo(context.Background(), up, j, journal.NewRunID(time.Now())) + if err != nil { + t.Fatal(err) + } + return res + } + if res := undo(up); res.Failed != 1 { + t.Fatalf("first undo: Failed = %d, want 1 (the rename back is blocked)", res.Failed) + } + if err := os.Remove(blocker); err != nil { + t.Fatal(err) + } + time.Sleep(10 * time.Millisecond) // a new run id + again, err := e.PlanUndo(run) + if err != nil { + t.Fatal(err) + } + f := undoFileNamed(t, again, "dl", "a.pdf") + if f.Refused != "" || len(f.Steps) != 1 || f.Steps[0].Action != "undo-rename" { + t.Fatalf("second plan: %+v; want one undo-rename step", f) + } + if res := undo(again); res.Failed != 0 || res.Applied != 1 { + t.Fatalf("second undo: %+v", res) + } + if b, err := os.ReadFile(filepath.Join(h, "dl", "a.pdf")); err != nil || string(b) != "one" { + t.Errorf("dl/a.pdf = %q, %v; want the original back", b, err) + } +} diff --git a/internal/journal/read.go b/internal/journal/read.go index 3de15d2..de06422 100644 --- a/internal/journal/read.go +++ b/internal/journal/read.go @@ -43,6 +43,42 @@ type Run struct { Dirs []string Counts map[string]int // action -> count of status "ok" Undone bool // a later run reversed this one + UndoOf string // for an undo run, the run it reverses; "" otherwise +} + +// ReversedKey identifies one reversal an undo run carried out: the file's +// directory and name, the undo action and the path it started from - enough +// to tell which step of the original run it reversed. +type ReversedKey struct { + Dir, File, Action, Src string +} + +// ReversedSteps counts, for runID, every reversal that earlier undo runs of +// it completed ("ok" undo- entries of runs whose run-start says they undo +// runID), so a later undo of the same run can offer only what is left +// (review M10). An undo run's own unparsable lines are skipped; a missing +// reversal is then offered again, where its own checks refuse it if it had +// in fact happened. +func ReversedSteps(path, runID string) (map[ReversedKey]int, error) { + lines, err := readLines(path) + if err != nil { + return nil, err + } + undoRuns := map[string]bool{} + for _, line := range lines { + if e, ok := parseLine(line); ok && e.Action == "run-start" && e.Detail == UndoOf(runID) { + undoRuns[e.Run] = true + } + } + out := map[ReversedKey]int{} + for _, line := range lines { + e, ok := parseLine(line) + if !ok || !undoRuns[e.Run] || e.Status != "ok" || !strings.HasPrefix(e.Action, "undo-") { + continue + } + out[ReversedKey{Dir: e.Dir, File: e.File, Action: e.Action, Src: e.Src}]++ + } + return out, nil } // Entries returns every entry belonging to runID, in file order. A line @@ -198,6 +234,7 @@ func Runs(path string, n int) ([]Run, error) { sort.SliceStable(runs, func(i, j int) bool { return runs[i].Start.After(runs[j].Start) }) for i := range runs { runs[i].Undone = undoes[runs[i].ID] + runs[i].UndoOf = pendingUndo[runs[i].ID] } if n > 0 && n < len(runs) { diff --git a/internal/journal/read_test.go b/internal/journal/read_test.go index 2f0f40f..2b46bfb 100644 --- a/internal/journal/read_test.go +++ b/internal/journal/read_test.go @@ -625,3 +625,51 @@ func TestEntriesFailsClosedOnMissingRunStart(t *testing.T) { t.Errorf("entries = %+v", got) } } + +// TestReversedStepsCountsEveryUndoOfARun: the steps earlier undo runs of a +// run already reversed - only "ok" undo entries of runs that undo it - so +// a later undo of the same run can offer just what is left (review M10). +// Runs also names the run an undo run reversed. +func TestReversedStepsCountsEveryUndoOfARun(t *testing.T) { + path := filepath.Join(t.TempDir(), "krino.log") + w, err := Open(path) + if err != nil { + t.Fatal(err) + } + at := time.Date(2026, 9, 11, 9, 0, 0, 0, time.UTC) + for _, e := range []Entry{ + {Time: at, Run: "R", Action: "run-start", Status: "ok"}, + {Time: at, Run: "R", Dir: "dl", File: "a.pdf", Step: 1, Action: "move", Status: "ok", Src: "/dl/a.pdf", Dst: "/w/a.pdf"}, + {Time: at, Run: "R", Action: "run-end", Status: "ok"}, + {Time: at.Add(time.Minute), Run: "U1", Action: "run-start", Status: "ok", Detail: UndoOf("R")}, + {Time: at.Add(time.Minute), Run: "U1", Dir: "dl", File: "a.pdf", Step: 1, Action: "undo-move", Status: "ok", Src: "/w/a.pdf", Dst: "/dl/a.pdf"}, + {Time: at.Add(time.Minute), Run: "U1", Dir: "dl", File: "b.pdf", Step: 1, Action: "undo-rename", Status: "failed", Src: "/dl/r-b.pdf", Dst: "/dl/b.pdf"}, + {Time: at.Add(time.Minute), Run: "U1", Action: "run-end", Status: "ok"}, + {Time: at.Add(2 * time.Minute), Run: "U2", Action: "run-start", Status: "ok", Detail: UndoOf("OTHER")}, + {Time: at.Add(2 * time.Minute), Run: "U2", Dir: "dl", File: "a.pdf", Step: 1, Action: "undo-move", Status: "ok", Src: "/w/a.pdf", Dst: "/dl/a.pdf"}, + {Time: at.Add(2 * time.Minute), Run: "U2", Action: "run-end", Status: "ok"}, + } { + if err := w.Append(e); err != nil { + t.Fatal(err) + } + } + w.Close() + got, err := ReversedSteps(path, "R") + if err != nil { + t.Fatal(err) + } + want := map[ReversedKey]int{{Dir: "dl", File: "a.pdf", Action: "undo-move", Src: "/w/a.pdf"}: 1} + if len(got) != len(want) || got[ReversedKey{Dir: "dl", File: "a.pdf", Action: "undo-move", Src: "/w/a.pdf"}] != 1 { + t.Errorf("ReversedSteps = %v, want %v", got, want) + } + runs, err := Runs(path, 0) + if err != nil { + t.Fatal(err) + } + for _, r := range runs { + want := map[string]string{"R": "", "U1": "R", "U2": "OTHER"}[r.ID] + if r.UndoOf != want { + t.Errorf("run %s: UndoOf = %q, want %q", r.ID, r.UndoOf, want) + } + } +} |