diff options
| author | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 13:58:25 +0200 |
|---|---|---|
| committer | Lukasz Kasprzak <lukas@labunix.xyz> | 2026-09-17 13:58:25 +0200 |
| commit | 9ab6686b98499c745024a474a71e3d99b6e14973 (patch) | |
| tree | 1debf57412d72c6289dd06bd17f0c024a490a3af /internal/engine | |
| parent | a80d470cbad7335fd5e534f32d935e5a49aadb24 (diff) | |
| download | krino-9ab6686b98499c745024a474a71e3d99b6e14973.tar.gz krino-9ab6686b98499c745024a474a71e3d99b6e14973.zip | |
a chain that deletes itself still gives back the file it displaced
(on-conflict overwrite) trashes the file in the way; §7.4 promises undo
restores it. Walking a file's log entries stopped dead at a permanent
delete, so the displace written earlier in the same chain was never
reached: the user's file stayed in the Trash, the refusal named only
the file they did not care about, and krino log called the run undone.
The displaced file is a different file, so it is offered as its own
entry in the undo plan, keyed by its own path - the deleted file stays
refused, since nothing of it can come back, and the copy or move that
preceded the delete stays unreversed too (undoing a copy whose original
was then deleted would destroy the last remaining copy).
The accounting matched: every reversible step of a deleted file was
subtracted, its displace included, so the run read (undone). Only what
genuinely cannot come back is subtracted now.
End to end, the scenario from the review: the only copy of a file is
displaced by an incoming one that is then permanently deleted.
before: archive/ empty, "(undone)", nothing offered
after: archive/a.pdf restored, run reads partly undone
Diffstat (limited to 'internal/engine')
| -rw-r--r-- | internal/engine/apply.go | 61 | ||||
| -rw-r--r-- | internal/engine/apply_test.go | 79 |
2 files changed, 140 insertions, 0 deletions
diff --git a/internal/engine/apply.go b/internal/engine/apply.go index e2d4db8..f54169c 100644 --- a/internal/engine/apply.go +++ b/internal/engine/apply.go @@ -344,6 +344,18 @@ type UndoFile struct { Declined bool } +// Label names the file for display: the directory it belongs to and the +// path within it, except for a file that lies outside any sorted directory +// - one displaced into the Trash by a chain that then deleted itself - whose +// path is absolute and stands alone. Dir stays set on those either way: it +// is what the undo locks. +func (uf UndoFile) Label() string { + if filepath.IsAbs(uf.File) { + return xdg.Abbrev(uf.File) + } + return uf.Dir + "/" + uf.File +} + // UndoStep is the reversal of one logged step. type UndoStep struct { Original journal.Entry // the step being reversed @@ -420,6 +432,13 @@ func (e *Engine) PlanUndo(runID string) (*UndoPlan, error) { if len(uf.Steps) == 0 && uf.Refused == "" { continue } + // A file refused outright still displaced someone else's file into + // the Trash, and that file is a different file - §7.4 promises undo + // restores it. Offered on its own, since the refusal is about the + // file that cannot come back, not about this one. + if rest, ok := displacedUndoFile(k.dir, byFile[k], reversed); ok { + up.Files = append(up.Files, rest) + } if uf.Refused == "" && onlyOccupiedDirectoryRemovals(uf.Steps) { // Nothing of the file itself is left to reverse, only directories // the run made that something else still occupies: offering them @@ -585,6 +604,48 @@ func planUndoFile(dir, file string, ents []journal.Entry, reversed map[journal.R return uf } +// displacedUndoFile offers the file a refused chain trashed to make room. +// It applies only to a chain planUndoFile refuses as a whole - a reversible +// chain reverses its own displace as one of its steps - and only to a +// refusal the displaced file is not itself the cause of: a permanent +// delete, which ends the deleted file for good while leaving the file it +// displaced sitting in the Trash, recoverable, with its original path now +// free. +// +// It is keyed by the displaced path rather than the chain's file, because +// that is what it puts back; a plan that named the chain's file twice would +// read as one file being reversed in two places. +func displacedUndoFile(dir string, ents []journal.Entry, reversed map[journal.ReversedKey]int) (UndoFile, bool) { + deleted := false + for _, en := range ents { + if en.Status == "ok" && en.Action == "delete" { + deleted = true + } + } + if !deleted { + return UndoFile{}, false + } + var out UndoFile + for i := len(ents) - 1; i >= 0; i-- { + en := ents[i] + if en.Status != "ok" || en.Action != "displace" { + continue + } + step := reverseStep(en) + if k := (journal.ReversedKey{Dir: dir, File: en.File, Action: step.Action, Src: step.Src}); reversed[k] > 0 { + reversed[k]-- + continue + } + out.Dir = dir + out.File = en.Src + out.Steps = append(out.Steps, step) + if step.Refused != "" && out.Refused == "" { + out.Refused = step.Refused + } + } + return out, len(out.Steps) > 0 +} + // undoProjection tracks what the reversal steps planUndoFile has already // queued (in the order they will execute) will do to the filesystem, so a // later step's occupancy check can tell a real, external occupant from a diff --git a/internal/engine/apply_test.go b/internal/engine/apply_test.go index a6b3185..c832e3a 100644 --- a/internal/engine/apply_test.go +++ b/internal/engine/apply_test.go @@ -1463,3 +1463,82 @@ func TestApplyUndoRetryLogsBothMkdirEntriesAndStillMarksOriginalRunUndone(t *tes t.Errorf("the undo run %q itself must never read as Undone", undoRun) } } + +// TestPermanentDeleteStillRestoresWhatItDisplaced: a chain that overwrites +// and then permanently deletes trashes a file the user owned to make room. +// That file is a different file, and §7.4 promises of it: "move the +// existing target to Trash first (logged, so undo restores it)". Walking +// the file's entries used to stop dead at the permanent delete, so the +// displace was never reached and the user's file stayed in the Trash with +// krino reporting the run fully undone. +func TestPermanentDeleteStillRestoresWhatItDisplaced(t *testing.T) { + // A real trash entry, so the reversal is offered rather than refused + // for a reason that has nothing to do with this test. + h := sandbox(t) + entry := filepath.Join(trash.Dir(), "files", "a.pdf") + if err := os.MkdirAll(filepath.Dir(entry), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(entry, []byte("the file that was in the way"), 0o644); err != nil { + t.Fatal(err) + } + fi, err := os.Lstat(entry) + if err != nil { + t.Fatal(err) + } + displaced := filepath.Join(h, "archive", "a.pdf") + info := filepath.Join(trash.Dir(), "info", "a.pdf.trashinfo") + if err := os.MkdirAll(filepath.Dir(info), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(info, []byte("[Trash Info]\nPath="+displaced+"\nDeletionDate=2026-09-17T00:00:00\n"), 0o644); err != nil { + t.Fatal(err) + } + ents := []journal.Entry{ + // Chronological, as the log has them: the displace first, then the + // move that needed the name, then the permanent delete. + {Action: "displace", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1, + Src: displaced, Dst: entry, Detail: "a.pdf", + Size: fi.Size(), ModTime: fi.ModTime()}, + {Action: "move", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1, + Src: filepath.Join(h, "dl", "a.pdf"), Dst: displaced}, + {Action: "delete", Status: "ok", File: "a.pdf", Dir: "dl", Step: 2, + Src: displaced}, + } + uf := planUndoFile("dl", "a.pdf", ents, map[journal.ReversedKey]int{}) + if uf.Refused == "" { + t.Error("the permanently deleted file is no longer refused") + } + + rest, ok := displacedUndoFile("dl", ents, map[journal.ReversedKey]int{}) + if !ok { + t.Fatal("the displaced file was not offered for reversal at all") + } + if rest.Refused != "" { + t.Errorf("the displaced file is refused: %q", rest.Refused) + } + if len(rest.Steps) != 1 || rest.Steps[0].Action != "undo-displace" { + t.Fatalf("steps = %+v; want one undo-displace", rest.Steps) + } + if rest.Steps[0].Dst != displaced { + t.Errorf("the reversal puts the file at %q, want %q", rest.Steps[0].Dst, displaced) + } + if rest.File != displaced { + t.Errorf("the offered file is %q, want the displaced file %q", rest.File, displaced) + } +} + +// TestDisplacedFileIsNotOfferedTwice: when the chain's own file is +// reversible, the displace is reversed as one of its steps, as before - +// the separate offer exists only for the file that cannot be reversed. +func TestDisplacedFileIsNotOfferedTwice(t *testing.T) { + ents := []journal.Entry{ + {Action: "displace", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1, + Src: "/archive/a.pdf", Dst: "/trash/files/a.pdf", Detail: "a.pdf"}, + {Action: "move", Status: "ok", File: "a.pdf", Dir: "dl", Step: 1, + Src: "/dl/a.pdf", Dst: "/archive/a.pdf"}, + } + if _, ok := displacedUndoFile("dl", ents, map[journal.ReversedKey]int{}); ok { + t.Error("a reversible chain's displace was offered a second time on its own") + } +} |
