The sheet handle: eviction, cell cap, and crash recovery
Open counts a tab's cells and refuses ErrTabTooLarge past MaxCellsPerTab (200,000) before doing any decrypt work, decrypts the tab, seeds Graph's cache from each cell's stored CachedSealed, and recomputes forward from any cell whose CacheVersion trails its InputVersion before serving a single read. Set writes an input and its own version bump (the dirty marker; no separate column needed) in one transaction, then recalculates dirty-forward and persists every touched cell's cache in a second. Handles are refcounted per (db, tabID); Release evicts immediately at zero, so a released handle can never mask a crash on the next Open. Watched TestStaleCacheIsRecomputedOnOpen fail for the right reason (recovery loop disabled) before implementing recovery, and watched TestSetDoesNotLeakPlaintext fail (sealing bypassed) before trusting the gate. Found and fixed a real bug during self-review: stamping a recovered cell's dependents with the stale origin's own InputVersion can regress an already-consistent dependent's CacheVersion below its own InputVersion; fixed by using one shared, provably-newer version stamp per recovery pass, pinned by TestRecoveryDoesNotRegressAConsistentDependentsVersion (confirmed to fail against the reverted buggy version). calc.Graph gains four small additive exports the handle needs and did not have (Dependents, Clear, CachedValue, SeedCache) plus an exported MaxDirtyPerPass, rather than duplicating collectRefs/rangeCells edge logic in the store package. Full reasoning, the binder decision (position+1 preserved, not migrated to real ids — carried-forward item 2) and known limitations are in task-10-report.md.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01APtNStXDZ5H2fAc1ZuGqtS
10 files changed,
+1203
−5
internal/sheets/calc/eval.go+1 −1internal/sheets/calc/graph.go+76 −2internal/sheets/calc/graph_recalc.go+2 −2internal/sheets/store/handle.go+198 −0internal/sheets/store/handle_codec.go+130 −0internal/sheets/store/handle_gate_test.go+64 −0internal/sheets/store/handle_more_test.go+226 −0internal/sheets/store/handle_open.go+156 −0internal/sheets/store/handle_set.go+176 −0internal/sheets/store/handle_test.go+174 −0
Intent
diff --git a/internal/sheets/calc/eval.go b/internal/sheets/calc/eval.go| index 7910ec8..291d0a8 100644 |
| --- a/internal/sheets/calc/eval.go |
| +++ b/internal/sheets/calc/eval.go |
| @@ -80,7 +80,7 @@ func refKey(r *Ref) CellKey { |
| // when a range-taking function evaluates it (evalCallArg), and again as |
| // dependency-graph precedent edges every time SetFormula parses that same |
| // formula (collectRefs). This is rangeCells' fan-in counterpart to |
| -// Recalc's maxDirtyPerPass, which caps the symmetric fan-out case. |
| +// Recalc's MaxDirtyPerPass, which caps the symmetric fan-out case. |
| const maxRangeCells = 200_000 |
| // rangeCells expands a Range into every CellKey it covers, normalizing a |
diff --git a/internal/sheets/calc/graph.go b/internal/sheets/calc/graph.go| index 8b881c8..3835a63 100644 |
| --- a/internal/sheets/calc/graph.go |
| +++ b/internal/sheets/calc/graph.go |
| @@ -1,12 +1,17 @@ |
| package calc |
| -// maxDirtyPerPass caps how many cells a single Recalc will touch. One |
| +// MaxDirtyPerPass caps how many cells a single Recalc will touch. One |
| // edit to a cell feeding a whole-column formula can transitively dirty |
| // thousands of dependents; past the cap Recalc stops without evaluating |
| // anything and just reports how many cells it would have touched, so the |
| // caller can tell the client to refetch its viewport instead of shipping |
| // (and the client diffing) an enormous delta. |
| -const maxDirtyPerPass = 5000 |
| +// |
| +// Exported so a caller that needs to decide up front whether a pass will |
| +// run — Task 10's handle, deciding whether to attempt a synchronous |
| +// dirty-forward recompute at all — can check against the same number |
| +// Recalc enforces, rather than keeping a second copy of it. |
| +const MaxDirtyPerPass = 5000 |
| // Graph holds the precedent/dependent edges between formula cells and a |
| // cache of their last computed value. It is what turns "this cell |
| @@ -132,3 +137,72 @@ func (g *Graph) transitiveDependents(origin CellKey) map[CellKey]struct{} { |
| } |
| return seen |
| } |
| + |
| +// Dependents returns the direct dependents of key — the cells whose |
| +// installed formula reads key, one hop, not transitive. Order is |
| +// unspecified. |
| +// |
| +// This exists for Task 10's handle: after a Recalc, the handle has to |
| +// know which cells' persisted cache rows to rewrite, and Recalc itself |
| +// deliberately returns only a touched count (it feeds the recalc |
| +// marker's derived-count field, §4.3 — not a cell list). Re-deriving |
| +// "which cells changed" by re-running collectRefs/rangeCells in the |
| +// store package would be a second implementation of edge-building this |
| +// package already gets right, including the maxRangeCells cap — exactly |
| +// the failure shape this project's own review history keeps finding. A |
| +// getter over the edges Graph already maintains avoids that: the handle |
| +// walks Dependents itself (see its own transitiveDependents) using |
| +// edges this package built and capped once. |
| +func (g *Graph) Dependents(key CellKey) []CellKey { |
| + deps := g.dependents[key] |
| + out := make([]CellKey, 0, len(deps)) |
| + for d := range deps { |
| + out = append(out, d) |
| + } |
| + return out |
| +} |
| + |
| +// Clear removes key's formula, its cached value, and its precedent |
| +// edges, exactly as SetFormula's replacement prologue does — but installs |
| +// nothing new. It is what a blanked cell uses: SetFormula only knows how |
| +// to install or replace a formula, and a cell with no content is neither. |
| +// |
| +// Dependents of key are left untouched: a cell that reads key still |
| +// depends on it after key goes blank (env.CellValue(key) now answers |
| +// empty rather than key no longer mattering to anything that reads it). |
| +func (g *Graph) Clear(key CellKey) { |
| + if old, ok := g.precedents[key]; ok { |
| + for p := range old { |
| + delete(g.dependents[p], key) |
| + } |
| + } |
| + delete(g.formulas, key) |
| + delete(g.precedents, key) |
| + delete(g.cache, key) |
| + delete(g.cached, key) |
| +} |
| + |
| +// CachedValue returns key's last computed value and true, or a zero |
| +// Value and false if nothing has been computed (or seeded) for it yet. |
| +// It never evaluates — a read-only peek at state Recalc, Value or |
| +// SeedCache already established, for a caller (Task 10's handle) that |
| +// needs to read back what a Recalc pass just computed for several cells |
| +// without re-walking each one. |
| +func (g *Graph) CachedValue(key CellKey) (Value, bool) { |
| + return g.cache[key], g.cached[key] |
| +} |
| + |
| +// SeedCache installs v as key's cached value directly, without |
| +// evaluating anything, as though a prior pass had already computed it. |
| +// |
| +// Task 10's handle uses this at Open to restore each cell's |
| +// last-known-good value, decrypted from its stored cache, before |
| +// deciding which cells (if any) are actually stale and need a real |
| +// Recalc. Without it, every formula cell would read as uncomputed after |
| +// a fresh Open — SetFormula always drops any existing cache — turning an |
| +// ordinary restart into a full-tab recompute instead of the targeted |
| +// dirty-forward one crash recovery actually needs. |
| +func (g *Graph) SeedCache(key CellKey, v Value) { |
| + g.cache[key] = v |
| + g.cached[key] = true |
| +} |
diff --git a/internal/sheets/calc/graph_recalc.go b/internal/sheets/calc/graph_recalc.go| index b7f296c..eab4402 100644 |
| --- a/internal/sheets/calc/graph_recalc.go |
| +++ b/internal/sheets/calc/graph_recalc.go |
| @@ -9,14 +9,14 @@ import "time" |
| // a formula cell — an edit to a formula's own text — in which case origin |
| // is recomputed too. |
| // |
| -// Past maxDirtyPerPass, Recalc evaluates nothing and returns the count it |
| +// Past MaxDirtyPerPass, Recalc evaluates nothing and returns the count it |
| // would have touched; see the constant's comment for why. |
| func (g *Graph) Recalc(origin CellKey, env Env) int { |
| dirty := g.transitiveDependents(origin) |
| if _, hasFormula := g.formulas[origin]; hasFormula { |
| dirty[origin] = struct{}{} |
| } |
| - if len(dirty) > maxDirtyPerPass { |
| + if len(dirty) > MaxDirtyPerPass { |
| return len(dirty) |
| } |
| g.color = make(map[CellKey]cellColor, len(dirty)) |
diff --git a/internal/sheets/store/handle.go b/internal/sheets/store/handle.go| new file mode 100644 |
| index 0000000..154f3f8 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle.go |
| @@ -0,0 +1,198 @@ |
| +// Package store's handle.go is the in-memory sheet handle: opening a tab |
| +// decrypts it into a refcounted graph, serves reads from that graph, and |
| +// recovers from a crash mid-recalculation before it serves a single one. |
| +// See handle_open.go for the load-and-recover path and handle_set.go for |
| +// the write path. |
| +package store |
| + |
| +import ( |
| + "errors" |
| + "fmt" |
| + "sync" |
| + "time" |
| + |
| + "gorm.io/gorm" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| +) |
| + |
| +// MaxCellsPerTab is a hard cap. |
| +// |
| +// The handle holds a decrypted tab plus its dependency graph in memory, so |
| +// "hold it in RAM" is only true up to a number. 200,000 keeps a tab in tens |
| +// of megabytes on a small shared box where many instances live. Google's |
| +// limit is 10M and essentially nobody reaches it, so this trades a limit |
| +// almost nobody meets for a memory ceiling everybody benefits from. Past it, |
| +// an honest error rather than a slow death. |
| +const MaxCellsPerTab = 200_000 |
| + |
| +// ErrTabTooLarge is returned by Open when a tab's cell count exceeds |
| +// MaxCellsPerTab. |
| +var ErrTabTooLarge = errors.New("store: tab exceeds MaxCellsPerTab") |
| + |
| +// cellIDs is a cell's real, stable database identity — as opposed to the |
| +// position-derived calc.CellKey the graph and every formula reference |
| +// inside it are keyed by. See handle_open.go's doc comment on the binder |
| +// for why the two are not the same numbers. |
| +type cellIDs struct{ RowID, ColID uint64 } |
| + |
| +// Handle is one tab, decrypted into memory: its dependency graph, its |
| +// cells' raw input text, and the row/col position bindings that translate |
| +// between a cell's real (row_id, col_id) and the graph's key space. A |
| +// Handle is not safe for concurrent use from multiple goroutines without |
| +// its own lock — which is exactly what mu provides; every exported method |
| +// takes it. |
| +type Handle struct { |
| + mu sync.Mutex |
| + db *gorm.DB |
| + sealer *seal.Sealer |
| + tabID uint64 |
| + graph *calc.Graph |
| + |
| + rowPos map[uint64]int // real Row.ID -> Position |
| + colPos map[uint64]int // real Col.ID -> Position |
| + |
| + rawText map[calc.CellKey]string // decrypted input text; in-memory only, never persisted as-is |
| + keyToIDs map[calc.CellKey]cellIDs // graph key -> the real ids that own it |
| + |
| + nextVersion uint64 // next input/cache-version stamp this handle will hand out |
| + |
| + regKey handleKey |
| + refCount int |
| +} |
| + |
| +// handleKey scopes the process-wide handle registry to a specific |
| +// database as well as a tab id — otherwise two tests (or, in principle, |
| +// two differently-configured stores in the same process) opening "tab 1" |
| +// against different *gorm.DB values would collide on the same cached |
| +// handle. |
| +type handleKey struct { |
| + db *gorm.DB |
| + tabID uint64 |
| +} |
| + |
| +var ( |
| + registryMu sync.Mutex |
| + registry = map[handleKey]*Handle{} |
| +) |
| + |
| +// Open returns the refcounted handle for tabID, building it from db if no |
| +// live handle exists yet, or incrementing the refcount and returning the |
| +// existing one if it does. Every Open must be matched with a Release. |
| +// |
| +// Open counts the tab's cells before doing anything else — refusing a |
| +// tab past MaxCellsPerTab is the one check cheap enough to run unconditionally, |
| +// and it must run before any decrypt work, or the cap protects nothing. |
| +// If the tab is under the cap, Open decrypts it, builds the dependency |
| +// graph, and recalculates forward from any cell whose CacheVersion trails |
| +// its InputVersion — see handle_open.go — before this call returns, so no |
| +// caller of Value ever observes a cache a crash left behind. |
| +func Open(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error) { |
| + var count int64 |
| + if err := db.Model(&sheets.Cell{}).Where("tab_id = ?", tabID).Count(&count).Error; err != nil { |
| + return nil, fmt.Errorf("store: count cells for tab: %w", err) |
| + } |
| + if count > MaxCellsPerTab { |
| + return nil, ErrTabTooLarge |
| + } |
| + |
| + key := handleKey{db: db, tabID: tabID} |
| + |
| + registryMu.Lock() |
| + defer registryMu.Unlock() |
| + |
| + if h, ok := registry[key]; ok { |
| + h.refCount++ |
| + return h, nil |
| + } |
| + |
| + h, err := buildHandle(db, sealer, tabID) |
| + if err != nil { |
| + return nil, err |
| + } |
| + h.regKey = key |
| + h.refCount = 1 |
| + registry[key] = h |
| + return h, nil |
| +} |
| + |
| +// Release decrements the handle's refcount. At zero, the handle is idle |
| +// and is evicted immediately: the next Open for this tab rebuilds it from |
| +// the database rather than resuming a stale in-memory copy, which is what |
| +// lets a process restart (or, in a test, a deliberate simulated crash |
| +// between Release and the next Open) be observed truthfully. |
| +func (h *Handle) Release() { |
| + registryMu.Lock() |
| + defer registryMu.Unlock() |
| + h.refCount-- |
| + if h.refCount <= 0 { |
| + delete(registry, h.regKey) |
| + } |
| +} |
| + |
| +// Value returns key's current value: whatever the graph has cached for |
| +// it, or an empty Value for a cell with no content (never set, blanked, |
| +// or referencing a row/col this tab doesn't have). It never evaluates — |
| +// Open and Set are what keep the graph's cache current, so a read is |
| +// always just a lookup. |
| +func (h *Handle) Value(rowID, colID uint64) calc.Value { |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + key, ok := h.cellKey(rowID, colID) |
| + if !ok { |
| + return calc.Value{} |
| + } |
| + v, _ := h.graph.CachedValue(key) |
| + return v |
| +} |
| + |
| +// Format returns the interned Format a cell uses, or nil if the cell has |
| +// no formatting (or does not exist). Unlike Value, this reads straight |
| +// through to the database rather than through the in-memory graph: format |
| +// assignment is comparatively rare and Format rows are already interned |
| +// (models.go), so there is no equivalent cache to keep warm here. |
| +func (h *Handle) Format(rowID, colID uint64) *sheets.Format { |
| + var cell sheets.Cell |
| + if err := h.db.Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID). |
| + First(&cell).Error; err != nil { |
| + return nil |
| + } |
| + if cell.FormatID == nil { |
| + return nil |
| + } |
| + var f sheets.Format |
| + if err := h.db.First(&f, *cell.FormatID).Error; err != nil { |
| + return nil |
| + } |
| + return &f |
| +} |
| + |
| +// cellKey translates a cell's real, stable (row_id, col_id) into the |
| +// graph's position-derived CellKey space. false means rowID or colID does |
| +// not belong to this tab (or this handle has not loaded it). |
| +func (h *Handle) cellKey(rowID, colID uint64) (calc.CellKey, bool) { |
| + rp, ok := h.rowPos[rowID] |
| + if !ok { |
| + return calc.CellKey{}, false |
| + } |
| + cp, ok := h.colPos[colID] |
| + if !ok { |
| + return calc.CellKey{}, false |
| + } |
| + return calc.CellKey{RowID: uint64(rp + 1), ColID: uint64(cp + 1)}, true |
| +} |
| + |
| +// handleEnv is calc.Env pinned to one instant for the lifetime of a |
| +// single Recalc pass — "one pass sees one instant" (calc.Env's own |
| +// doc) — and otherwise answering empty for any cell the graph does not |
| +// itself intercept, since every cell this handle tracks with content is |
| +// registered in the graph (see contentNode); a CellValue call reaching |
| +// this fallback means the cell has none. |
| +type handleEnv struct{ now time.Time } |
| + |
| +func newHandleEnv() handleEnv { return handleEnv{now: time.Now().UTC()} } |
| + |
| +func (e handleEnv) CellValue(calc.CellKey) calc.Value { return calc.Value{} } |
| +func (e handleEnv) Now() time.Time { return e.now } |
diff --git a/internal/sheets/store/handle_codec.go b/internal/sheets/store/handle_codec.go| new file mode 100644 |
| index 0000000..d271430 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_codec.go |
| @@ -0,0 +1,130 @@ |
| +package store |
| + |
| +import ( |
| + "encoding/binary" |
| + "errors" |
| + "fmt" |
| + "math" |
| + "strconv" |
| + "strings" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| +) |
| + |
| +// contentNode classifies a cell's raw input the way the grid does: empty |
| +// text is a blank cell, text starting with "=" is a formula (parsed with |
| +// the leading "=" stripped, matching calc.Parse's own contract), and |
| +// anything else is a literal — a number, a bare TRUE/FALSE, or otherwise |
| +// text. |
| +// |
| +// Every non-blank result is a calc.Node, so Set and Open register literal |
| +// and formula cells with the graph the same way (Graph.SetFormula), |
| +// letting Graph's own caching and dependency tracking do the work for |
| +// both instead of the handle keeping a second, parallel path for values |
| +// that happen not to reference anything. |
| +func contentNode(raw string) (node calc.Node, blank bool, err error) { |
| + if raw == "" { |
| + return nil, true, nil |
| + } |
| + if strings.HasPrefix(raw, "=") { |
| + n, perr := calc.Parse(raw[1:]) |
| + if perr != nil { |
| + return nil, false, perr |
| + } |
| + return n, false, nil |
| + } |
| + return literalNode(raw), false, nil |
| +} |
| + |
| +// literalNode parses raw the way a spreadsheet's own input coercion |
| +// would: a number if it parses as one, TRUE/FALSE (case-insensitive) as |
| +// a boolean, otherwise verbatim text. |
| +func literalNode(raw string) calc.Node { |
| + if n, err := strconv.ParseFloat(raw, 64); err == nil { |
| + return &calc.Num{V: n} |
| + } |
| + if strings.EqualFold(raw, "TRUE") { |
| + return &calc.Bool{V: true} |
| + } |
| + if strings.EqualFold(raw, "FALSE") { |
| + return &calc.Bool{V: false} |
| + } |
| + return &calc.Str{V: raw} |
| +} |
| + |
| +// encodeValue and decodeValue are CachedSealed's plaintext codec — sealed |
| +// under purpose "cell.cached" before this ever reaches disk, never |
| +// written or read anywhere else. Cell values are IEEE doubles end to end |
| +// (spec §5): a number is stored as its raw 8-byte bit pattern rather than |
| +// a decimal reformatting, so it survives round-trip even for the values a |
| +// text codec cannot represent — encoding/json, for one, rejects NaN and |
| +// Inf outright, and this evaluator has at least one path (POWER with a |
| +// fractional exponent on a negative base) that can produce a KindNum NaN |
| +// with no earlier guard against it reaching this codec. |
| +func encodeValue(v calc.Value) []byte { |
| + switch v.Kind { |
| + case calc.KindNum: |
| + b := make([]byte, 9) |
| + b[0] = byte(calc.KindNum) |
| + binary.BigEndian.PutUint64(b[1:], math.Float64bits(v.Num)) |
| + return b |
| + case calc.KindStr: |
| + b := make([]byte, 5+len(v.Str)) |
| + b[0] = byte(calc.KindStr) |
| + binary.BigEndian.PutUint32(b[1:5], uint32(len(v.Str))) |
| + copy(b[5:], v.Str) |
| + return b |
| + case calc.KindBool: |
| + bit := byte(0) |
| + if v.Bool { |
| + bit = 1 |
| + } |
| + return []byte{byte(calc.KindBool), bit} |
| + case calc.KindErr: |
| + return []byte{byte(calc.KindErr), byte(v.Err)} |
| + default: // calc.KindEmpty |
| + return []byte{byte(calc.KindEmpty)} |
| + } |
| +} |
| + |
| +// decodeValue is encodeValue's inverse. An empty byte slice — the shape a |
| +// freshly-created, never-recomputed cell's CachedSealed would seal — also |
| +// decodes to KindEmpty rather than erroring. |
| +func decodeValue(b []byte) (calc.Value, error) { |
| + if len(b) == 0 { |
| + return calc.Value{}, nil |
| + } |
| + kind := calc.ValueKind(b[0]) |
| + rest := b[1:] |
| + switch kind { |
| + case calc.KindEmpty: |
| + return calc.Value{}, nil |
| + case calc.KindNum: |
| + if len(rest) < 8 { |
| + return calc.Value{}, errors.New("store: truncated cached number") |
| + } |
| + return calc.Value{Kind: calc.KindNum, Num: math.Float64frombits(binary.BigEndian.Uint64(rest))}, nil |
| + case calc.KindStr: |
| + if len(rest) < 4 { |
| + return calc.Value{}, errors.New("store: truncated cached string length") |
| + } |
| + n := binary.BigEndian.Uint32(rest[:4]) |
| + rest = rest[4:] |
| + if uint32(len(rest)) < n { |
| + return calc.Value{}, errors.New("store: truncated cached string") |
| + } |
| + return calc.Value{Kind: calc.KindStr, Str: string(rest[:n])}, nil |
| + case calc.KindBool: |
| + if len(rest) < 1 { |
| + return calc.Value{}, errors.New("store: truncated cached bool") |
| + } |
| + return calc.Value{Kind: calc.KindBool, Bool: rest[0] != 0}, nil |
| + case calc.KindErr: |
| + if len(rest) < 1 { |
| + return calc.Value{}, errors.New("store: truncated cached error") |
| + } |
| + return calc.Value{Kind: calc.KindErr, Err: calc.ErrKind(rest[0])}, nil |
| + default: |
| + return calc.Value{}, fmt.Errorf("store: unknown cached value kind %d", kind) |
| + } |
| +} |
diff --git a/internal/sheets/store/handle_gate_test.go b/internal/sheets/store/handle_gate_test.go| new file mode 100644 |
| index 0000000..03fbca2 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_gate_test.go |
| @@ -0,0 +1,64 @@ |
| +package store_test |
| + |
| +import ( |
| + "path/filepath" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| + "amadan.net/oficina/sheets/internal/sheets/sealtest" |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +// The handle is the busiest writer in the app: every edit goes through |
| +// Set, which seals an input, a recalculated cache and an event payload |
| +// in one call. This is the most valuable place to run the plaintext |
| +// gate, per the task brief — it exercises the real write path Set takes, |
| +// not a synthetic insert standing in for it. |
| +// |
| +// This test builds its own db (rather than reusing seeded()) so it keeps |
| +// the file path in scope for the gate — seeded()'s signature is pinned to |
| +// the brief's own TestStaleCacheIsRecomputedOnOpen snippet and does not |
| +// return one. |
| +func TestSetDoesNotLeakPlaintext(t *testing.T) { |
| + dir := t.TempDir() |
| + dbPath := filepath.Join(dir, "test.db") |
| + db := openTestDB(t, dbPath) |
| + if err := db.AutoMigrate( |
| + &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{}, |
| + &sheets.Cell{}, &sheets.Format{}, |
| + ); err != nil { |
| + t.Fatalf("migrate: %v", err) |
| + } |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + const gateTabID, gateRow, gateCol uint64 = 1, 1, 1 |
| + if err := db.Create(&sheets.Tab{ID: gateTabID, SheetID: 1}).Error; err != nil { |
| + t.Fatalf("seed tab: %v", err) |
| + } |
| + if err := db.Create(&sheets.Row{ID: gateRow, TabID: gateTabID, Position: 0}).Error; err != nil { |
| + t.Fatalf("seed row: %v", err) |
| + } |
| + if err := db.Create(&sheets.Col{ID: gateCol, TabID: gateTabID, Position: 0}).Error; err != nil { |
| + t.Fatalf("seed col: %v", err) |
| + } |
| + |
| + h, err := store.Open(db, s, gateTabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + // sealtest.Canaries are strings AssertNoPlaintext greps the raw file |
| + // for; seeding one through Set — as a literal cell input, the shape |
| + // user content actually takes — is what proves the real write path |
| + // seals it, rather than a stand-in insert bypassing it. |
| + if err := h.Set(gateRow, gateCol, string(sealtest.Canaries[0]), actor); err != nil { |
| + t.Fatalf("Set: %v", err) |
| + } |
| + h.Release() |
| + |
| + checkpoint(t, db) |
| + sealtest.AssertNoPlaintext(t, dbPath) |
| +} |
diff --git a/internal/sheets/store/handle_more_test.go b/internal/sheets/store/handle_more_test.go| new file mode 100644 |
| index 0000000..1f663d9 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_more_test.go |
| @@ -0,0 +1,226 @@ |
| +package store_test |
| + |
| +import ( |
| + "path/filepath" |
| + "testing" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +// Set on an id pair this tab has no row or column for must fail rather |
| +// than silently writing a cell nothing addresses. |
| +func TestSetOnUnknownCellFails(t *testing.T) { |
| + db, sealer, tabID := seeded(t) |
| + h, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + defer h.Release() |
| + |
| + if err := h.Set(999999, colA, "1", actor); err == nil { |
| + t.Fatal("Set against an unknown row id: want error, got nil") |
| + } |
| +} |
| + |
| +// Blanking a cell deletes its row (cells are sparse — models.go) and |
| +// propagates the change to anything that referenced it. |
| +func TestSetBlankDeletesRowAndPropagates(t *testing.T) { |
| + db, sealer, tabID := seeded(t) |
| + h, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + defer h.Release() |
| + |
| + if err := h.Set(rowB, colA, "=A1*2", actor); err != nil { |
| + t.Fatalf("Set B1: %v", err) |
| + } |
| + if got := h.Value(rowB, colA); got.Kind != calc.KindNum || got.Num != 10 { |
| + t.Fatalf("B1 before blanking = %+v, want 10", got) |
| + } |
| + |
| + if err := h.Set(rowA, colA, "", actor); err != nil { |
| + t.Fatalf("Set A1 blank: %v", err) |
| + } |
| + |
| + var count int64 |
| + db.Model(&sheets.Cell{}).Where("row_id = ? AND col_id = ?", rowA, colA).Count(&count) |
| + if count != 0 { |
| + t.Fatalf("A1's row still exists after blanking; cells must be sparse") |
| + } |
| + |
| + // Arithmetic against an empty cell reads it as 0 (calc's own |
| + // convention), so B1 = A1*2 becomes 0, not an error or a stale 10. |
| + if got := h.Value(rowB, colA); got.Kind != calc.KindNum || got.Num != 0 { |
| + t.Fatalf("B1 after blanking A1 = %+v, want KindNum 0", got) |
| + } |
| +} |
| + |
| +// Format returns the interned format a cell's FormatID points at, and nil |
| +// for a cell with none. |
| +func TestFormatReturnsInternedFormat(t *testing.T) { |
| + db, sealer, tabID := seeded(t) |
| + h, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + defer h.Release() |
| + |
| + if f := h.Format(rowA, colA); f != nil { |
| + t.Fatalf("A1 has no FormatID; want nil, got %+v", f) |
| + } |
| + |
| + fmtRow := &sheets.Format{Digest: "test-digest", NumberFmt: "0.00"} |
| + if err := db.Create(fmtRow).Error; err != nil { |
| + t.Fatalf("create format: %v", err) |
| + } |
| + if err := db.Model(&sheets.Cell{}). |
| + Where("row_id = ? AND col_id = ?", rowA, colA). |
| + Update("format_id", fmtRow.ID).Error; err != nil { |
| + t.Fatalf("assign format: %v", err) |
| + } |
| + |
| + got := h.Format(rowA, colA) |
| + if got == nil || got.NumberFmt != "0.00" { |
| + t.Fatalf("Format(A1) = %+v, want NumberFmt 0.00", got) |
| + } |
| +} |
| + |
| +// A released handle is evicted immediately: the next Open rebuilds from |
| +// the database rather than resuming stale in-memory state. Two live |
| +// Opens for the same tab share one handle and its refcount. |
| +func TestReleaseEvictsAndReopenSeesFreshState(t *testing.T) { |
| + db, sealer, tabID := seeded(t) |
| + |
| + h1, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + h1b, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("second Open: %v", err) |
| + } |
| + if h1 != h1b { |
| + t.Fatal("two Opens for the same live tab returned different handles; refcounted sharing is broken") |
| + } |
| + h1b.Release() // refcount back to 1; h1 must still be usable |
| + |
| + if got := h1.Value(rowA, colA); got.Kind != calc.KindNum || got.Num != 5 { |
| + t.Fatalf("A1 via still-open handle = %+v, want 5", got) |
| + } |
| + h1.Release() // refcount to 0: evicted |
| + |
| + // Mutate directly, bypassing the handle entirely — a released handle |
| + // must not still be answering Value() from stale memory, and the next |
| + // Open must reflect this write. |
| + db.Model(&sheets.Cell{}). |
| + Where("row_id = ? AND col_id = ?", rowA, colA). |
| + Updates(map[string]any{"input_version": 5, "cache_version": 5}) |
| + |
| + h2, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("reopen: %v", err) |
| + } |
| + defer h2.Release() |
| + if h2 == h1 { |
| + t.Fatal("reopen after full release returned the evicted handle instead of a fresh one") |
| + } |
| +} |
| + |
| +// A dependent that is perfectly consistent on its own (its CacheVersion |
| +// already matches its own InputVersion) can still get swept into a stale |
| +// PRECEDENT's recovery pass — Open's recovery walks forward from a stale |
| +// origin to every transitive dependent, regardless of whether each one |
| +// was independently flagged. Stamping such a dependent with the stale |
| +// origin's OWN (possibly much older) InputVersion would regress its |
| +// CacheVersion below its own InputVersion, manufacturing a fresh — if |
| +// self-correcting — false positive on the very next Open. This pins that |
| +// recovery uses one version stamp for the whole pass (newer than every |
| +// version it read) rather than the triggering cell's own, so a |
| +// consistent dependent's version can only advance, never regress. |
| +func TestRecoveryDoesNotRegressAConsistentDependentsVersion(t *testing.T) { |
| + dir := t.TempDir() |
| + db := openTestDB(t, filepath.Join(dir, "test.db")) |
| + if err := db.AutoMigrate( |
| + &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{}, &sheets.Cell{}, |
| + ); err != nil { |
| + t.Fatalf("migrate: %v", err) |
| + } |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + const localTab, localRowA, localRowC, localCol uint64 = 1, 1, 2, 1 |
| + if err := db.Create(&sheets.Tab{ID: localTab, SheetID: 1}).Error; err != nil { |
| + t.Fatalf("seed tab: %v", err) |
| + } |
| + if err := db.Create([]*sheets.Row{ |
| + {ID: localRowA, TabID: localTab, Position: 0}, |
| + {ID: localRowC, TabID: localTab, Position: 1}, |
| + }).Error; err != nil { |
| + t.Fatalf("seed rows: %v", err) |
| + } |
| + if err := db.Create(&sheets.Col{ID: localCol, TabID: localTab, Position: 0}).Error; err != nil { |
| + t.Fatalf("seed col: %v", err) |
| + } |
| + |
| + sealInput := func(raw string) []byte { |
| + b, err := s.Seal("cell.input", []byte(raw)) |
| + if err != nil { |
| + t.Fatalf("seal input: %v", err) |
| + } |
| + return b |
| + } |
| + blankCache, err := s.Seal("cell.cached", nil) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + // A1: consistent at rest (InputVersion 5 == CacheVersion 5). |
| + if err := db.Create(&sheets.Cell{ |
| + TabID: localTab, RowID: localRowA, ColID: localCol, |
| + InputSealed: sealInput("1"), InputVersion: 5, |
| + CachedSealed: blankCache, CacheVersion: 5, |
| + }).Error; err != nil { |
| + t.Fatalf("seed A1: %v", err) |
| + } |
| + // C1 (at row localRowC): also consistent at rest, and at a HIGHER |
| + // version than A1 — the shape a formula edited well after its |
| + // precedent takes. |
| + if err := db.Create(&sheets.Cell{ |
| + TabID: localTab, RowID: localRowC, ColID: localCol, |
| + InputSealed: sealInput("=A1+1"), InputVersion: 10, |
| + CachedSealed: blankCache, CacheVersion: 10, |
| + }).Error; err != nil { |
| + t.Fatalf("seed C1: %v", err) |
| + } |
| + |
| + // Simulate a crash that bumped A1's InputVersion without rewriting |
| + // C1's cache — the same shape as the brief's own crash test, but with |
| + // the bump smaller than C1's already-consistent version, which is |
| + // exactly what exposes a stamp-regression bug. |
| + if err := db.Model(&sheets.Cell{}). |
| + Where("row_id = ? AND col_id = ?", localRowA, localCol). |
| + Update("input_version", 6).Error; err != nil { |
| + t.Fatalf("tamper A1: %v", err) |
| + } |
| + |
| + h, err := store.Open(db, s, localTab) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + defer h.Release() |
| + |
| + var c sheets.Cell |
| + db.Where("row_id = ? AND col_id = ?", localRowC, localCol).First(&c) |
| + if c.CacheVersion < c.InputVersion { |
| + t.Fatalf("C1 regressed: CacheVersion %d < its own InputVersion %d", c.CacheVersion, c.InputVersion) |
| + } |
| + if got := h.Value(localRowC, localCol); got.Kind != calc.KindNum || got.Num != 2 { |
| + t.Fatalf("C1 = %+v, want KindNum 2 (A1=1, C1=A1+1)", got) |
| + } |
| +} |
diff --git a/internal/sheets/store/handle_open.go b/internal/sheets/store/handle_open.go| new file mode 100644 |
| index 0000000..8560ba3 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_open.go |
| @@ -0,0 +1,156 @@ |
| +package store |
| + |
| +import ( |
| + "fmt" |
| + |
| + "gorm.io/gorm" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| +) |
| + |
| +// The binder, and a deliberate choice about it (task 10 carry-forward |
| +// item 2): calc.CellKey is generic — {RowID, ColID uint64} — but every |
| +// reference calc itself resolves (refKey, rangeCells, and the lookup |
| +// functions built directly on RowPos/ColPos) computes it as position+1, |
| +// unconditionally, deep inside Parse/Eval, not just at the one placeholder |
| +// site Task 7 flagged. Migrating that to real Row.ID/Col.ID would mean |
| +// rewriting calc's own evaluator, not just wiring a translation layer in |
| +// front of it — out of reach for a task scoped to the store package, and |
| +// a much larger, riskier change than "bind ids" suggests on its own. |
| +// |
| +// So this handle PRESERVES the position+1 convention rather than |
| +// migrating it: cellKey (handle.go) maps a cell's real, stable |
| +// (row_id, col_id) — the Cell table's actual primary key, and what Set's |
| +// and Format's callers pass in — to the position-derived CellKey the |
| +// graph and every parsed formula already use internally. rowPos/colPos |
| +// below are that binding table, built fresh from the tab's Row/Col |
| +// records every time a handle is built. |
| +// |
| +// The consequence to carry forward: because the graph's key space is |
| +// POSITION, not identity, it is only valid for as long as positions do |
| +// not move. Nothing in this task inserts, deletes or reorders rows or |
| +// columns — Set only ever edits the content of a row/col pair that |
| +// already exists — so the graph never goes stale against reality within |
| +// this task's scope. But calc's evaluator has no notion of "the row that |
| +// used to be at position 4" once something IS inserted at position 2 and |
| +// old position 4 becomes 5: refKey and rangeCells will silently compute |
| +// the WRONG CellKey for that formula after such an edit. Whichever task |
| +// next adds row/col insert, delete or reorder must not patch around this; |
| +// it must either (a) rebuild this handle's graph from scratch after every |
| +// structural edit (discard and re-run buildHandle), which is correct but |
| +// means a structural edit is as expensive as a cold open, or (b) migrate |
| +// calc's evaluator to real ids, which is the more invasive but more |
| +// durable fix. This handle does neither because it needs neither yet. |
| +func buildHandle(db *gorm.DB, sealer *seal.Sealer, tabID uint64) (*Handle, error) { |
| + var rows []sheets.Row |
| + if err := db.Where("tab_id = ?", tabID).Find(&rows).Error; err != nil { |
| + return nil, fmt.Errorf("store: load rows: %w", err) |
| + } |
| + var cols []sheets.Col |
| + if err := db.Where("tab_id = ?", tabID).Find(&cols).Error; err != nil { |
| + return nil, fmt.Errorf("store: load cols: %w", err) |
| + } |
| + var cells []sheets.Cell |
| + if err := db.Where("tab_id = ?", tabID).Find(&cells).Error; err != nil { |
| + return nil, fmt.Errorf("store: load cells: %w", err) |
| + } |
| + |
| + h := &Handle{ |
| + db: db, |
| + sealer: sealer, |
| + tabID: tabID, |
| + graph: calc.NewGraph(), |
| + rowPos: make(map[uint64]int, len(rows)), |
| + colPos: make(map[uint64]int, len(cols)), |
| + rawText: make(map[calc.CellKey]string, len(cells)), |
| + keyToIDs: make(map[calc.CellKey]cellIDs, len(cells)), |
| + } |
| + for _, r := range rows { |
| + h.rowPos[r.ID] = r.Position |
| + } |
| + for _, c := range cols { |
| + h.colPos[c.ID] = c.Position |
| + } |
| + |
| + var stale []calc.CellKey |
| + var maxVersion uint64 |
| + |
| + for _, c := range cells { |
| + key, ok := h.cellKey(c.RowID, c.ColID) |
| + if !ok { |
| + continue // orphaned: its row or col record is missing from this tab |
| + } |
| + plain, err := sealer.Open("cell.input", c.InputSealed) |
| + if err != nil { |
| + return nil, fmt.Errorf("store: decrypt input at row %d col %d: %w", c.RowID, c.ColID, err) |
| + } |
| + raw := string(plain) |
| + node, blank, perr := contentNode(raw) |
| + if perr != nil { |
| + return nil, fmt.Errorf("store: parse stored formula at row %d col %d: %w", c.RowID, c.ColID, perr) |
| + } |
| + h.rawText[key] = raw |
| + if blank { |
| + continue // sparse cells should never be blank, but tolerate it rather than crash |
| + } |
| + h.graph.SetFormula(key, node) |
| + h.keyToIDs[key] = cellIDs{RowID: c.RowID, ColID: c.ColID} |
| + |
| + // Restore the pre-open cache so a clean cell need not be recomputed |
| + // merely because the process restarted. A decrypt or decode |
| + // failure here is treated as "not cached" rather than aborting |
| + // Open outright: the cache is a derived optimization, always |
| + // reconstructable from the input, and is itself flagged stale |
| + // below so it gets a real recompute rather than serving nothing. |
| + cacheOK := false |
| + if cachedPlain, err := sealer.Open("cell.cached", c.CachedSealed); err == nil { |
| + if v, err := decodeValue(cachedPlain); err == nil { |
| + h.graph.SeedCache(key, v) |
| + cacheOK = true |
| + } |
| + } |
| + |
| + if c.InputVersion > maxVersion { |
| + maxVersion = c.InputVersion |
| + } |
| + if c.CacheVersion > maxVersion { |
| + maxVersion = c.CacheVersion |
| + } |
| + if !cacheOK || c.CacheVersion < c.InputVersion { |
| + stale = append(stale, key) |
| + } |
| + } |
| + h.nextVersion = maxVersion + 1 |
| + |
| + // Crash recovery: any cell whose CacheVersion trails its InputVersion |
| + // (or whose cache failed to restore above) is recomputed — origin and |
| + // every transitive dependent — and persisted BEFORE this function |
| + // returns, so no caller of Value ever observes what a crash left |
| + // behind. |
| + // |
| + // Every pass in this loop shares ONE version stamp: h.nextVersion, the |
| + // same value the very next Set on this handle would otherwise hand |
| + // out. That is deliberate, not merely simpler than stamping each pass |
| + // with the origin cell's own InputVersion: this loop can touch a cell |
| + // as a DEPENDENT of more than one stale origin, or as a dependent of |
| + // one stale origin despite being perfectly consistent on its own |
| + // (CacheVersion already equal to its own InputVersion). Stamping such |
| + // a cell with an origin's InputVersion — which can be OLDER than the |
| + // dependent's own InputVersion — would silently regress its |
| + // CacheVersion below a value it already legitimately holds, manufacturing |
| + // a fresh (if self-correcting) false positive on the very next Open. |
| + // A single stamp taken from h.nextVersion is, by construction, greater |
| + // than every InputVersion loaded above, so it can never regress |
| + // anything it touches. |
| + recoveryVersion := h.nextVersion |
| + h.nextVersion++ |
| + for _, key := range stale { |
| + if err := h.recalcAndPersist(key, recoveryVersion, true); err != nil { |
| + return nil, fmt.Errorf("store: crash recovery: %w", err) |
| + } |
| + } |
| + |
| + return h, nil |
| +} |
diff --git a/internal/sheets/store/handle_set.go b/internal/sheets/store/handle_set.go| new file mode 100644 |
| index 0000000..010fdf5 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_set.go |
| @@ -0,0 +1,176 @@ |
| +package store |
| + |
| +import ( |
| + "fmt" |
| + |
| + "gorm.io/gorm" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| +) |
| + |
| +// Set writes a cell's new raw input and recalculates forward from it. |
| +// |
| +// The input write, plus the version bump that IS the dirty marker |
| +// (models.go: CacheVersion records the InputVersion its cache was |
| +// derived from — bumping InputVersion here without yet touching |
| +// CacheVersion is exactly what makes this cell look dirty), lands in one |
| +// transaction. Recalculation — and persisting what it computes — happens |
| +// afterward, in a second transaction. A crash between the two leaves |
| +// precisely the state Open's crash-recovery pass exists to detect and |
| +// fix: no separate marker column is needed because the version gap IS |
| +// the marker, and clearing it is writing a CacheVersion that has caught |
| +// back up. |
| +func (h *Handle) Set(rowID, colID uint64, raw string, actor uint64) error { |
| + h.mu.Lock() |
| + defer h.mu.Unlock() |
| + |
| + key, ok := h.cellKey(rowID, colID) |
| + if !ok { |
| + return fmt.Errorf("store: row %d col %d is not part of tab %d", rowID, colID, h.tabID) |
| + } |
| + |
| + node, blank, err := contentNode(raw) |
| + if err != nil { |
| + return err |
| + } |
| + |
| + v := h.nextVersion |
| + prevRaw := h.rawText[key] |
| + |
| + if blank { |
| + if err := h.db.Transaction(func(tx *gorm.DB) error { |
| + if err := tx.Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID). |
| + Delete(&sheets.Cell{}).Error; err != nil { |
| + return err |
| + } |
| + return AppendInput(tx, h.sealer, InputEvent{ |
| + TabID: h.tabID, RowID: rowID, ColID: colID, ActorID: actor, |
| + OldRaw: prevRaw, NewRaw: raw, |
| + }) |
| + }); err != nil { |
| + return err |
| + } |
| + h.nextVersion++ |
| + delete(h.rawText, key) |
| + delete(h.keyToIDs, key) |
| + h.graph.Clear(key) |
| + // Blanked: no row survives to carry a cache version, so origin |
| + // itself is excluded from the persist pass below — only its |
| + // dependents (still referencing an now-empty cell) need one. |
| + return h.recalcAndPersist(key, v, false) |
| + } |
| + |
| + sealedInput, err := h.sealer.Seal("cell.input", []byte(raw)) |
| + if err != nil { |
| + return err |
| + } |
| + if err := h.db.Transaction(func(tx *gorm.DB) error { |
| + res := tx.Model(&sheets.Cell{}). |
| + Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, rowID, colID). |
| + Updates(map[string]any{ |
| + "input_sealed": sealedInput, |
| + "input_version": v, |
| + "updated_by": actor, |
| + }) |
| + if res.Error != nil { |
| + return res.Error |
| + } |
| + if res.RowsAffected == 0 { |
| + if err := tx.Create(&sheets.Cell{ |
| + TabID: h.tabID, RowID: rowID, ColID: colID, |
| + InputSealed: sealedInput, InputVersion: v, |
| + UpdatedBy: actor, |
| + }).Error; err != nil { |
| + return err |
| + } |
| + } |
| + return AppendInput(tx, h.sealer, InputEvent{ |
| + TabID: h.tabID, RowID: rowID, ColID: colID, ActorID: actor, |
| + OldRaw: prevRaw, NewRaw: raw, |
| + }) |
| + }); err != nil { |
| + return err |
| + } |
| + h.nextVersion++ |
| + h.rawText[key] = raw |
| + h.graph.SetFormula(key, node) |
| + h.keyToIDs[key] = cellIDs{RowID: rowID, ColID: colID} |
| + |
| + return h.recalcAndPersist(key, v, true) |
| +} |
| + |
| +// recalcAndPersist propagates a change at origin to its transitive |
| +// dependents, evaluates every touched cell exactly once (via Graph.Recalc), |
| +// and writes each one's new CachedSealed/CacheVersion in a single |
| +// transaction — the step that closes the dirty marker Set (or Open's |
| +// recovery pass) opened. includeOrigin is false only for a cell that was |
| +// just blanked: its row no longer exists to receive a cache write. |
| +// |
| +// Past calc.MaxDirtyPerPass, this deliberately does nothing: the touched |
| +// set is computed BEFORE Recalc is called specifically so this can be |
| +// decided without running Recalc at all, matching (not duplicating) |
| +// Recalc's own cap — see calc.MaxDirtyPerPass's comment for why an |
| +// over-wide pass is refused rather than run. |
| +func (h *Handle) recalcAndPersist(origin calc.CellKey, v uint64, includeOrigin bool) error { |
| + touched := h.transitiveDependents(origin) |
| + if includeOrigin { |
| + touched[origin] = struct{}{} |
| + } |
| + if len(touched) == 0 { |
| + return nil |
| + } |
| + if len(touched) > calc.MaxDirtyPerPass { |
| + return nil |
| + } |
| + |
| + h.graph.Recalc(origin, newHandleEnv()) |
| + |
| + return h.db.Transaction(func(tx *gorm.DB) error { |
| + for key := range touched { |
| + ids, ok := h.keyToIDs[key] |
| + if !ok { |
| + continue // defensive: every touched key was registered via SetFormula |
| + } |
| + val, _ := h.graph.CachedValue(key) |
| + sealedCache, err := h.sealer.Seal("cell.cached", encodeValue(val)) |
| + if err != nil { |
| + return err |
| + } |
| + if err := tx.Model(&sheets.Cell{}). |
| + Where("tab_id = ? AND row_id = ? AND col_id = ?", h.tabID, ids.RowID, ids.ColID). |
| + Updates(map[string]any{ |
| + "cached_sealed": sealedCache, |
| + "cache_version": v, |
| + }).Error; err != nil { |
| + return err |
| + } |
| + } |
| + // triggerID 0: AppendInput does not hand back the id of the event |
| + // row it just created, and correlating one would cost a second |
| + // query for bookkeeping only — the marker's real job, the |
| + // derived-count, is unaffected. See events.go. |
| + return AppendRecalc(tx, 0, len(touched)) |
| + }) |
| +} |
| + |
| +// transitiveDependents mirrors calc.Graph's own private BFS of the same |
| +// name, walking Graph.Dependents instead of an unexported edge map — see |
| +// that method's doc for why the handle asks Graph for edges rather than |
| +// rebuilding them. |
| +func (h *Handle) transitiveDependents(origin calc.CellKey) map[calc.CellKey]struct{} { |
| + seen := make(map[calc.CellKey]struct{}) |
| + queue := []calc.CellKey{origin} |
| + for len(queue) > 0 { |
| + cur := queue[0] |
| + queue = queue[1:] |
| + for _, dep := range h.graph.Dependents(cur) { |
| + if _, ok := seen[dep]; ok { |
| + continue |
| + } |
| + seen[dep] = struct{}{} |
| + queue = append(queue, dep) |
| + } |
| + } |
| + return seen |
| +} |
diff --git a/internal/sheets/store/handle_test.go b/internal/sheets/store/handle_test.go| new file mode 100644 |
| index 0000000..42a0217 |
| --- /dev/null |
| +++ b/internal/sheets/store/handle_test.go |
| @@ -0,0 +1,174 @@ |
| +package store_test |
| + |
| +import ( |
| + "errors" |
| + "path/filepath" |
| + "testing" |
| + |
| + "gorm.io/gorm" |
| + |
| + "amadan.net/oficina/sheets/internal/sheets" |
| + "amadan.net/oficina/sheets/internal/sheets/calc" |
| + "amadan.net/oficina/sheets/internal/sheets/seal" |
| + "amadan.net/oficina/sheets/internal/sheets/store" |
| +) |
| + |
| +// Fixed ids seeded() uses, so every test can reference a cell by name |
| +// instead of threading ids through the test body. |
| +const ( |
| + tabID uint64 = 10 |
| + rowA uint64 = 100 // position 0 |
| + rowB uint64 = 101 // position 1 |
| + colA uint64 = 200 // position 0 |
| + actor uint64 = 7 |
| +) |
| + |
| +// seeded builds a real database (through rastrillo's driver, WAL mode — |
| +// the same reasoning as store/helpers_test.go's openTestDB), migrates |
| +// every sheets model a handle touches, and seeds one tab with two rows, |
| +// one column, and a single literal cell at (rowA, colA) holding "5". It |
| +// returns the db, the sealer, and the tab id; rowA, rowB, colA and actor |
| +// above are the ids to address it with. |
| +func seeded(t *testing.T) (*gorm.DB, *seal.Sealer, uint64) { |
| + t.Helper() |
| + dir := t.TempDir() |
| + db := openTestDB(t, filepath.Join(dir, "test.db")) |
| + if err := db.AutoMigrate( |
| + &sheets.Sheet{}, &sheets.Tab{}, &sheets.Row{}, &sheets.Col{}, |
| + &sheets.Cell{}, &sheets.Format{}, |
| + ); err != nil { |
| + t.Fatalf("migrate sheets models: %v", err) |
| + } |
| + |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatalf("seal.New: %v", err) |
| + } |
| + |
| + if err := db.Create(&sheets.Tab{ID: tabID, SheetID: 1}).Error; err != nil { |
| + t.Fatalf("seed tab: %v", err) |
| + } |
| + if err := db.Create([]*sheets.Row{ |
| + {ID: rowA, TabID: tabID, Position: 0}, |
| + {ID: rowB, TabID: tabID, Position: 1}, |
| + }).Error; err != nil { |
| + t.Fatalf("seed rows: %v", err) |
| + } |
| + if err := db.Create(&sheets.Col{ID: colA, TabID: tabID, Position: 0}).Error; err != nil { |
| + t.Fatalf("seed col: %v", err) |
| + } |
| + |
| + inputSealed, err := s.Seal("cell.input", []byte("5")) |
| + if err != nil { |
| + t.Fatalf("seal input: %v", err) |
| + } |
| + // CacheVersion is deliberately left at its zero value, behind |
| + // InputVersion 1 — this cell is stale by construction, exactly the |
| + // shape a freshly-imported or freshly-restored cell takes before |
| + // anything has ever recomputed it. That forces every test's first |
| + // Open to run a real recompute for A1 rather than trusting a |
| + // hand-seeded placeholder cache, which would otherwise silently |
| + // stand in for "5" without ever being checked against it. |
| + if err := db.Create(&sheets.Cell{ |
| + TabID: tabID, RowID: rowA, ColID: colA, |
| + InputSealed: inputSealed, InputVersion: 1, |
| + }).Error; err != nil { |
| + t.Fatalf("seed cell: %v", err) |
| + } |
| + |
| + return db, s, tabID |
| +} |
| + |
| +// This is the test that matters. A crash between the input write and its |
| +// dependents' rewrite leaves cached values disagreeing with inputs — |
| +// silently, and forever, unless the handle detects it. |
| +func TestStaleCacheIsRecomputedOnOpen(t *testing.T) { |
| + db, sealer, tabID := seeded(t) |
| + |
| + h, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatalf("Open: %v", err) |
| + } |
| + if err := h.Set(rowB, colA, "=A1*2", actor); err != nil { |
| + t.Fatalf("Set: %v", err) |
| + } |
| + h.Release() |
| + |
| + // Simulate a crash mid-recalc: bump A1's input version without |
| + // rewriting B1's cache. This is exactly what a panic between the two |
| + // writes leaves behind. |
| + db.Model(&sheets.Cell{}). |
| + Where("row_id = ? AND col_id = ?", rowA, colA). |
| + Updates(map[string]any{"input_version": 99}) |
| + |
| + h2, err := store.Open(db, sealer, tabID) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer h2.Release() |
| + |
| + var b sheets.Cell |
| + db.Where("row_id = ? AND col_id = ?", rowB, colA).First(&b) |
| + if b.CacheVersion < 99 { |
| + t.Fatalf("stale cache served: CacheVersion %d < InputVersion 99", b.CacheVersion) |
| + } |
| + |
| + // The value itself must also be correct, not just the version stamp: |
| + // A1 is still 5 (only its version was tampered, not its content), so |
| + // B1 = A1*2 must read 10, both from the handle and from what got |
| + // persisted. |
| + if got := h2.Value(rowB, colA); got.Kind != calc.KindNum || got.Num != 10 { |
| + t.Fatalf("B1 = %+v, want KindNum 10", got) |
| + } |
| +} |
| + |
| +// A tab past the cap fails honestly rather than dying slowly. |
| +func TestCellCapIsAnHonestError(t *testing.T) { |
| + dir := t.TempDir() |
| + db := openTestDB(t, filepath.Join(dir, "test.db")) |
| + if err := db.AutoMigrate(&sheets.Cell{}); err != nil { |
| + t.Fatalf("migrate: %v", err) |
| + } |
| + s, err := seal.New(make32()) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + |
| + const oversizedTabID uint64 = 999 |
| + seedOversizedTab(t, db, oversizedTabID, store.MaxCellsPerTab+1) |
| + |
| + _, err = store.Open(db, s, oversizedTabID) |
| + if !errors.Is(err, store.ErrTabTooLarge) { |
| + t.Fatalf("oversized tab: %v, want ErrTabTooLarge", err) |
| + } |
| +} |
| + |
| +// seedOversizedTab inserts n minimal, content-free cell rows for tabID — |
| +// enough to trip the cap check, which counts rows before Open ever tries |
| +// to decrypt one. Raw, batched SQL rather than sealer.Seal + GORM.Create |
| +// per row: sealing and decrypting MaxCellsPerTab+1 rows for a test that |
| +// only exercises the COUNT(*) short-circuit would make this test |
| +// minutes slower for no coverage gained. |
| +func seedOversizedTab(t *testing.T, db *gorm.DB, tabID uint64, n int) { |
| + t.Helper() |
| + const batch = 500 |
| + for start := 0; start < n; start += batch { |
| + end := start + batch |
| + if end > n { |
| + end = n |
| + } |
| + values := make([]any, 0, (end-start)*3) |
| + placeholders := "" |
| + for i := start; i < end; i++ { |
| + if i > start { |
| + placeholders += "," |
| + } |
| + placeholders += "(?,?,?)" |
| + values = append(values, tabID, uint64(i+1), uint64(1)) |
| + } |
| + sql := "INSERT INTO cells (tab_id, row_id, col_id) VALUES " + placeholders |
| + if err := db.Exec(sql, values...).Error; err != nil { |
| + t.Fatalf("seed oversized tab: %v", err) |
| + } |
| + } |
| +} |