summaryrefslogtreecommitdiff
path: root/internal/journal
diff options
context:
space:
mode:
authorLukasz Kasprzak <lukas@labunix.xyz>2026-09-14 22:26:23 +0200
committerLukasz Kasprzak <lukas@labunix.xyz>2026-09-14 22:26:23 +0200
commit21339a73f5cd592ebd92937a1440aa9f13e75bbb (patch)
tree9b3e71e1f05e3c8cf49a34630615cb1f8c4a317c /internal/journal
parentd50f3670bf2ccb843e9001321d072a2d5680cb2a (diff)
downloadkrino-21339a73f5cd592ebd92937a1440aa9f13e75bbb.tar.gz
krino-21339a73f5cd592ebd92937a1440aa9f13e75bbb.zip
plan 10: a damaged log line refuses only its file, not the run
Diffstat (limited to 'internal/journal')
-rw-r--r--internal/journal/read.go39
-rw-r--r--internal/journal/read_test.go59
2 files changed, 76 insertions, 22 deletions
diff --git a/internal/journal/read.go b/internal/journal/read.go
index de06422..48cd6b0 100644
--- a/internal/journal/read.go
+++ b/internal/journal/read.go
@@ -81,9 +81,12 @@ func ReversedSteps(path, runID string) (map[ReversedKey]int, error) {
return out, nil
}
-// Entries returns every entry belonging to runID, in file order. A line
-// that fails to parse is skipped, but Entries fails closed within the run's
-// own window - from its run-start line to its run-end line, or to end of
+// Entries returns every entry belonging to runID, in file order. Since plan
+// 10 (re-review N1), an unparsable line whose run column names another run
+// is ignored, and one of this run whose directory and file columns are still
+// readable is returned as a "damaged" entry for that file, so undo refuses
+// that file alone. Otherwise a line that fails to parse is skipped, but
+// Entries fails closed within the run's own window - from its run-start line to its run-end line, or to end of
// file when there is no run-end (a crashed run, which is precisely when
// corruption is likely): any unparsable line found inside that window sets
// the returned error, whether or not the line's own Run column can still be
@@ -136,14 +139,25 @@ func Entries(path, runID string) ([]Entry, error) {
}
continue
}
- if badLine != 0 {
+ run, runFound := runFieldOf(line)
+ if runFound && run != runID {
+ // Another run's damaged line: runs of different directories can
+ // interleave, and it says nothing about this one (re-review N1).
continue
}
- if inWindow {
- badLine = i + 1
+ ours := inWindow || (runFound && run == runID)
+ if !ours {
+ continue
+ }
+ if dir, file, ok := fileFieldsOf(line); ok {
+ // A line of this run cut or damaged where its file is still
+ // readable: that file's chain may be missing a step, so it is
+ // returned as damaged and PlanUndo refuses just that file; the
+ // rest of the run stays undoable (re-review N1).
+ out = append(out, Entry{Run: runID, Dir: dir, File: file, Action: "damaged", Status: "damaged", Detail: fmt.Sprintf("line %d", i+1)})
continue
}
- if run, found := runFieldOf(line); found && run == runID {
+ if badLine == 0 {
badLine = i + 1
}
}
@@ -167,6 +181,17 @@ func runFieldOf(line string) (string, bool) {
return unescape(f[1]), true
}
+// fileFieldsOf best-effort extracts a line's directory and file columns when
+// the line otherwise fails to parse; ok is false when the line is cut before
+// them or names no file (a run-start or run-end line).
+func fileFieldsOf(line string) (dir, file string, ok bool) {
+ f := strings.SplitN(line, "\t", 5)
+ if len(f) < 5 || f[3] == "" {
+ return "", "", false
+ }
+ return unescape(f[2]), unescape(f[3]), true
+}
+
// Runs summarises every run found in the log, newest first. n <= 0 means
// all. As with Entries, an unparsable line is skipped rather than failing
// the read - here silently and always, even when it belonged to the run
diff --git a/internal/journal/read_test.go b/internal/journal/read_test.go
index 2b46bfb..fb6f77f 100644
--- a/internal/journal/read_test.go
+++ b/internal/journal/read_test.go
@@ -303,15 +303,18 @@ func TestEntriesBothFailureModesReportsBadLineFirst(t *testing.T) {
t.Fatal(err)
}
+ // Since plan 10 a damaged line whose file is readable refuses only that
+ // file (a "damaged" entry); the run as a whole still fails closed here,
+ // on its missing run-start.
got, err := Entries(path, "A")
if err == nil {
- t.Fatal("Entries returned no error with both a bad line and a missing run-start present")
+ t.Fatal("Entries returned no error with a missing run-start")
}
- if !strings.Contains(err.Error(), "unparsable line 2") {
- t.Errorf("error = %q, want it to report the unparsable line (line 2), not the missing run-start", err)
+ if !strings.Contains(err.Error(), "no readable run-start") {
+ t.Errorf("error = %q, want it to name the missing run-start", err)
}
- if len(got) != 1 || got[0].Action != "run-end" {
- t.Errorf("entries = %+v, want just the surviving run-end", got)
+ if len(got) != 2 || got[0].Action != "damaged" || got[0].File != "x.pdf" || got[1].Action != "run-end" {
+ t.Errorf("entries = %+v, want x.pdf damaged, then run-end", got)
}
}
@@ -412,8 +415,10 @@ func TestEntriesAdjacentRunStartsOneCorrupted(t *testing.T) {
// TestEntriesReportsAMangledLine: a corrupt line that is not the log's
// final line must not be silently dropped by Entries the way Runs drops it
-// - PlanUndo needs to know a step went missing so it can refuse the whole
-// run rather than half-undo a file (spec ยง10).
+// - PlanUndo needs to know a step went missing. Since plan 10 (re-review
+// N1) a line whose directory and file columns are readable is returned as a
+// "damaged" entry for that file, so only that file is refused and the rest
+// of the run can still be undone.
func TestEntriesReportsAMangledLine(t *testing.T) {
path := filepath.Join(t.TempDir(), "krino.log")
w, _ := Open(path)
@@ -451,17 +456,41 @@ func TestEntriesReportsAMangledLine(t *testing.T) {
}
got, err := Entries(path, "A")
- if err == nil {
- t.Fatal("Entries did not report the mangled line")
+ if err != nil {
+ t.Fatalf("Entries = %v; a damaged line with a readable file must not refuse the run", err)
}
- if !strings.Contains(err.Error(), "line 2") {
- t.Errorf("error %q does not name line 2", err)
+ if len(got) != 3 || got[1].Action != "damaged" || got[1].Dir != "dl" || got[1].File != "x.pdf" || !strings.Contains(got[1].Detail, "line 2") {
+ t.Fatalf("entries = %+v; want run-start, x.pdf damaged (line 2), run-end", got)
}
- if len(got) != 2 {
- t.Fatalf("got %d entries, want the 2 surviving (run-start, run-end): %+v", len(got), got)
+}
+
+// TestEntriesIgnoresAnotherRunsDamagedLine: a damaged line whose run column
+// names another run - two directories' runs can interleave in one log - does
+// not refuse this run, even inside its window (re-review N1).
+func TestEntriesIgnoresAnotherRunsDamagedLine(t *testing.T) {
+ path := filepath.Join(t.TempDir(), "krino.log")
+ w, _ := Open(path)
+ at := time.Date(2026, 9, 11, 10, 2, 3, 0, time.UTC)
+ for _, e := range []Entry{
+ {Time: at, Run: "A", Action: "run-start", Status: "ok"},
+ {Time: at, Run: "A", Dir: "dl", File: "x.pdf", Step: 1, Action: "move", Status: "ok", Src: "/a/x.pdf", Dst: "/b/x.pdf"},
+ {Time: at, Run: "A", Action: "run-end", Status: "ok"},
+ } {
+ if err := w.Append(e); err != nil {
+ t.Fatal(err)
+ }
}
- if got[0].Action != "run-start" || got[1].Action != "run-end" {
- t.Errorf("entries = %+v", got)
+ w.Close()
+ raw, _ := os.ReadFile(path)
+ lines := strings.Split(strings.TrimRight(string(raw), "\n"), "\n")
+ cut := "2026-09-11T10:02:04Z\tB\tscans\ty.pdf\t1\tmo"
+ lines = append(lines[:2], append([]string{cut}, lines[2:]...)...)
+ if err := os.WriteFile(path, []byte(strings.Join(lines, "\n")+"\n"), 0o644); err != nil {
+ t.Fatal(err)
+ }
+ got, err := Entries(path, "A")
+ if err != nil || len(got) != 3 {
+ t.Errorf("Entries(A) = %+v, %v; want A's three entries and no error", got, err)
}
}