diff options
Diffstat (limited to 'internal/engine')
| -rw-r--r-- | internal/engine/apply.go | 19 | ||||
| -rw-r--r-- | internal/engine/undo_identity_test.go | 51 |
2 files changed, 68 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) + } +} |
