History diff: keep a stepped-back object in place; number hunks right

Rewind put an object's old form at the end of its list, so a history
entry's diff showed the changed object moving (turning hairpin off for
one forward also moved its forward and DNAT lines). It now sorts after
stepping back. go-udiff v0.4.1 numbers the new side of hunks after one
with a moved line too early, so the chain after @@ could be the one
before; hunkStarts numbers them again. History diffs name their sides
"before N" and "after N".

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
abrown38andClaude Opus 5.5 committed 2026-10-23 10:49:41 +01:00
1 parent 540f4b85ac
commit 0d16358617
4 files changed
+143 -2

No files matched your search

+2
View File
@@ -335,6 +335,8 @@ func Rewind(st *model.State, h *model.HistoryEntry) error {
return err
}
}
// put appends what it brings back; its position puts it in place.
st.Sort()
return nil
}
+67
View File
@@ -1,6 +1,8 @@
package server
import (
"encoding/json"
"fmt"
"strings"
"testing"
@@ -70,3 +72,68 @@ func TestHistoryDiff(t *testing.T) {
t.Errorf("unknown entry: %d", code)
}
}
// TestHunkStarts: after a hunk with unchanged lines between its edits (a
// moved line), the later hunks' new sides start where they do.
func TestHunkStarts(t *testing.T) {
var a, b strings.Builder
for i := 1; i <= 20; i++ {
fmt.Fprintf(&a, "%d\n", i)
switch i {
case 4:
case 6:
b.WriteString("6\n4\n")
case 14:
default:
fmt.Fprintf(&b, "%d\n", i)
}
}
diff := hunkStarts(udiff.Unified("a", "b", a.String(), b.String()))
var heads []string
for _, l := range strings.Split(diff, "\n") {
if strings.HasPrefix(l, "@@") {
heads = append(heads, l)
}
}
// diff -u: the move nets nothing, the deleted 14 one line.
if want := []string{"@@ -1,9 +1,9 @@", "@@ -11,7 +11,6 @@"}; strings.Join(heads, "|") != strings.Join(want, "|") {
t.Fatalf("hunk headers %q, want %q\n%s", heads, want, diff)
}
}
// TestHistoryDiffUpdate: an update's diff is only what it changed; the
// object stepped back to its old form stays where it was.
func TestHistoryDiffUpdate(t *testing.T) {
e := newEnv(t)
e.call(e.token, "POST", "/zones", model.Zone{Name: "lan", Interfaces: []string{"eth1"}}, nil)
e.call(e.token, "POST", "/zones", model.Zone{Name: "wan", Interfaces: []string{"wan"}, External: true}, nil)
var ids []int64
for i, port := range []string{"8096", "443", "50300", "7777", "25"} {
var mr api.MutationResponse
e.call(e.token, "POST", "/forwards", map[string]any{"name": "f" + port, "ports": port, "to": fmt.Sprintf("10.0.10.%d", 30+i),
"public_addresses": []string{"203.0.113.1"}, "hairpin": true}, &mr)
var f model.PortForward
if err := json.Unmarshal(mr.Object, &f); err != nil {
t.Fatal(err)
}
ids = append(ids, f.ID)
}
var mr api.MutationResponse
if code := e.call(e.token, "PATCH", "/forwards/"+itoa(ids[2]), map[string]any{"hairpin": false}, &mr); code != 200 {
t.Fatalf("update: %d", code)
}
var out struct{ Diff string }
if code := e.call(e.token, "GET", "/history/"+itoa(mr.HistoryID)+"/diff", nil, &out); code != 200 {
t.Fatalf("diff: %d", code)
}
var changed []string
for _, l := range strings.Split(out.Diff, "\n") {
if (strings.HasPrefix(l, "+") || strings.HasPrefix(l, "-")) && !strings.HasPrefix(l, "+++") && !strings.HasPrefix(l, "---") {
changed = append(changed, l)
}
}
if len(changed) != 1 || !strings.HasPrefix(changed[0], "-") || !strings.Contains(changed[0], "-hairpin") ||
!strings.Contains(out.Diff, "(before "+itoa(mr.HistoryID)+")") || !strings.Contains(out.Diff, "@@ chain srcnat") {
t.Errorf("entry %d's diff, want only the hairpin line gone, in chain srcnat:\n%s", mr.HistoryID, out.Diff)
}
}
+64 -2
View File
@@ -3,6 +3,7 @@ package server
import (
"encoding/json"
"errors"
"fmt"
"net/http"
"net/netip"
"sort"
@@ -98,11 +99,71 @@ func (s *Server) respondChange(w http.ResponseWriter, r *http.Request, p *auth.P
// rulesetDiff is the unified diff of the rulesets two configurations
// make: the file housecarl-ruleset.service loads, now and after.
func (s *Server) rulesetDiff(before, after *model.State) string {
return s.labeledDiff(before, after, "now", "after")
}
// labeledDiff is rulesetDiff with its own names for the two sides.
func (s *Server) labeledDiff(before, after *model.State, from, to string) string {
if before == nil || after == nil {
return ""
}
now, then := s.fw.Render(before).Text, s.fw.Render(after).Text
return blockHeaders(udiff.Unified("ruleset.nft (now)", "ruleset.nft (after)", now, then), now, then)
diff := hunkStarts(udiff.Unified("ruleset.nft ("+from+")", "ruleset.nft ("+to+")", now, then))
return blockHeaders(diff, now, then)
}
// hunkStarts numbers each hunk's new side again, from its old side and
// the lines the hunks before it added and removed. go-udiff (v0.4.1)
// leaves out of its count the unchanged lines between two edits of one
// hunk, so after a hunk like a moved line every later one started too
// early ("@@ -76,9 +74,9 @@" for "+76,9"), and blockHeaders named the
// chain before it.
func hunkStarts(diff string) string {
lines := strings.SplitAfter(diff, "\n")
delta := 0
for i := 0; i < len(lines); i++ {
f := strings.Fields(lines[i])
if !strings.HasPrefix(lines[i], "@@ ") || len(f) < 4 || f[3] != "@@" {
continue
}
start, _, _ := strings.Cut(f[1][1:], ",")
from, err := strconv.Atoi(start)
if err != nil {
continue
}
oldN, newN := 0, 0
for _, l := range lines[i+1:] {
if strings.HasPrefix(l, "@@ ") {
break
}
switch {
case strings.HasPrefix(l, "-"):
oldN++
case strings.HasPrefix(l, "+"):
newN++
case strings.HasPrefix(l, " "):
oldN++
newN++
}
}
// An empty side is numbered by the line before it.
first := from
if oldN == 0 {
first++
}
to := first + delta
if newN == 0 {
to--
}
side := "+" + strconv.Itoa(to)
if newN > 1 {
side += "," + strconv.Itoa(newN)
}
f[2] = side
lines[i] = strings.Join(f, " ") + "\n"
delta += newN - oldN
}
return strings.Join(lines, "")
}
// blockHeaders names, after each hunk's @@ line, the chain (or other
@@ -705,7 +766,8 @@ func (s *Server) historyDiff(w http.ResponseWriter, r *http.Request, p *auth.Pri
return err
}
}
writeJSON(w, http.StatusOK, map[string]string{"diff": s.rulesetDiff(st, after)})
before, done := fmt.Sprintf("before %d", id), fmt.Sprintf("after %d", id)
writeJSON(w, http.StatusOK, map[string]string{"diff": s.labeledDiff(st, after, before, done)})
return nil
}