From f208da94d6e30c7df4ade36f962467cb17239ce6 Mon Sep 17 00:00:00 2001 From: Steve Dudenhoeffer Date: Tue, 21 Jul 2026 05:07:30 +0000 Subject: [PATCH] =?UTF-8?q?Revision=20history:=20change=20sets=20+=20revis?= =?UTF-8?q?ions=20+=20revert=20=E2=80=94=20the=20undo=20substrate=20(#48)?= =?UTF-8?q?=20(#61)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Steve Dudenhoeffer --- DESIGN.md | 8 +- internal/api/api.go | 10 +- internal/api/errors.go | 14 + internal/api/history.go | 80 +++ internal/api/history_test.go | 153 ++++ internal/domain/domain.go | 91 +++ internal/service/gardens.go | 16 +- internal/service/objects.go | 39 +- internal/service/ops.go | 56 +- internal/service/plantings.go | 33 +- internal/service/revisions.go | 654 +++++++++++++++++ internal/service/revisions_test.go | 706 +++++++++++++++++++ internal/store/migrations/0006_revisions.sql | 53 ++ internal/store/objects.go | 27 + internal/store/plantings.go | 52 +- internal/store/revisions.go | 196 +++++ 16 files changed, 2169 insertions(+), 19 deletions(-) create mode 100644 internal/api/history.go create mode 100644 internal/api/history_test.go create mode 100644 internal/service/revisions.go create mode 100644 internal/service/revisions_test.go create mode 100644 internal/store/migrations/0006_revisions.sql create mode 100644 internal/store/revisions.go diff --git a/DESIGN.md b/DESIGN.md index d1aebb0..cfd16aa 100644 --- a/DESIGN.md +++ b/DESIGN.md @@ -22,6 +22,7 @@ SQLite, centimeters everywhere (display-side imperial conversion only), `version - **garden_shares** — (garden_id, user_id) unique, role `viewer`|`editor`, created_by. Owner is implicit via `gardens.owner_id`, never a share row. - **garden_objects** — one polymorphic table: `kind` (`bed`|`grow_bag`|`container`|`in_ground`|`tree`|`path`|`structure`), name, `shape` (`rect`|`circle`; `polygon` + `points` JSON reserved in schema, not in the v1 editor), center `x_cm`,`y_cm` in garden space, `width_cm`,`height_cm` (circle: width=diameter), `rotation_deg`, `z_index`, `plantable` bool, nullable `color` override, `props` JSON for kind-specific extras (bed height, tree species…), notes. - **plants** — catalog. `owner_id NULL` = built-in (~30 seeded via migration, read-only, clone to customize). name, category (`vegetable`|`herb`|`flower`|`fruit`|`tree_shrub`|`cover`), `spacing_cm` (mature spacing), `color` hex, `icon` (emoji string — zero assets in v1), nullable `days_to_maturity`, notes. Users see built-ins + their own. +- **change_sets** / **revisions** — the undo substrate. A change set groups the row-level revisions one logical operation produced (`source` ui/agent/api, `summary`, nullable `agent_run_id`, nullable `reverts_id`); a revision is `(entity_type, entity_id, op, before, after)` with full JSON row snapshots. The unit of undo is the **operation, not the row**. Revert is itself a change set, so an undo can be undone; it is version-guarded per entity and reports conflicts rather than clobbering a later edit. Garden *deletion* is deliberately not revertible (the cascade is invisible to the service, and would take the history with it). - **plantings** ("plops") — object_id, plant_id, center `x_cm`,`y_cm` **in the parent object's local frame** (moving/rotating a bed moves its plants for free), `radius_cm` (a plop is a circular patch), nullable `count` (NULL = derived: `max(1, round(π·r² / spacing_cm²))`), nullable label, `planted_at`/`removed_at` dates. "Clear bed" sets `removed_at`; v1 shows rows where `removed_at IS NULL`. Year-over-year history and a future plan/scenario layer build on these dates without schema surgery. ## Geometry @@ -52,6 +53,8 @@ POST /auth/register | /auth/login | /auth/logout GET /auth/me GET /auth GET /auth/oidc/login GET /auth/oidc/callback (authorization-code + PKCE) GET,POST /gardens GET,PATCH,DELETE /gardens/:id GET /gardens/:id/full ← one-shot editor load: garden + objects + plantings + referenced plants +GET /gardens/:id/history ← change sets, newest first (paginated) +POST /change-sets/:id/revert ← undo an operation; 201, or 409 + the conflicts it skipped POST /gardens/:id/copy ← deep-copy a garden you own (objects + active plops; not shares/link) POST /gardens/:id/objects PATCH,DELETE /objects/:id POST /objects/:id/plantings PATCH,DELETE /plantings/:id @@ -76,7 +79,8 @@ internal/store/ sqlite.go (open/pragmas), migrate.go, migrations/*.sql internal/domain/ plain structs + sentinel errors (ErrNotFound, ErrForbidden, ErrVersionConflict) internal/service/ ★ THE SEAM. All permission checks + business ops. auth.go, gardens.go, - objects.go, plantings.go, plants.go, shares.go, ops.go (FillRegion etc.) + objects.go, plantings.go, plants.go, shares.go, ops.go (FillRegion etc.), + revisions.go (change-set scoping, recording, revert) internal/api/ thin gin handlers (decode → service → encode), session middleware, ginserver setup, routes.go, oidc.go, spa.go (embed fallback) internal/webdist/ dist.go: //go:embed all:dist (web build copied in by Makefile) @@ -85,6 +89,8 @@ web/ Vite app Makefile (cd web && npm run build) → copy dist → CGO_ENABLED=0 go build ``` +**Every mutation is recorded.** Service mutations call `record(...)`, which joins the change set on the context if one is open and otherwise opens a single-op one for that mutation alone. That auto-scope is what keeps undo shallow: REST handlers need no changes at all, and only the agent wraps a whole turn (`WithChangeSet`) so a multi-step turn undoes as one unit. A multi-row operation passes all its changes in one `record` call for the same reason. + **The service seam is the point.** Every operation is a method on `*service.Service` taking `(ctx, actorUserID, args)`; all ACL checks live there, not in handlers. REST handlers are thin adapters; future agent tools are equally thin `llm.DefineTool[Args]` adapters over the same methods — so agent calls inherit permission enforcement for free. Bulk ops designed for agents from the start, e.g. `FillRegion(ctx, actor, objectID, region, plantID, spacingOverride)` with `region` a rect/circle in object-local coordinates. ### Auth diff --git a/internal/api/api.go b/internal/api/api.go index 8fe83d8..3cfb26b 100644 --- a/internal/api/api.go +++ b/internal/api/api.go @@ -81,8 +81,9 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine { gardens.GET("/:id", h.getGarden) gardens.PATCH("/:id", h.updateGarden) gardens.DELETE("/:id", h.deleteGarden) - gardens.POST("/:id/copy", h.copyGarden) // duplicate a garden the actor owns - gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load + gardens.POST("/:id/copy", h.copyGarden) // duplicate a garden the actor owns + gardens.GET("/:id/full", h.getGardenFull) // one-shot editor load + gardens.GET("/:id/history", h.getGardenHistory) // change sets, newest first gardens.POST("/:id/objects", h.createObject) // Sharing (owner-managed; a recipient may remove their own share). @@ -111,6 +112,11 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine { plantings.PATCH("/:id", h.updatePlanting) plantings.DELETE("/:id", h.deletePlanting) + // Undo. A change set is addressed by its own id; the service resolves the + // owning garden for the permission check, same as objects and plantings. + changeSets := v1.Group("/change-sets", h.requireAuth()) + changeSets.POST("/:id/revert", h.revertChangeSet) + // Plant catalog: built-ins (seeded, read-only) plus the actor's own rows. plants := v1.Group("/plants", h.requireAuth()) plants.GET("", h.listPlants) diff --git a/internal/api/errors.go b/internal/api/errors.go index 966c826..2a16c31 100644 --- a/internal/api/errors.go +++ b/internal/api/errors.go @@ -69,6 +69,20 @@ func writeVersionConflict(c *gin.Context, current any) { }) } +// intQuery reads a non-negative integer query parameter, falling back to def on +// an absent or malformed value. +func intQuery(c *gin.Context, name string, def int) int { + raw := c.Query(name) + if raw == "" { + return def + } + v, err := strconv.Atoi(raw) + if err != nil || v < 0 { + return def + } + return v +} + // parseIDParam reads a positive int64 path parameter, writing a 400 and // returning ok=false on a malformed value. func parseIDParam(c *gin.Context, name string) (int64, bool) { diff --git a/internal/api/history.go b/internal/api/history.go new file mode 100644 index 0000000..de77f40 --- /dev/null +++ b/internal/api/history.go @@ -0,0 +1,80 @@ +package api + +import ( + "net/http" + + "github.com/gin-gonic/gin" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// Change history and undo (#48). Two endpoints: read a garden's change sets, and +// revert one. Both encode domain types directly, like the rest of the package — +// domain.ChangeSet already carries the actor name, revert linkage and per-op +// counts the history list renders, and its Revisions field is omitempty so the +// JSON snapshots never ride along on a list response. + +// historyResponse is the body of GET /gardens/:id/history. hasMore lets the +// client page without a separate count query over a table that only grows. +type historyResponse struct { + ChangeSets []domain.ChangeSet `json:"changeSets"` + HasMore bool `json:"hasMore"` +} + +// revertResponse is the body of POST /change-sets/:id/revert on every path. +// Carrying both fields regardless is what lets the UI say "2 of 3 changes undone; +// the north bed was edited since and was left alone" instead of a generic +// failure — a partial revert really did change things, and pretending otherwise +// would be a lie. ChangeSet is null when nothing needed reverting. +type revertResponse struct { + ChangeSet *domain.ChangeSet `json:"changeSet"` + Conflicts []domain.RevertConflict `json:"conflicts"` +} + +func (h *handlers) getGardenHistory(c *gin.Context) { + gardenID, ok := parseIDParam(c, "id") + if !ok { + return + } + // 0 means "the service's default"; it clamps the upper bound too. + limit := intQuery(c, "limit", 0) + offset := intQuery(c, "offset", 0) + + sets, hasMore, err := h.svc.GardenHistory(c.Request.Context(), mustActor(c).ID, gardenID, limit, offset) + if err != nil { + writeServiceError(c, err) + return + } + c.JSON(http.StatusOK, historyResponse{ChangeSets: sets, HasMore: hasMore}) +} + +func (h *handlers) revertChangeSet(c *gin.Context) { + id, ok := parseIDParam(c, "id") + if !ok { + return + } + // A revert through the REST API is a person clicking undo. The agent reverts + // its own work through the service directly and stamps SourceAgent, so the + // history badge can tell the two apart. + cs, conflicts, err := h.svc.RevertChangeSet(c.Request.Context(), mustActor(c).ID, id, domain.SourceUI) + if err != nil { + writeServiceError(c, err) + return + } + if conflicts == nil { + conflicts = []domain.RevertConflict{} + } + body := revertResponse{ChangeSet: cs, Conflicts: conflicts} + switch { + case len(conflicts) > 0: + // 409 even when part of the revert applied: something the caller asked for + // did not happen, and the body says exactly what. + c.JSON(http.StatusConflict, body) + case cs == nil: + // Every revision resolved to a no-op (already undone by hand, say). Nothing + // was created, so 200 rather than a 201 pointing at nothing. + c.JSON(http.StatusOK, body) + default: + c.JSON(http.StatusCreated, body) + } +} diff --git a/internal/api/history_test.go b/internal/api/history_test.go new file mode 100644 index 0000000..f4cc616 --- /dev/null +++ b/internal/api/history_test.go @@ -0,0 +1,153 @@ +package api + +import ( + "net/http" + "strconv" + "testing" +) + +func historyPath(gardenID int64) string { + return "/api/v1/gardens/" + strconv.FormatInt(gardenID, 10) + "/history" +} + +func revertPath(changeSetID int64) string { + return "/api/v1/change-sets/" + strconv.FormatInt(changeSetID, 10) + "/revert" +} + +// TestHistoryAndRevertAPI walks the whole loop over HTTP: a mutation shows up in +// history without anyone asking for it, reverting it answers 201 with the new +// change set, and the change is actually gone from /full. +func TestHistoryAndRevertAPI(t *testing.T) { + r := authEngine(t, localCfg()) + cookie := registerAndCookie(t, r, "hist@example.com") + gid := createGardenAPI(t, r, cookie, "G") + + w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{ + "kind": "bed", "name": "North Bed", "xCm": 100, "yCm": 100, "widthCm": 100, "heightCm": 100, + }, cookie) + if w.Code != http.StatusCreated { + t.Fatalf("create object: status %d, body %s", w.Code, w.Body.String()) + } + // The create landed in history with no explicit change set anywhere. + w = doJSON(t, r, http.MethodGet, historyPath(gid), nil, cookie) + if w.Code != http.StatusOK { + t.Fatalf("history: status %d, body %s", w.Code, w.Body.String()) + } + body := decodeMap(t, w.Body.Bytes()) + sets, _ := body["changeSets"].([]any) + if len(sets) != 1 { + t.Fatalf("got %d change sets, want 1: %s", len(sets), w.Body.String()) + } + if body["hasMore"].(bool) { + t.Error("hasMore should be false for a single-entry history") + } + entry := sets[0].(map[string]any) + if entry["summary"] != "Added North Bed" { + t.Errorf("summary = %v", entry["summary"]) + } + if entry["source"] != "ui" || entry["actorName"] == "" { + t.Errorf("unexpected entry: %+v", entry) + } + counts, _ := entry["counts"].([]any) + if len(counts) != 1 { + t.Fatalf("counts = %+v", entry["counts"]) + } + + // Revert it: 201, and the new change set points back at the original. + csID := int64(entry["id"].(float64)) + w = doJSON(t, r, http.MethodPost, revertPath(csID), nil, cookie) + if w.Code != http.StatusCreated { + t.Fatalf("revert: status %d, body %s", w.Code, w.Body.String()) + } + rev := decodeMap(t, w.Body.Bytes()) + cs := rev["changeSet"].(map[string]any) + if int64(cs["revertsId"].(float64)) != csID { + t.Errorf("revertsId = %v, want %d", cs["revertsId"], csID) + } + if conflicts, _ := rev["conflicts"].([]any); len(conflicts) != 0 { + t.Errorf("unexpected conflicts: %+v", conflicts) + } + + // The object is gone from the editor payload. + w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie) + full := decodeMap(t, w.Body.Bytes()) + if objects, _ := full["objects"].([]any); len(objects) != 0 { + t.Errorf("%d objects survived the revert", len(objects)) + } +} + +// TestRevertConflictAPI: a partial revert answers 409 and names what it skipped, +// because "2 of 3 undone, the north bed was edited since" is the only useful +// thing to say — a bare failure would be a lie about what happened. +func TestRevertConflictAPI(t *testing.T) { + r := authEngine(t, localCfg()) + cookie := registerAndCookie(t, r, "conflict@example.com") + gid := createGardenAPI(t, r, cookie, "G") + + w := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{ + "kind": "bed", "name": "Bed", "xCm": 100, "yCm": 100, "widthCm": 100, "heightCm": 100, + }, cookie) + obj := decodeMap(t, w.Body.Bytes()) + objectID := int64(obj["id"].(float64)) + version := int64(obj["version"].(float64)) + + // Move it (change set #2), then edit it again (change set #3). + w = doJSON(t, r, http.MethodPatch, objectPath(objectID), map[string]any{ + "xCm": 300, "version": version, + }, cookie) + if w.Code != http.StatusOK { + t.Fatalf("move: status %d, body %s", w.Code, w.Body.String()) + } + version = int64(decodeMap(t, w.Body.Bytes())["version"].(float64)) + w = doJSON(t, r, http.MethodPatch, objectPath(objectID), map[string]any{ + "name": "Renamed", "version": version, + }, cookie) + if w.Code != http.StatusOK { + t.Fatalf("rename: status %d, body %s", w.Code, w.Body.String()) + } + + // Undoing the move now conflicts: the row changed after it. + w = doJSON(t, r, http.MethodGet, historyPath(gid), nil, cookie) + sets := decodeMap(t, w.Body.Bytes())["changeSets"].([]any) + moveID := int64(sets[1].(map[string]any)["id"].(float64)) // newest first: rename, move, create + + w = doJSON(t, r, http.MethodPost, revertPath(moveID), nil, cookie) + if w.Code != http.StatusConflict { + t.Fatalf("revert: status %d, want 409, body %s", w.Code, w.Body.String()) + } + conflicts := decodeMap(t, w.Body.Bytes())["conflicts"].([]any) + if len(conflicts) != 1 { + t.Fatalf("conflicts = %+v", conflicts) + } + c := conflicts[0].(map[string]any) + if c["reason"] != "changed" || c["name"] != "Renamed" { + t.Errorf("conflict = %+v", c) + } + + // The object was left exactly alone. + w = doJSON(t, r, http.MethodGet, fullPath(gid), nil, cookie) + objects := decodeMap(t, w.Body.Bytes())["objects"].([]any) + o := objects[0].(map[string]any) + if o["xCm"].(float64) != 300 || o["name"] != "Renamed" { + t.Errorf("conflicted object was modified: %+v", o) + } +} + +// TestHistoryRequiresAccess — an unauthenticated caller can't read history, and a +// stranger gets 404 rather than a hint that the garden exists. +func TestHistoryRequiresAccess(t *testing.T) { + r := authEngine(t, localCfg()) + owner := registerAndCookie(t, r, "owner@example.com") + gid := createGardenAPI(t, r, owner, "G") + stranger := registerAndCookie(t, r, "stranger@example.com") + + if w := doJSON(t, r, http.MethodGet, historyPath(gid), nil, nil); w.Code != http.StatusUnauthorized { + t.Errorf("anonymous history status = %d, want 401", w.Code) + } + if w := doJSON(t, r, http.MethodGet, historyPath(gid), nil, stranger); w.Code != http.StatusNotFound { + t.Errorf("stranger history status = %d, want 404", w.Code) + } + if w := doJSON(t, r, http.MethodPost, revertPath(1), nil, stranger); w.Code != http.StatusNotFound { + t.Errorf("stranger revert status = %d, want 404", w.Code) + } +} diff --git a/internal/domain/domain.go b/internal/domain/domain.go index 1183185..b13ffbb 100644 --- a/internal/domain/domain.go +++ b/internal/domain/domain.go @@ -93,6 +93,97 @@ const ( CategoryFruit = "fruit" CategoryTreeShrub = "tree_shrub" CategoryCover = "cover" + + // Where a change set came from. SourceAgent is what the history UI badges + // differently, so an autonomous edit is never mistaken for a hand edit. + SourceUI = "ui" + SourceAgent = "agent" + SourceAPI = "api" + + // The entity types a revision can snapshot. Garden covers metadata edits + // only — garden creation and deletion are deliberately not revertible. + EntityGarden = "garden" + EntityObject = "object" + EntityPlanting = "planting" + + OpCreate = "create" + OpUpdate = "update" + OpDelete = "delete" +) + +// ChangeSet groups the revisions produced by one logical operation — the unit of +// undo. A revert is itself a change set (RevertsID names its target), so history +// is append-only and an undo can be undone. +type ChangeSet struct { + ID int64 `json:"id"` + GardenID int64 `json:"gardenId"` + ActorID int64 `json:"actorId"` + Source string `json:"source"` + Summary string `json:"summary"` + AgentRunID *string `json:"agentRunId,omitempty"` + RevertsID *int64 `json:"revertsId,omitempty"` + CreatedAt string `json:"createdAt"` + + // Computed by the service for the history list, never persisted. + ActorName string `json:"actorName,omitempty"` + // RevertedByID is the change set that reverted this one, if any — what lets + // the list mark an entry as already undone. + RevertedByID *int64 `json:"revertedById,omitempty"` + // Counts tallies the revisions by (entity type, op) so the list can say + // "3 plops added, 1 bed moved" without loading every revision. + Counts []ChangeCount `json:"counts,omitempty"` + // Revisions is populated only when a single change set is loaded in full. + Revisions []Revision `json:"revisions,omitempty"` +} + +// ChangeCount is one (entity type, op) tally within a change set. +type ChangeCount struct { + EntityType string `json:"entityType"` + Op string `json:"op"` + N int `json:"n"` +} + +// Revision is one row-level change within a change set. Before/After are JSON +// row snapshots (nil for create/delete respectively) and include the row's +// version, which the revert guard compares against. +type Revision struct { + ID int64 `json:"id"` + ChangeSetID int64 `json:"changeSetId"` + Seq int64 `json:"seq"` + EntityType string `json:"entityType"` + EntityID int64 `json:"entityId"` + Op string `json:"op"` + Before *string `json:"before,omitempty"` + After *string `json:"after,omitempty"` +} + +// RevertConflict reports one entity a revert deliberately left alone, because +// reverting it would have discarded a change made after the change set being +// reverted. The rest of the change set still reverts. +type RevertConflict struct { + EntityType string `json:"entityType"` + EntityID int64 `json:"entityId"` + // Reason is one of the ConflictReason* values below. + Reason string `json:"reason"` + // Name is a human label for the entity where one exists (an object's name), + // so the UI can say "the north bed" rather than "object 41". + Name string `json:"name,omitempty"` +} + +// Why a revert skipped an entity. +const ( + // ConflictChanged: the row was edited after the change set being reverted, so + // restoring the old snapshot would silently discard that edit. + ConflictChanged = "changed" + // ConflictMissing: the row is gone, so there is nothing to revert to. + ConflictMissing = "missing" + // ConflictExists: a row with that id exists again, so restoring the deleted + // snapshot would overwrite it. + ConflictExists = "exists" + // ConflictUnsupported: this kind of change has no inverse the revert knows how + // to apply. Reported rather than silently skipped, so a change recorded by a + // later feature that revert wasn't taught about is visible instead of quiet. + ConflictUnsupported = "unsupported" ) // User is a pansy account. It may have a local password, OIDC identity, or both. diff --git a/internal/service/gardens.go b/internal/service/gardens.go index 54ca7a1..0811e2f 100644 --- a/internal/service/gardens.go +++ b/internal/service/gardens.go @@ -140,7 +140,8 @@ func (s *Service) ListGardens(ctx context.Context, actorID int64) ([]domain.Gard // version mismatch it returns (current garden, ErrVersionConflict) so the handler // can return the fresh row for the client to rebase. func (s *Service) UpdateGarden(ctx context.Context, actorID, gardenID int64, in GardenInput, version int64) (*domain.Garden, error) { - if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner); err != nil { + before, err := s.requireGardenRole(ctx, actorID, gardenID, roleOwner) + if err != nil { return nil, err } g, err := gardenFromInput(in, false) // no defaults: an update states every field @@ -153,7 +154,18 @@ func (s *Service) UpdateGarden(ctx context.Context, actorID, gardenID int64, in if updated != nil { updated.MyRole = roleOwner.String() // only the owner reaches here } - return updated, err + if err != nil { + return updated, err + } + // MyRole is computed, not stored; blank it in the snapshot so a revert can't + // write a role into a row that has no such column. + snap := *before + snap.MyRole = "" + after := *updated + after.MyRole = "" + s.record(ctx, gardenID, actorID, "Edited garden settings", + changeUpdate(domain.EntityGarden, gardenID, &snap, &after)) + return updated, nil } // CopyGarden duplicates a garden into a new one owned by the actor, carrying diff --git a/internal/service/objects.go b/internal/service/objects.go index d392bea..c2357ca 100644 --- a/internal/service/objects.go +++ b/internal/service/objects.go @@ -122,7 +122,23 @@ func (s *Service) CreateObject(ctx context.Context, actorID, gardenID int64, in if err := finalizeObject(o, g); err != nil { return nil, err } - return s.store.CreateObject(ctx, o) + created, err := s.store.CreateObject(ctx, o) + if err != nil { + return nil, err + } + s.record(ctx, gardenID, actorID, "Added "+objectLabel(created), + changeCreate(domain.EntityObject, created.ID, created)) + return created, nil +} + +// objectLabel names an object for a change-set summary: its own name when it has +// one, else its kind ("bed", "grow_bag"), so history reads as "Added bed" rather +// than "Added ". +func objectLabel(o *domain.GardenObject) string { + if o.Name != "" { + return o.Name + } + return o.Kind } // objectForRole loads an object and enforces that the actor holds at least min @@ -148,21 +164,36 @@ func (s *Service) UpdateObject(ctx context.Context, actorID, objectID int64, pat return nil, err } + before := *o // objectForRole returns the current row, and the patch mutates it applyObjectPatch(o, patch) if err := finalizeObject(o, g); err != nil { return nil, err } o.Version = version - return s.store.UpdateObject(ctx, o) + updated, err := s.store.UpdateObject(ctx, o) + if err != nil { + return updated, err // may carry the current row on a version conflict + } + s.record(ctx, g.ID, actorID, "Edited "+objectLabel(updated), + changeUpdate(domain.EntityObject, updated.ID, &before, updated)) + return updated, nil } // DeleteObject removes an object (its plantings cascade) from a garden the actor // can edit. func (s *Service) DeleteObject(ctx context.Context, actorID, objectID int64) error { - if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil { + o, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor) + if err != nil { return err } - return s.store.DeleteObject(ctx, objectID) + // deleteObjectRecording snapshots the plantings the FK is about to cascade + // away, so the delete is actually revertible rather than merely listed. + changes, err := s.deleteObjectRecording(ctx, o) + if err != nil { + return err + } + s.record(ctx, g.ID, actorID, "Deleted "+objectLabel(o), changes...) + return nil } // GardenFull returns the whole editor payload for a garden the actor can view. diff --git a/internal/service/ops.go b/internal/service/ops.go index 70e3f42..9fd6d0a 100644 --- a/internal/service/ops.go +++ b/internal/service/ops.go @@ -2,6 +2,8 @@ package service import ( "context" + "fmt" + "log/slog" "math" "strings" @@ -151,6 +153,14 @@ func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.Garde if err != nil { return nil, err } + // One record call with every plop, so a fill auto-scopes into ONE change set + // with N revisions — undoing a fill is one click, not N. + changes := make([]change, 0, len(created)) + for i := range created { + changes = append(changes, changeCreate(domain.EntityPlanting, created[i].ID, &created[i])) + } + s.record(ctx, o.GardenID, actorID, + fmt.Sprintf("Planted %d %s in %s", len(created), plant.Name, objectLabel(o)), changes...) for i := range created { created[i].DerivedCount = derivedCount(created[i].RadiusCM, spacing) } @@ -219,11 +229,53 @@ func (s *Service) FillNamedRegion(ctx context.Context, actorID, objectID int64, // non-plantable after it was planted must still be clearable (you can always // remove existing plops, only not add new ones). func (s *Service) ClearObject(ctx context.Context, actorID, objectID int64) (int, error) { - if _, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor); err != nil { + o, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor) + if err != nil { return 0, err } + // Snapshot the rows the bulk UPDATE is about to touch, since it reports only a + // count — then clear exactly those ids. Clearing "every active plop" instead + // would let a plop created between this read and the UPDATE be removed with no + // revision recorded: cleared, with no way to undo it. + before, err := s.store.ListActivePlantingsForObject(ctx, objectID) + if err != nil { + return 0, err + } + ids := make([]int64, 0, len(before)) + for i := range before { + ids = append(ids, before[i].ID) + } today := s.now().UTC().Format(dateLayout) - return s.store.ClearObjectPlantings(ctx, objectID, today) + n, err := s.store.ClearObjectPlantings(ctx, objectID, today, ids) + if err != nil || n == 0 { + return n, err + } + + // From here the clear HAS happened. A failure to build the history entry must + // not be reported as a failed clear — the caller would retry an operation that + // already applied. Log the gap and report success, matching how record() + // treats its own write failures. + after, err := s.store.ListPlantingsForObject(ctx, objectID) + if err != nil { + slog.Error("service: clear succeeded but history could not be recorded", + "error", err, "object", objectID, "cleared", n) + return n, nil + } + afterByID := make(map[int64]*domain.Planting, len(after)) + for i := range after { + afterByID[after[i].ID] = &after[i] + } + changes := make([]change, 0, len(before)) + for i := range before { + b := before[i] + a, ok := afterByID[b.ID] + if !ok { + continue // deleted outright between the two reads; nothing coherent to record + } + changes = append(changes, changeUpdate(domain.EntityPlanting, b.ID, &b, a)) + } + s.record(ctx, g.ID, actorID, fmt.Sprintf("Cleared %s (%d plantings)", objectLabel(o), n), changes...) + return n, nil } // DescribeResult is a structured summary of a garden for prompting an agent. diff --git a/internal/service/plantings.go b/internal/service/plantings.go index 84daf69..b1006b9 100644 --- a/internal/service/plantings.go +++ b/internal/service/plantings.go @@ -72,7 +72,7 @@ type PlantingPatch struct { // CreatePlanting places a plop in a plantable object the actor can edit. func (s *Service) CreatePlanting(ctx context.Context, actorID, objectID int64, in PlantingInput) (*domain.Planting, error) { - o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor) + o, g, err := s.objectForRole(ctx, actorID, objectID, roleEditor) if err != nil { return nil, err } @@ -105,6 +105,8 @@ func (s *Service) CreatePlanting(ctx context.Context, actorID, objectID int64, i if err != nil { return nil, err } + s.record(ctx, g.ID, actorID, "Planted "+plant.Name+" in "+objectLabel(o), + changeCreate(domain.EntityPlanting, created.ID, created)) created.DerivedCount = derivedCount(created.RadiusCM, plant.SpacingCM) return created, nil } @@ -116,11 +118,12 @@ func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64, if err != nil { return nil, err // ErrNotFound } - o, _, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor) + o, g, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor) if err != nil { return nil, err } + before := *pl // GetPlanting returns the current row, and the patch mutates it originalPlantID := pl.PlantID applyPlantingPatch(pl, patch) // Fetch the plop's plant for the derived count. Only when the actor is @@ -155,10 +158,26 @@ func (s *Service) UpdatePlanting(ctx context.Context, actorID, plantingID int64, } return updated, err } + s.record(ctx, g.ID, actorID, plantingEditSummary(&before, updated, plant, o), + changeUpdate(domain.EntityPlanting, updated.ID, &before, updated)) updated.DerivedCount = derivedCount(updated.RadiusCM, plant.SpacingCM) return updated, nil } +// plantingEditSummary describes a plop edit for the history list. Soft-removal +// ("clear bed", harvested) is the one edit worth naming specifically — it reads +// as a removal to the person who did it, not as an edit. +func plantingEditSummary(before, after *domain.Planting, plant *domain.Plant, o *domain.GardenObject) string { + switch { + case before.RemovedAt == nil && after.RemovedAt != nil: + return "Removed " + plant.Name + " from " + objectLabel(o) + case before.RemovedAt != nil && after.RemovedAt == nil: + return "Restored " + plant.Name + " in " + objectLabel(o) + default: + return "Edited " + plant.Name + " in " + objectLabel(o) + } +} + // DeletePlanting hard-deletes a plop in an object the actor can edit. (Soft // removal — "clear bed" / harvested — sets removed_at via UpdatePlanting.) func (s *Service) DeletePlanting(ctx context.Context, actorID, plantingID int64) error { @@ -166,10 +185,16 @@ func (s *Service) DeletePlanting(ctx context.Context, actorID, plantingID int64) if err != nil { return err } - if _, _, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor); err != nil { + o, g, err := s.objectForRole(ctx, actorID, pl.ObjectID, roleEditor) + if err != nil { return err } - return s.store.DeletePlanting(ctx, plantingID) + if err := s.store.DeletePlanting(ctx, plantingID); err != nil { + return err + } + s.record(ctx, g.ID, actorID, "Deleted a planting from "+objectLabel(o), + changeDelete(domain.EntityPlanting, pl.ID, pl)) + return nil } // visiblePlant loads a plant the actor may reference in a planting: a built-in or diff --git a/internal/service/revisions.go b/internal/service/revisions.go new file mode 100644 index 0000000..d71afa2 --- /dev/null +++ b/internal/service/revisions.go @@ -0,0 +1,654 @@ +package service + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "log/slog" + "sync" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// This file is pansy's undo substrate (#48). The agent acts freely — "empty the +// garlic bed and plant cucumbers" runs without a confirmation prompt — which is +// only defensible because the result is easy to roll back. +// +// The unit of undo is the OPERATION, not the row: a change set groups the +// row-level revisions it produced, and revert replays their inverses. Two +// properties shape everything here: +// +// - Revert is itself a change set (reverts_id names its target). History is +// append-only, so an undo can be undone. git revert, not git reset. +// - Revert is version-guarded per entity. If a row changed after the change set +// being reverted, restoring the old snapshot would silently discard that +// edit, so it is reported as a conflict and left alone — the rest of the +// change set still reverts. + +// maxHistoryPageSize caps a history page so a season of edits can't be pulled in +// one request. +const maxHistoryPageSize = 100 + +// defaultHistoryPageSize is the page size when a caller doesn't ask for one. +const defaultHistoryPageSize = 50 + +// changeSetKey is the context key carrying the ambient change scope. +type changeSetKey struct{} + +// changeScope accumulates the revisions of one logical operation. Revisions are +// buffered rather than written as they happen, so an operation that fails partway +// leaves no half-recorded change set behind — the whole thing lands, or none +// of it does. +type changeScope struct { + gardenID int64 + actorID int64 + source string + summary string + agentRunID *string + + mu sync.Mutex + revs []domain.Revision +} + +func (sc *changeScope) append(revs []domain.Revision) { + sc.mu.Lock() + defer sc.mu.Unlock() + sc.revs = append(sc.revs, revs...) +} + +func (sc *changeScope) taken() []domain.Revision { + sc.mu.Lock() + defer sc.mu.Unlock() + return sc.revs +} + +// scopeFrom returns the change scope on ctx, or nil when the caller opened none. +func scopeFrom(ctx context.Context) *changeScope { + sc, _ := ctx.Value(changeSetKey{}).(*changeScope) + return sc +} + +// change is one row-level mutation waiting to be recorded. before/after are the +// row structs themselves; they are marshalled to JSON snapshots at record time. +type change struct { + entityType string + entityID int64 + op string + before any + after any +} + +func changeCreate(entityType string, id int64, after any) change { + return change{entityType: entityType, entityID: id, op: domain.OpCreate, after: after} +} + +func changeUpdate(entityType string, id int64, before, after any) change { + return change{entityType: entityType, entityID: id, op: domain.OpUpdate, before: before, after: after} +} + +func changeDelete(entityType string, id int64, before any) change { + return change{entityType: entityType, entityID: id, op: domain.OpDelete, before: before} +} + +// ChangeSetOptions describes the change set WithChangeSet opens. +type ChangeSetOptions struct { + // Source is one of domain.SourceUI / SourceAgent / SourceAPI. The history UI + // badges agent changes differently, so an autonomous edit is never mistaken + // for a hand edit. + Source string + // Summary is the one-line description shown in the history list. + Summary string + // AgentRunID joins this change set back to the executus run that produced it. + AgentRunID *string +} + +// WithChangeSet runs fn with a change scope on the context, so every mutation fn +// performs lands in ONE change set — the agent's "a whole turn is one undo" +// guarantee. The change set is written only if fn succeeds; if fn changed +// nothing, none is written and (nil, nil) is returned. +// +// Requires editor role on the garden, checked up front so a scope is never opened +// for an actor who couldn't have mutated anything anyway. +func (s *Service) WithChangeSet(ctx context.Context, actorID, gardenID int64, opts ChangeSetOptions, fn func(context.Context) error) (*domain.ChangeSet, error) { + if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleEditor); err != nil { + return nil, err + } + if !validChangeSource(opts.Source) { + return nil, domain.ErrInvalidInput + } + sc := &changeScope{ + gardenID: gardenID, actorID: actorID, + source: opts.Source, summary: opts.Summary, agentRunID: opts.AgentRunID, + } + if err := fn(context.WithValue(ctx, changeSetKey{}, sc)); err != nil { + // fn failed, but the mutations it completed before failing are already + // committed — this scope buffers history, not data. Dropping the buffer + // would leave those changes with no history and no way to undo them, + // which is exactly the situation undo exists for. Record what happened, + // mark the summary, and still report the failure. + sc.summary = partialSummary(sc.summary) + if _, cerr := s.commitScope(ctx, sc, nil); cerr != nil { + slog.Error("service: partial turn could not be recorded", "error", cerr, "garden", gardenID) + } + return nil, err + } + return s.commitScope(ctx, sc, nil) +} + +// partialSummary marks a change set whose operation failed partway, so the +// history list can say so rather than presenting it as a completed action. +func partialSummary(summary string) string { + if summary == "" { + return "Partly applied (the operation failed partway)" + } + return summary + " (failed partway)" +} + +// commitScope writes a scope's buffered revisions as one change set. revertsID is +// set only by RevertChangeSet. A scope with no revisions writes nothing — an +// operation that changed nothing doesn't belong in history. +func (s *Service) commitScope(ctx context.Context, sc *changeScope, revertsID *int64) (*domain.ChangeSet, error) { + revs := sc.taken() + if len(revs) == 0 { + return nil, nil + } + return s.store.WriteChangeSet(ctx, &domain.ChangeSet{ + GardenID: sc.gardenID, + ActorID: sc.actorID, + Source: sc.source, + Summary: sc.summary, + AgentRunID: sc.agentRunID, + RevertsID: revertsID, + }, revs) +} + +func validChangeSource(s string) bool { + return s == domain.SourceUI || s == domain.SourceAgent || s == domain.SourceAPI +} + +// record files the changes a mutation just made. When the caller opened a scope +// (WithChangeSet) they join it; otherwise this operation becomes its own +// single-op change set on the spot — the AUTO-SCOPE path, which is what keeps +// this feature invasive but shallow: REST handlers need no edits at all and every +// UI mutation lands in history for free. Only the agent wraps a whole turn. +// +// Note a multi-row operation (FillRegion, ClearObject) passes all of its changes +// in ONE call, so it auto-scopes into one change set with N revisions rather than +// N change sets. +// +// Recording is best-effort by design: the row is already written, so failing the +// caller here would report a failure that didn't happen and invite a duplicate +// retry. A history gap is logged loudly instead. +func (s *Service) record(ctx context.Context, gardenID, actorID int64, summary string, changes ...change) { + if len(changes) == 0 { + return + } + revs, err := toRevisions(changes) + if err != nil { + slog.Error("service: snapshot revisions", "error", err, "garden", gardenID, "summary", summary) + return + } + if sc := scopeFrom(ctx); sc != nil { + sc.append(revs) + return + } + if _, err := s.store.WriteChangeSet(ctx, &domain.ChangeSet{ + GardenID: gardenID, ActorID: actorID, Source: domain.SourceUI, Summary: summary, + }, revs); err != nil { + slog.Error("service: record change set", "error", err, "garden", gardenID, "summary", summary) + } +} + +// toRevisions marshals each change's before/after rows to JSON snapshots. +func toRevisions(changes []change) ([]domain.Revision, error) { + revs := make([]domain.Revision, 0, len(changes)) + for _, c := range changes { + before, err := snapshot(c.before) + if err != nil { + return nil, err + } + after, err := snapshot(c.after) + if err != nil { + return nil, err + } + revs = append(revs, domain.Revision{ + EntityType: c.entityType, EntityID: c.entityID, Op: c.op, + Before: before, After: after, + }) + } + return revs, nil +} + +// snapshot marshals a row to a JSON string, preserving nil as a NULL column. +func snapshot(v any) (*string, error) { + if v == nil { + return nil, nil + } + b, err := json.Marshal(v) + if err != nil { + return nil, fmt.Errorf("service: marshal snapshot: %w", err) + } + s := string(b) + return &s, nil +} + +// GardenHistory returns a page of a garden's change sets, newest first, for an +// actor who can at least view it, plus whether more pages follow. limit is +// clamped to [1, maxHistoryPageSize]. +// +// hasMore is answered by reading one row beyond the page and discarding it, +// rather than by a second COUNT over a table that only grows. Keeping that here +// (not in the handler) means the clamp and the probe can't disagree. +func (s *Service) GardenHistory(ctx context.Context, actorID, gardenID int64, limit, offset int) ([]domain.ChangeSet, bool, error) { + if _, err := s.requireGardenRole(ctx, actorID, gardenID, roleViewer); err != nil { + return nil, false, err + } + if limit <= 0 { + limit = defaultHistoryPageSize + } + if limit > maxHistoryPageSize { + limit = maxHistoryPageSize + } + if offset < 0 { + offset = 0 + } + sets, err := s.store.ListChangeSets(ctx, gardenID, limit+1, offset) + if err != nil { + return nil, false, err + } + if len(sets) > limit { + return sets[:limit], true, nil + } + return sets, false, nil +} + +// RevertChangeSet undoes a change set by applying the inverse of each of its +// revisions, inside one NEW change set that points back at the target — so the +// undo is itself undoable. Requires editor role on the garden. source says who +// asked (the agent can undo its own work, and the history badge should show that). +// +// Entities that changed after the target change set are reported as conflicts and +// left untouched; everything else still reverts. Returns the new change set (nil +// when nothing needed reverting) alongside those conflicts. +// +// The inverses are applied one row at a time rather than in a single +// transaction, because the store's version-guarded update path is per-row. That +// means a failure partway leaves the garden partly reverted — so whatever DID +// apply is recorded before the error is returned, and the partial revert is +// itself a change set you can undo. Losing the history for changes that really +// happened would be strictly worse than the partial state. +func (s *Service) RevertChangeSet(ctx context.Context, actorID, changeSetID int64, source string) (*domain.ChangeSet, []domain.RevertConflict, error) { + target, err := s.store.GetChangeSet(ctx, changeSetID) + if err != nil { + return nil, nil, err + } + if _, err := s.requireGardenRole(ctx, actorID, target.GardenID, roleEditor); err != nil { + return nil, nil, err + } + if !validChangeSource(source) { + return nil, nil, domain.ErrInvalidInput + } + + conflicts := []domain.RevertConflict{} + sc := &changeScope{ + gardenID: target.GardenID, actorID: actorID, source: source, + summary: revertSummary(target), + } + // A change set may hold several revisions for the SAME entity (an agent turn + // that moves a bed and then renames it). Each inverse bumps that row's + // version, so from the second one on, the version in the snapshot no longer + // matches the live row — and a naive guard would call our own work a conflict. + // This tracks what we just wrote so the guard compares against reality. + applied := map[entityKey]int64{} + + for _, r := range planRevert(target.Revisions) { + changes, conflict, err := s.applyInverse(ctx, r, applied) + if err != nil { + // Record what actually landed before surfacing the failure, so the + // partial revert is visible and undoable rather than orphaned. + if _, cerr := s.commitScope(ctx, sc, &target.ID); cerr != nil { + slog.Error("service: partial revert could not be recorded", "error", cerr, "changeSet", changeSetID) + } + return nil, nil, err + } + if conflict != nil { + conflicts = append(conflicts, *conflict) + continue + } + revs, err := toRevisions(changes) + if err != nil { + return nil, nil, err + } + sc.append(revs) + } + + cs, err := s.commitScope(ctx, sc, &target.ID) + if err != nil { + return nil, nil, err + } + return cs, conflicts, nil +} + +// entityKey identifies a row across the entity types a revision can name. +type entityKey struct { + entityType string + id int64 +} + +func keyOf(r domain.Revision) entityKey { + return entityKey{entityType: r.EntityType, id: r.EntityID} +} + +// planRevert orders a change set's revisions for inverse application, because the +// inverses carry foreign-key dependencies the original operations did not. Five +// passes, in this order: +// +// 0. restore deleted OBJECTS — parents first, or a restored plop has no object +// to hang off and its FK rejects it; +// 1. restore everything else deleted; +// 2. undo updates; +// 3. delete created non-objects (plantings) — children before parents; +// 4. delete created OBJECTS last, so a delete never leans on the cascade to +// clean up rows this revert is itself responsible for. +// +// Reverse seq order within each pass: the last change made is the first undone. +// The passes are explicit rather than relying on reverse seq happening to order +// parents correctly, which it only does by luck. +func planRevert(revs []domain.Revision) []domain.Revision { + rank := func(r domain.Revision) int { + switch { + case r.Op == domain.OpDelete && r.EntityType == domain.EntityObject: + return 0 + case r.Op == domain.OpDelete: + return 1 + case r.Op == domain.OpUpdate: + return 2 + case r.Op == domain.OpCreate && r.EntityType == domain.EntityObject: + return 4 + default: // create, non-object + return 3 + } + } + planned := make([]domain.Revision, 0, len(revs)) + for pass := 0; pass <= 4; pass++ { + for i := len(revs) - 1; i >= 0; i-- { + if rank(revs[i]) == pass { + planned = append(planned, revs[i]) + } + } + } + return planned +} + +// applyInverse undoes one revision, returning the changes it made (to be recorded +// in the revert's own change set), or a conflict describing why it declined. A +// returned error is an infrastructure failure and aborts the revert; a conflict +// is a normal outcome that skips just this entity. +// +// applied carries the versions this revert has already written, so a change set +// holding several revisions for one entity doesn't mistake its own earlier work +// for someone else's edit. +func (s *Service) applyInverse(ctx context.Context, r domain.Revision, applied map[entityKey]int64) ([]change, *domain.RevertConflict, error) { + switch r.EntityType { + case domain.EntityObject: + return revertEntity(ctx, s, r, applied, objectRevertOps(s)) + case domain.EntityPlanting: + return revertEntity(ctx, s, r, applied, plantingRevertOps(s)) + case domain.EntityGarden: + return revertEntity(ctx, s, r, applied, gardenRevertOps(s)) + default: + return nil, conflict(r, domain.ConflictUnsupported, ""), nil + } +} + +// revertOps is everything reverting one entity type needs. The three +// implementations differ only in which store methods they call and how a row +// names itself; the ordering, guards and conflict reporting are shared below so +// they cannot drift apart. +type revertOps[T any] struct { + entityType string + get func(ctx context.Context, id int64) (*T, error) + update func(ctx context.Context, row *T) (*T, error) + // restore re-inserts a deleted row under its original id. nil means this + // entity type is never deleted (gardens), and a delete revision for it is + // reported as unsupported rather than silently skipped. + restore func(ctx context.Context, row *T) (*T, error) + // remove undoes a creation. It returns the changes it made, because deleting + // an object also deletes the plantings hanging off it. + remove func(ctx context.Context, row *T) ([]change, error) + // blockRemoval optionally refuses a removal, e.g. an object that has gained + // plantings since. Returns a conflict reason, or "" to allow it. + blockRemoval func(ctx context.Context, row *T) (string, error) + version func(row *T) int64 + setVersion func(row *T, v int64) + label func(row *T) string +} + +// revertEntity applies the inverse of one revision for one entity type. +func revertEntity[T any]( + ctx context.Context, s *Service, r domain.Revision, applied map[entityKey]int64, ops revertOps[T], +) ([]change, *domain.RevertConflict, error) { + key := keyOf(r) + + // Inverse of a delete is a restore, under the original id. + if r.Op == domain.OpDelete { + if ops.restore == nil { + return nil, conflict(r, domain.ConflictUnsupported, ""), nil + } + if existing, err := ops.get(ctx, r.EntityID); err == nil { + return nil, conflict(r, domain.ConflictExists, ops.label(existing)), nil + } else if !errors.Is(err, domain.ErrNotFound) { + return nil, nil, err + } + var target T + if err := unsnapshot(r.Before, &target); err != nil { + return nil, nil, err + } + restored, err := ops.restore(ctx, &target) + if err != nil { + // The id may have been taken between the check above and this insert. + // Re-check rather than surfacing a driver-specific constraint error: + // "someone else has that id now" is a conflict, not a failure. + if existing, gerr := ops.get(ctx, r.EntityID); gerr == nil { + return nil, conflict(r, domain.ConflictExists, ops.label(existing)), nil + } + return nil, nil, err + } + applied[key] = ops.version(restored) + return []change{changeCreate(ops.entityType, r.EntityID, restored)}, nil, nil + } + + cur, err := ops.get(ctx, r.EntityID) + if errors.Is(err, domain.ErrNotFound) { + if r.Op == domain.OpCreate { + return nil, nil, nil // already gone: the inverse is a no-op, not a conflict + } + return nil, conflict(r, domain.ConflictMissing, ""), nil + } + if err != nil { + return nil, nil, err + } + if c := versionGuard(r, ops.version(cur), ops.label(cur), applied[key]); c != nil { + return nil, c, nil + } + + // Inverse of a create is a removal. + if r.Op == domain.OpCreate { + if ops.remove == nil { + return nil, conflict(r, domain.ConflictUnsupported, ""), nil + } + if ops.blockRemoval != nil { + reason, err := ops.blockRemoval(ctx, cur) + if err != nil { + return nil, nil, err + } + if reason != "" { + return nil, conflict(r, reason, ops.label(cur)), nil + } + } + changes, err := ops.remove(ctx, cur) + if err != nil { + return nil, nil, err + } + delete(applied, key) + return changes, nil, nil + } + + if r.Op != domain.OpUpdate { + return nil, conflict(r, domain.ConflictUnsupported, ""), nil + } + var target T + if err := unsnapshot(r.Before, &target); err != nil { + return nil, nil, err + } + ops.setVersion(&target, ops.version(cur)) + updated, err := ops.update(ctx, &target) + if errors.Is(err, domain.ErrVersionConflict) { + return nil, conflict(r, domain.ConflictChanged, ops.label(cur)), nil + } + if err != nil { + return nil, nil, err + } + applied[key] = ops.version(updated) + return []change{changeUpdate(ops.entityType, r.EntityID, cur, updated)}, nil, nil +} + +func objectRevertOps(s *Service) revertOps[domain.GardenObject] { + return revertOps[domain.GardenObject]{ + entityType: domain.EntityObject, + get: s.store.GetObject, + update: s.store.UpdateObject, + restore: s.store.RestoreObject, + remove: s.deleteObjectRecording, + // Undoing "added a bed" would cascade away anything planted in it since. + // Those plops are snapshotted, so it would be recoverable — but deleting + // someone's plants as a side effect of an unrelated undo should be + // reported, not performed quietly. + blockRemoval: func(ctx context.Context, o *domain.GardenObject) (string, error) { + plops, err := s.store.ListPlantingsForObject(ctx, o.ID) + if err != nil { + return "", err + } + if len(plops) > 0 { + return domain.ConflictChanged, nil + } + return "", nil + }, + version: func(o *domain.GardenObject) int64 { return o.Version }, + setVersion: func(o *domain.GardenObject, v int64) { o.Version = v }, + label: func(o *domain.GardenObject) string { return o.Name }, + } +} + +func plantingRevertOps(s *Service) revertOps[domain.Planting] { + return revertOps[domain.Planting]{ + entityType: domain.EntityPlanting, + get: s.store.GetPlanting, + update: s.store.UpdatePlanting, + restore: s.store.RestorePlanting, + remove: func(ctx context.Context, p *domain.Planting) ([]change, error) { + if err := s.store.DeletePlanting(ctx, p.ID); err != nil { + return nil, err + } + return []change{changeDelete(domain.EntityPlanting, p.ID, p)}, nil + }, + version: func(p *domain.Planting) int64 { return p.Version }, + setVersion: func(p *domain.Planting, v int64) { p.Version = v }, + label: func(p *domain.Planting) string { return "" }, + } +} + +// gardenRevertOps has no restore or remove: garden creation and deletion are +// never recorded (see the migration), so metadata updates are all that can reach +// here, and anything else is honestly reported as unsupported. +func gardenRevertOps(s *Service) revertOps[domain.Garden] { + return revertOps[domain.Garden]{ + entityType: domain.EntityGarden, + get: s.store.GetGarden, + update: s.store.UpdateGarden, + version: func(g *domain.Garden) int64 { return g.Version }, + setVersion: func(g *domain.Garden, v int64) { g.Version = v }, + label: func(g *domain.Garden) string { return g.Name }, + } +} + +// deleteObjectRecording deletes an object and returns every change to record. +// Deleting an object cascades its plantings away in SQLite without the service +// ever seeing them, so they are snapshotted first — otherwise the delete could be +// listed in history but never reverted. Shared by DeleteObject and the revert of +// an object creation, so both stay honest about the cascade. +func (s *Service) deleteObjectRecording(ctx context.Context, o *domain.GardenObject) ([]change, error) { + plops, err := s.store.ListPlantingsForObject(ctx, o.ID) + if err != nil { + return nil, err + } + if err := s.store.DeleteObject(ctx, o.ID); err != nil { + return nil, err + } + changes := make([]change, 0, len(plops)+1) + changes = append(changes, changeDelete(domain.EntityObject, o.ID, o)) + for i := range plops { + p := plops[i] + changes = append(changes, changeDelete(domain.EntityPlanting, p.ID, &p)) + } + return changes, nil +} + +// versionGuard reports a conflict when the row's current version differs from the +// one the change set left behind — i.e. something edited it since, and reverting +// would silently discard that edit. A revision with no after-snapshot (a delete) +// has nothing to compare and passes. +// +// appliedVersion is non-zero when THIS revert already wrote to this row, which +// happens whenever a change set holds more than one revision for the same entity. +// In that case the live version is our own doing and is the correct thing to +// expect — comparing against the snapshot would flag our own work as a conflict. +func versionGuard(r domain.Revision, currentVersion int64, name string, appliedVersion int64) *domain.RevertConflict { + if r.After == nil { + return nil + } + expected := appliedVersion + if expected == 0 { + var after struct { + Version int64 `json:"version"` + } + if err := json.Unmarshal([]byte(*r.After), &after); err != nil { + return conflict(r, domain.ConflictUnsupported, name) + } + expected = after.Version + } + if expected != currentVersion { + return conflict(r, domain.ConflictChanged, name) + } + return nil +} + +func conflict(r domain.Revision, reason, name string) *domain.RevertConflict { + return &domain.RevertConflict{EntityType: r.EntityType, EntityID: r.EntityID, Reason: reason, Name: name} +} + +// unsnapshot parses a JSON row snapshot back into a row struct. A missing +// snapshot where one is required is a corrupt revision, not a normal outcome. +func unsnapshot(s *string, into any) error { + if s == nil { + return fmt.Errorf("service: revision is missing the snapshot it needs") + } + if err := json.Unmarshal([]byte(*s), into); err != nil { + return fmt.Errorf("service: parse snapshot: %w", err) + } + return nil +} + +// revertSummary describes the revert in the history list. Reverting a revert +// reads as "Redid …" rather than a stack of nested "Undid" prefixes. +func revertSummary(target *domain.ChangeSet) string { + verb := "Undid" + if target.RevertsID != nil { + verb = "Redid" + } + if target.Summary == "" { + return verb + " an earlier change" + } + return verb + ": " + target.Summary +} diff --git a/internal/service/revisions_test.go b/internal/service/revisions_test.go new file mode 100644 index 0000000..2963b47 --- /dev/null +++ b/internal/service/revisions_test.go @@ -0,0 +1,706 @@ +package service + +import ( + "context" + "encoding/json" + "errors" + "strings" + "testing" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// history returns a garden's change sets newest-first, failing the test on error. +func history(t *testing.T, s *Service, actor, gardenID int64) []domain.ChangeSet { + t.Helper() + sets, _, err := s.GardenHistory(context.Background(), actor, gardenID, 0, 0) + if err != nil { + t.Fatalf("GardenHistory: %v", err) + } + return sets +} + +// revisionsOf loads a change set's revisions. +func revisionsOf(t *testing.T, s *Service, id int64) []domain.Revision { + t.Helper() + cs, err := s.store.GetChangeSet(context.Background(), id) + if err != nil { + t.Fatalf("GetChangeSet: %v", err) + } + return cs.Revisions +} + +// TestAutoScopeRecordsEveryMutation is the acceptance criterion that keeps this +// feature shallow: a plain REST-shaped mutation, with nobody opening a change +// set, still lands in history. +func TestAutoScopeRecordsEveryMutation(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + bed := seedBed(t, s, owner, g.ID) + if _, err := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{Name: strPtr("North Bed")}, bed.Version); err != nil { + t.Fatalf("UpdateObject: %v", err) + } + + sets := history(t, s, owner, g.ID) + if len(sets) != 2 { + t.Fatalf("got %d change sets, want 2 (create + update)", len(sets)) + } + // Newest first. + if sets[0].Summary != "Edited North Bed" { + t.Errorf("newest summary = %q", sets[0].Summary) + } + if sets[0].Source != domain.SourceUI { + t.Errorf("source = %q, want ui", sets[0].Source) + } + if sets[0].ActorID != owner || sets[0].ActorName == "" { + t.Errorf("actor not resolved: %+v", sets[0]) + } + if len(sets[0].Counts) != 1 || sets[0].Counts[0].Op != domain.OpUpdate || sets[0].Counts[0].N != 1 { + t.Errorf("counts = %+v", sets[0].Counts) + } +} + +// TestFillRegionIsOneChangeSet: N plops, one undo. The whole reason the unit of +// undo is the operation and not the row. +func TestFillRegionIsOneChangeSet(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + plant := seedOwnPlant(t, s, owner, 15) + ctx := context.Background() + + created, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil) + if err != nil { + t.Fatalf("FillNamedRegion: %v", err) + } + if len(created) < 2 { + t.Fatalf("expected a multi-plop fill, got %d", len(created)) + } + + sets := history(t, s, owner, g.ID) + fill := sets[0] + revs := revisionsOf(t, s, fill.ID) + if len(revs) != len(created) { + t.Fatalf("fill produced %d revisions for %d plops", len(revs), len(created)) + } + for _, r := range revs { + if r.Op != domain.OpCreate || r.EntityType != domain.EntityPlanting { + t.Errorf("unexpected revision %+v", r) + } + if r.Before != nil || r.After == nil { + t.Errorf("a create should snapshot only the after state: %+v", r) + } + } + + // Reverting removes all N and leaves the bed as it was. + _, conflicts, err := s.RevertChangeSet(ctx, owner, fill.ID, domain.SourceUI) + if err != nil { + t.Fatalf("RevertChangeSet: %v", err) + } + if len(conflicts) != 0 { + t.Fatalf("unexpected conflicts: %+v", conflicts) + } + active, err := s.store.ListActivePlantingsForObject(ctx, bed.ID) + if err != nil { + t.Fatalf("list: %v", err) + } + if len(active) != 0 { + t.Errorf("%d plops survived the revert", len(active)) + } +} + +// TestRevertRestoresObjectGeometry covers the everyday case: move a bed, undo it. +func TestRevertRestoresObjectGeometry(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + moved, err := s.UpdateObject(ctx, owner, bed.ID, + ObjectPatch{XCM: f64Ptr(300), YCM: f64Ptr(400)}, bed.Version) + if err != nil { + t.Fatalf("UpdateObject: %v", err) + } + + sets := history(t, s, owner, g.ID) + if _, conflicts, err := s.RevertChangeSet(ctx, owner, sets[0].ID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("revert: err=%v conflicts=%+v", err, conflicts) + } + + back, err := s.store.GetObject(ctx, bed.ID) + if err != nil { + t.Fatalf("GetObject: %v", err) + } + if back.XCM != bed.XCM || back.YCM != bed.YCM { + t.Errorf("position = (%v,%v), want the original (%v,%v)", back.XCM, back.YCM, bed.XCM, bed.YCM) + } + // The revert is a WRITE, not a rewind: the row's version moves forward. + if back.Version <= moved.Version { + t.Errorf("version = %d, want > %d (revert is an append-only change)", back.Version, moved.Version) + } +} + +// TestRevertOfRevertRestoresTheChange — undo is undoable, because a revert is +// itself a change set rather than a rewrite of history. +func TestRevertOfRevertRestoresTheChange(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + if _, err := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{XCM: f64Ptr(300)}, bed.Version); err != nil { + t.Fatalf("UpdateObject: %v", err) + } + move := history(t, s, owner, g.ID)[0] + + undo, _, err := s.RevertChangeSet(ctx, owner, move.ID, domain.SourceUI) + if err != nil { + t.Fatalf("revert: %v", err) + } + if undo.RevertsID == nil || *undo.RevertsID != move.ID { + t.Fatalf("revert doesn't point at its target: %+v", undo) + } + if o, _ := s.store.GetObject(ctx, bed.ID); o.XCM != bed.XCM { + t.Fatalf("undo didn't restore x: %v", o.XCM) + } + + redo, conflicts, err := s.RevertChangeSet(ctx, owner, undo.ID, domain.SourceUI) + if err != nil || len(conflicts) != 0 { + t.Fatalf("revert of revert: err=%v conflicts=%+v", err, conflicts) + } + if o, _ := s.store.GetObject(ctx, bed.ID); o.XCM != 300 { + t.Errorf("redo didn't reapply the move: x = %v", o.XCM) + } + if redo.Summary == "" { + t.Error("a revert should carry a summary") + } + + // The original is marked as reverted, and every step is still in the list. + sets := history(t, s, owner, g.ID) + if len(sets) != 4 { // create bed, move, undo, redo + t.Fatalf("got %d change sets, want 4", len(sets)) + } + var moveEntry *domain.ChangeSet + for i := range sets { + if sets[i].ID == move.ID { + moveEntry = &sets[i] + } + } + if moveEntry == nil || moveEntry.RevertedByID == nil || *moveEntry.RevertedByID != undo.ID { + t.Errorf("the move isn't marked as reverted: %+v", moveEntry) + } +} + +// TestRevertConflictsOnLaterEdit is the guard that stops undo from silently +// discarding someone's newer work — and the promise that the REST of the change +// set still reverts. +func TestRevertConflictsOnLaterEdit(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + bedA := seedBed(t, s, owner, g.ID) + bedB, err := s.CreateObject(ctx, owner, g.ID, ObjectInput{ + Kind: domain.KindBed, Name: "B", XCM: 200, YCM: 200, WidthCM: 100, HeightCM: 100, + }) + if err != nil { + t.Fatalf("CreateObject: %v", err) + } + + // One change set touching both beds. + _, err = s.WithChangeSet(ctx, owner, g.ID, ChangeSetOptions{Source: domain.SourceAgent, Summary: "Rearranged"}, + func(ctx context.Context) error { + if _, err := s.UpdateObject(ctx, owner, bedA.ID, ObjectPatch{XCM: f64Ptr(600)}, bedA.Version); err != nil { + return err + } + _, err := s.UpdateObject(ctx, owner, bedB.ID, ObjectPatch{XCM: f64Ptr(300)}, bedB.Version) + return err + }) + if err != nil { + t.Fatalf("WithChangeSet: %v", err) + } + turn := history(t, s, owner, g.ID)[0] + if turn.Source != domain.SourceAgent || len(revisionsOf(t, s, turn.ID)) != 2 { + t.Fatalf("expected one agent change set with 2 revisions: %+v", turn) + } + + // Someone edits bed A afterwards. + cur, _ := s.store.GetObject(ctx, bedA.ID) + if _, err := s.UpdateObject(ctx, owner, bedA.ID, ObjectPatch{Name: strPtr("Touched")}, cur.Version); err != nil { + t.Fatalf("later edit: %v", err) + } + + _, conflicts, err := s.RevertChangeSet(ctx, owner, turn.ID, domain.SourceUI) + if err != nil { + t.Fatalf("RevertChangeSet: %v", err) + } + if len(conflicts) != 1 { + t.Fatalf("got %d conflicts, want 1: %+v", len(conflicts), conflicts) + } + if conflicts[0].EntityID != bedA.ID || conflicts[0].Reason != domain.ConflictChanged { + t.Errorf("unexpected conflict %+v", conflicts[0]) + } + if conflicts[0].Name != "Touched" { + t.Errorf("conflict should name the entity as it stands now, got %q", conflicts[0].Name) + } + + // A was left alone; B still reverted. + a, _ := s.store.GetObject(ctx, bedA.ID) + if a.XCM != 600 || a.Name != "Touched" { + t.Errorf("conflicted object was modified: %+v", a) + } + b, _ := s.store.GetObject(ctx, bedB.ID) + if b.XCM != bedB.XCM { + t.Errorf("bed B should have reverted to %v, got %v", bedB.XCM, b.XCM) + } +} + +// TestRevertRestoresDeletedObjectWithItsPlantings covers the cascade: deleting an +// object takes its plops with it at the FK level, where the service never sees +// them. If they aren't snapshotted the delete is listed in history but can't +// actually be undone. +func TestRevertRestoresDeletedObjectWithItsPlantings(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + plant := seedOwnPlant(t, s, owner, 15) + ctx := context.Background() + + plop, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{ + PlantID: plant.ID, XCM: 10, YCM: 10, RadiusCM: 20, + }) + if err != nil { + t.Fatalf("CreatePlanting: %v", err) + } + if err := s.DeleteObject(ctx, owner, bed.ID); err != nil { + t.Fatalf("DeleteObject: %v", err) + } + + del := history(t, s, owner, g.ID)[0] + revs := revisionsOf(t, s, del.ID) + if len(revs) != 2 { + t.Fatalf("delete recorded %d revisions, want 2 (the bed and its plop)", len(revs)) + } + + if _, conflicts, err := s.RevertChangeSet(ctx, owner, del.ID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("revert: err=%v conflicts=%+v", err, conflicts) + } + + back, err := s.store.GetObject(ctx, bed.ID) + if err != nil { + t.Fatalf("bed not restored: %v", err) + } + if back.ID != bed.ID || back.Name != bed.Name || back.XCM != bed.XCM { + t.Errorf("restored bed differs: %+v", back) + } + restoredPlop, err := s.store.GetPlanting(ctx, plop.ID) + if err != nil { + t.Fatalf("plop not restored: %v", err) + } + if restoredPlop.ObjectID != bed.ID || restoredPlop.XCM != plop.XCM { + t.Errorf("restored plop differs: %+v", restoredPlop) + } +} + +// TestRevertClearObject: "clear bed" is a bulk soft-remove, and undoing it should +// bring the whole bed back planted. +func TestRevertClearObject(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + plant := seedOwnPlant(t, s, owner, 15) + ctx := context.Background() + + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil); err != nil { + t.Fatalf("fill: %v", err) + } + before, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID) + + n, err := s.ClearObject(ctx, owner, bed.ID) + if err != nil { + t.Fatalf("ClearObject: %v", err) + } + if n != len(before) { + t.Fatalf("cleared %d of %d", n, len(before)) + } + if active, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID); len(active) != 0 { + t.Fatalf("%d plops still active after clear", len(active)) + } + + clear := history(t, s, owner, g.ID)[0] + if _, conflicts, err := s.RevertChangeSet(ctx, owner, clear.ID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("revert: err=%v conflicts=%+v", err, conflicts) + } + after, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID) + if len(after) != len(before) { + t.Errorf("%d plops active after undo, want %d", len(after), len(before)) + } +} + +// TestRevertPermissions: a viewer may read history but not undo; a stranger sees +// neither (existence stays masked). +func TestRevertPermissions(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + viewer := seedUser(t, s, "v@example.com") + stranger := seedUser(t, s, "s@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + if _, err := s.AddShare(ctx, owner, g.ID, "v@example.com", domain.RoleViewer); err != nil { + t.Fatalf("AddShare: %v", err) + } + _ = bed + cs := history(t, s, owner, g.ID)[0] + + if _, _, err := s.GardenHistory(ctx, viewer, g.ID, 0, 0); err != nil { + t.Errorf("a viewer should be able to read history: %v", err) + } + if _, _, err := s.RevertChangeSet(ctx, viewer, cs.ID, domain.SourceUI); !errors.Is(err, domain.ErrForbidden) { + t.Errorf("viewer revert err = %v, want ErrForbidden", err) + } + if _, _, err := s.GardenHistory(ctx, stranger, g.ID, 0, 0); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("stranger history err = %v, want ErrNotFound", err) + } + if _, _, err := s.RevertChangeSet(ctx, stranger, cs.ID, domain.SourceUI); !errors.Is(err, domain.ErrNotFound) { + t.Errorf("stranger revert err = %v, want ErrNotFound", err) + } +} + +// TestWithChangeSetRecordsWhatCommittedOnFailure — a turn that fails partway has +// still committed the mutations it got through, because this scope buffers +// HISTORY, not data. Dropping the buffer would leave those changes with no +// history and no way to undo them, which is exactly the situation undo exists +// for. So they're recorded, marked as partial, and the failure is still reported. +func TestWithChangeSetRecordsWhatCommittedOnFailure(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + sentinel := errors.New("boom") + _, err := s.WithChangeSet(ctx, owner, g.ID, ChangeSetOptions{Source: domain.SourceAgent, Summary: "Half a turn"}, + func(ctx context.Context) error { + if _, err := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{XCM: f64Ptr(600)}, bed.Version); err != nil { + return err + } + return sentinel + }) + if !errors.Is(err, sentinel) { + t.Fatalf("err = %v, want the sentinel", err) + } + + // The move really happened, so it must be in history and undoable. + if o, _ := s.store.GetObject(ctx, bed.ID); o.XCM != 600 { + t.Fatalf("the committed move was rolled back? x = %v", o.XCM) + } + sets := history(t, s, owner, g.ID) + partial := sets[0] + if !strings.Contains(partial.Summary, "failed partway") { + t.Errorf("summary = %q, want it to say the turn failed partway", partial.Summary) + } + if len(revisionsOf(t, s, partial.ID)) != 1 { + t.Errorf("expected the one committed mutation to be recorded") + } + if _, conflicts, err := s.RevertChangeSet(ctx, owner, partial.ID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("the partial turn should be undoable: err=%v conflicts=%+v", err, conflicts) + } + if o, _ := s.store.GetObject(ctx, bed.ID); o.XCM != bed.XCM { + t.Errorf("undo of the partial turn didn't restore x: %v", o.XCM) + } +} + +// TestChangeSetSkippedWhenNothingChanged — a scope that mutates nothing writes no +// change set, so history isn't padded with empty entries. +func TestChangeSetSkippedWhenNothingChanged(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + before := len(history(t, s, owner, g.ID)) + cs, err := s.WithChangeSet(ctx, owner, g.ID, ChangeSetOptions{Source: domain.SourceAgent, Summary: "Looked around"}, + func(context.Context) error { return nil }) + if err != nil { + t.Fatalf("WithChangeSet: %v", err) + } + if cs != nil { + t.Errorf("expected no change set, got %+v", cs) + } + if got := len(history(t, s, owner, g.ID)); got != before { + t.Errorf("history grew by %d", got-before) + } +} + +// TestGardenMetadataRevert covers the third entity type. +func TestGardenMetadataRevert(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + if _, err := s.UpdateGarden(ctx, owner, g.ID, GardenInput{ + Name: "Renamed", WidthCM: 1200, HeightCM: 1000, UnitPref: domain.UnitImperial, + }, g.Version); err != nil { + t.Fatalf("UpdateGarden: %v", err) + } + cs := history(t, s, owner, g.ID)[0] + if _, conflicts, err := s.RevertChangeSet(ctx, owner, cs.ID, domain.SourceUI); err != nil || len(conflicts) != 0 { + t.Fatalf("revert: err=%v conflicts=%+v", err, conflicts) + } + back, _ := s.store.GetGarden(ctx, g.ID) + if back.Name != g.Name || back.WidthCM != g.WidthCM { + t.Errorf("garden not restored: %+v", back) + } + // MyRole is computed, never stored — the snapshot must not have carried it + // back into the row. + if back.MyRole != "" { + t.Errorf("MyRole leaked into the stored row: %q", back.MyRole) + } +} + +// TestPlanRevertOrdersRestoresParentFirst pins the ordering the FKs depend on, +// rather than trusting reverse-seq to get it right by luck. +func TestPlanRevertOrdersRestoresParentFirst(t *testing.T) { + revs := []domain.Revision{ + {Seq: 1, EntityType: domain.EntityPlanting, EntityID: 10, Op: domain.OpDelete}, + {Seq: 2, EntityType: domain.EntityObject, EntityID: 1, Op: domain.OpDelete}, + {Seq: 3, EntityType: domain.EntityObject, EntityID: 2, Op: domain.OpCreate}, + {Seq: 4, EntityType: domain.EntityPlanting, EntityID: 11, Op: domain.OpCreate}, + {Seq: 5, EntityType: domain.EntityObject, EntityID: 3, Op: domain.OpUpdate}, + } + got := planRevert(revs) + want := []struct { + entity string + id int64 + }{ + {domain.EntityObject, 1}, // restore parents first + {domain.EntityPlanting, 10}, // then children + {domain.EntityObject, 3}, // then updates + {domain.EntityPlanting, 11}, // then delete created children + {domain.EntityObject, 2}, // and only then their parents + } + if len(got) != len(want) { + t.Fatalf("planned %d revisions, want %d", len(got), len(want)) + } + for i, w := range want { + if got[i].EntityType != w.entity || got[i].EntityID != w.id { + t.Errorf("step %d = %s/%d, want %s/%d", i, got[i].EntityType, got[i].EntityID, w.entity, w.id) + } + } +} + +// TestSnapshotsCaptureVersion — the revert guard compares against the version in +// the after-snapshot, so it has to actually be there. +func TestSnapshotsCaptureVersion(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + + revs := revisionsOf(t, s, history(t, s, owner, g.ID)[0].ID) + if len(revs) != 1 || revs[0].After == nil { + t.Fatalf("unexpected revisions %+v", revs) + } + var snap domain.GardenObject + if err := json.Unmarshal([]byte(*revs[0].After), &snap); err != nil { + t.Fatalf("snapshot isn't a garden object: %v", err) + } + if snap.Version != bed.Version || snap.ID != bed.ID { + t.Errorf("snapshot = %+v, want version %d id %d", snap, bed.Version, bed.ID) + } +} + +func f64Ptr(v float64) *float64 { return &v } + +// TestRevertChangeSetTouchingOneEntityTwice — an agent turn that moves a bed and +// then renames it holds two revisions for the same object. Each inverse bumps +// that row's version, so from the second one on the snapshot's version no longer +// matches the live row. Without tracking what the revert itself just wrote, the +// guard flags our own work as somebody else's edit and the undo half-fails. +func TestRevertChangeSetTouchingOneEntityTwice(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + _, err := s.WithChangeSet(ctx, owner, g.ID, ChangeSetOptions{Source: domain.SourceAgent, Summary: "Two edits"}, + func(ctx context.Context) error { + moved, err := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{XCM: f64Ptr(600)}, bed.Version) + if err != nil { + return err + } + _, err = s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{Name: strPtr("Renamed")}, moved.Version) + return err + }) + if err != nil { + t.Fatalf("WithChangeSet: %v", err) + } + turn := history(t, s, owner, g.ID)[0] + if len(revisionsOf(t, s, turn.ID)) != 2 { + t.Fatalf("expected 2 revisions for the same object") + } + + _, conflicts, err := s.RevertChangeSet(ctx, owner, turn.ID, domain.SourceUI) + if err != nil { + t.Fatalf("RevertChangeSet: %v", err) + } + if len(conflicts) != 0 { + t.Fatalf("the revert conflicted with its own work: %+v", conflicts) + } + back, _ := s.store.GetObject(ctx, bed.ID) + if back.XCM != bed.XCM || back.Name != bed.Name { + t.Errorf("both edits should have been undone, got x=%v name=%q", back.XCM, back.Name) + } +} + +// TestRevertOfObjectCreateRefusesWhenPlanted — undoing "added a bed" would +// cascade away anything planted in it since. Those plops would be snapshotted and +// so recoverable, but deleting someone's plants as a side effect of an unrelated +// undo should be reported, not performed quietly. +func TestRevertOfObjectCreateRefusesWhenPlanted(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + bed := seedBed(t, s, owner, g.ID) + create := history(t, s, owner, g.ID)[0] + + plant := seedOwnPlant(t, s, owner, 15) + if _, err := s.CreatePlanting(ctx, owner, bed.ID, PlantingInput{ + PlantID: plant.ID, XCM: 0, YCM: 0, RadiusCM: 20, + }); err != nil { + t.Fatalf("CreatePlanting: %v", err) + } + + cs, conflicts, err := s.RevertChangeSet(ctx, owner, create.ID, domain.SourceUI) + if err != nil { + t.Fatalf("RevertChangeSet: %v", err) + } + if len(conflicts) != 1 || conflicts[0].EntityID != bed.ID { + t.Fatalf("expected one conflict naming the bed, got %+v", conflicts) + } + if cs != nil { + t.Errorf("nothing should have been reverted, got change set %+v", cs) + } + if _, err := s.store.GetObject(ctx, bed.ID); err != nil { + t.Errorf("the planted bed was deleted anyway: %v", err) + } +} + +// TestHistoryDoesNotDuplicateMultiplyRevertedEntries — more than one change set +// can point at the same target (undo, redo, undo again). Looking that up with a +// LEFT JOIN would emit one duplicate row per revert and silently corrupt the +// page, so it is a scalar subquery instead. Written against the store because +// getting a target legitimately reverted twice through the service is hard: the +// second attempt conflicts, which is itself the correct behaviour. +func TestHistoryDoesNotDuplicateMultiplyRevertedEntries(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + ctx := context.Background() + + target := history(t, s, owner, g.ID) // garden create isn't recorded; start empty + if len(target) != 0 { + t.Fatalf("expected an empty history, got %+v", target) + } + base, err := s.store.WriteChangeSet(ctx, &domain.ChangeSet{ + GardenID: g.ID, ActorID: owner, Source: domain.SourceUI, Summary: "Base", + }, []domain.Revision{{EntityType: domain.EntityGarden, EntityID: g.ID, Op: domain.OpUpdate}}) + if err != nil { + t.Fatalf("WriteChangeSet: %v", err) + } + for i := 0; i < 2; i++ { + if _, err := s.store.WriteChangeSet(ctx, &domain.ChangeSet{ + GardenID: g.ID, ActorID: owner, Source: domain.SourceUI, Summary: "Undo", RevertsID: &base.ID, + }, []domain.Revision{{EntityType: domain.EntityGarden, EntityID: g.ID, Op: domain.OpUpdate}}); err != nil { + t.Fatalf("WriteChangeSet revert %d: %v", i, err) + } + } + + sets := history(t, s, owner, g.ID) + if len(sets) != 3 { + t.Fatalf("got %d change sets, want 3 — a join would have duplicated the base row: %+v", len(sets), sets) + } + seen := map[int64]int{} + for _, cs := range sets { + seen[cs.ID]++ + } + for id, n := range seen { + if n != 1 { + t.Errorf("change set %d appears %d times", id, n) + } + } + for _, cs := range sets { + if cs.ID == base.ID && (cs.RevertedByID == nil || *cs.RevertedByID == 0) { + t.Error("the base change set should be marked as reverted") + } + } +} + +// TestRevertSourceIsRecorded — an agent undoing its own work must be +// distinguishable from a person clicking undo, or the history badge lies. +func TestRevertSourceIsRecorded(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + ctx := context.Background() + + if _, err := s.UpdateObject(ctx, owner, bed.ID, ObjectPatch{XCM: f64Ptr(300)}, bed.Version); err != nil { + t.Fatalf("UpdateObject: %v", err) + } + move := history(t, s, owner, g.ID)[0] + + undo, _, err := s.RevertChangeSet(ctx, owner, move.ID, domain.SourceAgent) + if err != nil { + t.Fatalf("revert: %v", err) + } + if undo.Source != domain.SourceAgent { + t.Errorf("source = %q, want agent", undo.Source) + } + if _, _, err := s.RevertChangeSet(ctx, owner, move.ID, "nonsense"); !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("an unknown source should be rejected, got %v", err) + } +} + +// TestClearObjectOnlyClearsWhatItSnapshotted — the clear targets the exact ids it +// read, so a plop created between the read and the UPDATE can't be removed +// without a revision to undo it by. +func TestClearObjectOnlyClearsWhatItSnapshotted(t *testing.T) { + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g := seedGarden(t, s, owner) + bed := seedBed(t, s, owner, g.ID) + plant := seedOwnPlant(t, s, owner, 15) + ctx := context.Background() + + if _, err := s.FillNamedRegion(ctx, owner, bed.ID, "all", plant.ID, nil); err != nil { + t.Fatalf("fill: %v", err) + } + before, _ := s.store.ListActivePlantingsForObject(ctx, bed.ID) + + n, err := s.ClearObject(ctx, owner, bed.ID) + if err != nil { + t.Fatalf("ClearObject: %v", err) + } + clear := history(t, s, owner, g.ID)[0] + revs := revisionsOf(t, s, clear.ID) + // Every row the clear touched has a revision; the count and the revisions agree. + if n != len(before) || len(revs) != n { + t.Errorf("cleared %d of %d with %d revisions — all three should match", n, len(before), len(revs)) + } +} diff --git a/internal/store/migrations/0006_revisions.sql b/internal/store/migrations/0006_revisions.sql new file mode 100644 index 0000000..3b4912f --- /dev/null +++ b/internal/store/migrations/0006_revisions.sql @@ -0,0 +1,53 @@ +-- Revision history: the undo substrate (#48). +-- +-- The unit of undo is the OPERATION, not the row. "Empty the garlic bed and +-- plant cucumbers" touches one object and many plantings; undoing half of that +-- is worse than useless. So a change_set groups the row-level revisions it +-- produced, and revert replays their inverses. +-- +-- Two properties this schema is built around: +-- * Revert is itself a change set (reverts_id points at its target). History is +-- append-only and never rewritten, so an undo can be undone. git revert, not +-- git reset. +-- * Every revision carries full JSON row snapshots INCLUDING version, which is +-- what the revert guard compares against so a later edit is reported as a +-- conflict rather than silently clobbered. +-- +-- JSON snapshots rather than a shadow table per entity: the rows are small, +-- household scale makes storage a non-issue, and one code path covers all three +-- entity types. +-- +-- Deliberate gap: garden DELETION is not revertible. Dropping a garden cascades +-- its objects and plantings without the service ever seeing them, and the +-- ON DELETE CASCADE below would take the history with it. The history UI says so +-- rather than pretending otherwise. +CREATE TABLE change_sets ( + id INTEGER PRIMARY KEY, + garden_id INTEGER NOT NULL REFERENCES gardens (id) ON DELETE CASCADE, + actor_id INTEGER NOT NULL REFERENCES users (id) ON DELETE CASCADE, + source TEXT NOT NULL CHECK (source IN ('ui', 'agent', 'api')), + summary TEXT NOT NULL DEFAULT '', + agent_run_id TEXT, -- executus run id when source='agent' + reverts_id INTEGER REFERENCES change_sets (id), -- set when this change set reverts another + created_at TEXT NOT NULL DEFAULT (strftime('%Y-%m-%dT%H:%M:%SZ', 'now')) +); + +-- The history panel's only query: one garden, newest first. +CREATE INDEX idx_change_sets_garden ON change_sets (garden_id, created_at DESC); + +-- "Has this change set been reverted?" — the list marks reverted entries, so the +-- lookup by target must not be a scan. +CREATE INDEX idx_change_sets_reverts ON change_sets (reverts_id) WHERE reverts_id IS NOT NULL; + +CREATE TABLE revisions ( + id INTEGER PRIMARY KEY, + change_set_id INTEGER NOT NULL REFERENCES change_sets (id) ON DELETE CASCADE, + seq INTEGER NOT NULL, -- order within the change set + entity_type TEXT NOT NULL CHECK (entity_type IN ('garden', 'object', 'planting')), + entity_id INTEGER NOT NULL, + op TEXT NOT NULL CHECK (op IN ('create', 'update', 'delete')), + before TEXT, -- JSON row snapshot; NULL for create + after TEXT -- JSON row snapshot; NULL for delete +); + +CREATE INDEX idx_revisions_change_set ON revisions (change_set_id, seq); diff --git a/internal/store/objects.go b/internal/store/objects.go index 03cf57b..da17ccf 100644 --- a/internal/store/objects.go +++ b/internal/store/objects.go @@ -61,6 +61,33 @@ func (d *DB) CreateObject(ctx context.Context, o *domain.GardenObject) (*domain. return created, nil } +// RestoreObject re-inserts a deleted object under its ORIGINAL id, preserving +// version and timestamps — the inverse of a delete, for reverting a change set +// (#48). SQLite allows an explicit INTEGER PRIMARY KEY insert once the row is +// gone; if the id has been taken again the insert fails on the primary key, which +// is the correct outcome (the service checks first and reports a conflict). +// +// Deliberately not routed through objectInsert: that omits id/version/timestamps +// because a normal create must let the database assign them. +func (d *DB) RestoreObject(ctx context.Context, o *domain.GardenObject) (*domain.GardenObject, error) { + restored, err := scanObject(d.sql.QueryRowContext(ctx, + `INSERT INTO garden_objects + (id, garden_id, kind, name, shape, points, x_cm, y_cm, width_cm, height_cm, + rotation_deg, z_index, plantable, color, props, grid_size_cm, snap_to_grid, notes, + version, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + strftime('%Y-%m-%dT%H:%M:%SZ', 'now')) + RETURNING `+objectColumns, + o.ID, o.GardenID, o.Kind, o.Name, o.Shape, o.Points, o.XCM, o.YCM, o.WidthCM, o.HeightCM, + o.RotationDeg, o.ZIndex, boolToInt(o.Plantable), o.Color, o.Props, + o.GridSizeCM, boolToInt(o.SnapToGrid), o.Notes, o.Version, o.CreatedAt, + )) + if err != nil { + return nil, fmt.Errorf("store: restore object: %w", err) + } + return restored, nil +} + // GetObject returns the object with the given id, or domain.ErrNotFound. func (d *DB) GetObject(ctx context.Context, id int64) (*domain.GardenObject, error) { o, err := scanObject(d.sql.QueryRowContext(ctx, diff --git a/internal/store/plantings.go b/internal/store/plantings.go index 1af1c9b..14c16f7 100644 --- a/internal/store/plantings.go +++ b/internal/store/plantings.go @@ -71,15 +71,59 @@ func (d *DB) ListActivePlantingsForObject(ctx context.Context, objectID int64) ( objectID) } -// ClearObjectPlantings soft-removes every active plop in an object in one UPDATE +// ListPlantingsForObject returns every plop in an object, removed ones included. +// Used when an object is deleted: the FK cascades its plantings away without the +// service seeing them, so they are snapshotted first or the delete would not be +// revertible. Always a non-nil slice. +func (d *DB) ListPlantingsForObject(ctx context.Context, objectID int64) ([]domain.Planting, error) { + return queryPlantings(ctx, d.sql, + `SELECT `+plantingColumns+` FROM plantings WHERE object_id = ? ORDER BY id`, + objectID) +} + +// RestorePlanting re-inserts a deleted plop under its ORIGINAL id, preserving +// version and timestamps — the plop counterpart of RestoreObject, and subject to +// the same reasoning. Its parent object must exist again first, or the FK +// rejects it; the revert orders object restores ahead of planting restores. +func (d *DB) RestorePlanting(ctx context.Context, p *domain.Planting) (*domain.Planting, error) { + restored, err := scanPlanting(d.sql.QueryRowContext(ctx, + `INSERT INTO plantings + (id, object_id, plant_id, x_cm, y_cm, radius_cm, count, label, planted_at, removed_at, + version, created_at, updated_at) + VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, + strftime('%Y-%m-%dT%H:%M:%SZ', 'now')) + RETURNING `+plantingColumns, + p.ID, p.ObjectID, p.PlantID, p.XCM, p.YCM, p.RadiusCM, p.Count, p.Label, + p.PlantedAt, p.RemovedAt, p.Version, p.CreatedAt, + )) + if err != nil { + return nil, fmt.Errorf("store: restore planting: %w", err) + } + return restored, nil +} + +// ClearObjectPlantings soft-removes the given plops of an object in one UPDATE // (sets removed_at=date, bumps version) and returns how many rows it affected. -func (d *DB) ClearObjectPlantings(ctx context.Context, objectID int64, date string) (int, error) { +// +// It takes explicit ids rather than clearing "every active plop" so the caller +// can snapshot exactly the rows it is about to change. Clearing by predicate +// would let a plop created between the caller's read and this UPDATE be removed +// without a revision — cleared, but with no way to undo it. +func (d *DB) ClearObjectPlantings(ctx context.Context, objectID int64, date string, ids []int64) (int, error) { + if len(ids) == 0 { + return 0, nil + } + args := make([]any, 0, len(ids)+2) + args = append(args, date, objectID) + for _, id := range ids { + args = append(args, id) + } res, err := d.sql.ExecContext(ctx, `UPDATE plantings SET removed_at = ?, version = version + 1, updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now') - WHERE object_id = ? AND removed_at IS NULL`, - date, objectID, + WHERE object_id = ? AND removed_at IS NULL AND id IN (`+placeholders(len(ids))+`)`, + args..., ) if err != nil { return 0, fmt.Errorf("store: clear object plantings: %w", err) diff --git a/internal/store/revisions.go b/internal/store/revisions.go new file mode 100644 index 0000000..669ebab --- /dev/null +++ b/internal/store/revisions.go @@ -0,0 +1,196 @@ +package store + +import ( + "context" + "database/sql" + "errors" + "fmt" + + "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" +) + +// changeSetColumns lists change_sets columns in the order scanChangeSet expects. +const changeSetColumns = `id, garden_id, actor_id, source, summary, agent_run_id, reverts_id, created_at` + +func scanChangeSet(s scanner) (*domain.ChangeSet, error) { + var cs domain.ChangeSet + if err := s.Scan( + &cs.ID, &cs.GardenID, &cs.ActorID, &cs.Source, &cs.Summary, + &cs.AgentRunID, &cs.RevertsID, &cs.CreatedAt, + ); err != nil { + return nil, err + } + return &cs, nil +} + +// revisionColumns lists revisions columns in the order scanRevision expects. +const revisionColumns = `id, change_set_id, seq, entity_type, entity_id, op, before, after` + +func scanRevision(s scanner) (*domain.Revision, error) { + var r domain.Revision + if err := s.Scan( + &r.ID, &r.ChangeSetID, &r.Seq, &r.EntityType, &r.EntityID, &r.Op, &r.Before, &r.After, + ); err != nil { + return nil, err + } + return &r, nil +} + +// WriteChangeSet inserts a change set and all of its revisions in one +// transaction, so history never records half an operation. The service buffers +// revisions while the operation runs and calls this once it has succeeded — +// which is also why an operation that fails partway leaves no change set behind. +// seq is assigned from the slice order. Returns the stored change set. +func (d *DB) WriteChangeSet(ctx context.Context, cs *domain.ChangeSet, revs []domain.Revision) (*domain.ChangeSet, error) { + tx, err := d.sql.BeginTx(ctx, nil) + if err != nil { + return nil, fmt.Errorf("store: begin change set: %w", err) + } + defer func() { _ = tx.Rollback() }() + + created, err := scanChangeSet(tx.QueryRowContext(ctx, + `INSERT INTO change_sets (garden_id, actor_id, source, summary, agent_run_id, reverts_id) + VALUES (?, ?, ?, ?, ?, ?) + RETURNING `+changeSetColumns, + cs.GardenID, cs.ActorID, cs.Source, cs.Summary, cs.AgentRunID, cs.RevertsID)) + if err != nil { + return nil, fmt.Errorf("store: insert change set: %w", err) + } + + for i := range revs { + r := &revs[i] + if _, err := tx.ExecContext(ctx, + `INSERT INTO revisions (change_set_id, seq, entity_type, entity_id, op, before, after) + VALUES (?, ?, ?, ?, ?, ?, ?)`, + created.ID, int64(i+1), r.EntityType, r.EntityID, r.Op, r.Before, r.After, + ); err != nil { + return nil, fmt.Errorf("store: insert revision: %w", err) + } + } + + if err := tx.Commit(); err != nil { + return nil, fmt.Errorf("store: commit change set: %w", err) + } + return created, nil +} + +// ListChangeSets returns a garden's change sets newest-first, one page at a time. +// Each carries its actor's display name, the id of the change set that reverted +// it (if any), and per-(entity,op) counts — everything the history list renders, +// without a second round-trip per row. Always a non-nil slice. +func (d *DB) ListChangeSets(ctx context.Context, gardenID int64, limit, offset int) ([]domain.ChangeSet, error) { + // The "was this reverted?" lookup is a scalar subquery, NOT a LEFT JOIN: a + // change set can be reverted more than once (undo, redo, undo again), and a + // join would then emit one duplicate row per revert and silently corrupt the + // page. MIN(id) names the first revert, which is the one worth showing. + rows, err := d.sql.QueryContext(ctx, + `SELECT `+qualifyColumns("cs", changeSetColumns)+`, u.display_name, + (SELECT MIN(r.id) FROM change_sets r WHERE r.reverts_id = cs.id) + FROM change_sets cs + JOIN users u ON u.id = cs.actor_id + WHERE cs.garden_id = ? + ORDER BY cs.id DESC + LIMIT ? OFFSET ?`, + gardenID, limit, offset) + if err != nil { + return nil, fmt.Errorf("store: list change sets: %w", err) + } + defer rows.Close() + + sets := []domain.ChangeSet{} + ids := []any{} + byID := map[int64]int{} + for rows.Next() { + var cs domain.ChangeSet + if err := rows.Scan( + &cs.ID, &cs.GardenID, &cs.ActorID, &cs.Source, &cs.Summary, + &cs.AgentRunID, &cs.RevertsID, &cs.CreatedAt, &cs.ActorName, &cs.RevertedByID, + ); err != nil { + return nil, fmt.Errorf("store: scan change set: %w", err) + } + byID[cs.ID] = len(sets) + ids = append(ids, cs.ID) + sets = append(sets, cs) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("store: iterate change sets: %w", err) + } + if len(sets) == 0 { + return sets, nil + } + + // One grouped query for the whole page rather than N per-row counts. + countRows, err := d.sql.QueryContext(ctx, + `SELECT change_set_id, entity_type, op, COUNT(*) + FROM revisions + WHERE change_set_id IN (`+placeholders(len(ids))+`) + GROUP BY change_set_id, entity_type, op + ORDER BY change_set_id, entity_type, op`, + ids...) + if err != nil { + return nil, fmt.Errorf("store: count revisions: %w", err) + } + defer countRows.Close() + for countRows.Next() { + var csID int64 + var c domain.ChangeCount + if err := countRows.Scan(&csID, &c.EntityType, &c.Op, &c.N); err != nil { + return nil, fmt.Errorf("store: scan revision count: %w", err) + } + if i, ok := byID[csID]; ok { + sets[i].Counts = append(sets[i].Counts, c) + } + } + if err := countRows.Err(); err != nil { + return nil, fmt.Errorf("store: iterate revision counts: %w", err) + } + return sets, nil +} + +// GetChangeSet returns one change set with its revisions loaded in seq order, or +// domain.ErrNotFound. +func (d *DB) GetChangeSet(ctx context.Context, id int64) (*domain.ChangeSet, error) { + cs, err := scanChangeSet(d.sql.QueryRowContext(ctx, + `SELECT `+changeSetColumns+` FROM change_sets WHERE id = ?`, id)) + if errors.Is(err, sql.ErrNoRows) { + return nil, domain.ErrNotFound + } + if err != nil { + return nil, fmt.Errorf("store: get change set: %w", err) + } + + rows, err := d.sql.QueryContext(ctx, + `SELECT `+revisionColumns+` FROM revisions WHERE change_set_id = ? ORDER BY seq`, id) + if err != nil { + return nil, fmt.Errorf("store: list revisions: %w", err) + } + defer rows.Close() + + cs.Revisions = []domain.Revision{} + for rows.Next() { + r, err := scanRevision(rows) + if err != nil { + return nil, fmt.Errorf("store: scan revision: %w", err) + } + cs.Revisions = append(cs.Revisions, *r) + } + if err := rows.Err(); err != nil { + return nil, fmt.Errorf("store: iterate revisions: %w", err) + } + return cs, nil +} + +// placeholders returns "?, ?, …" for an IN clause of n values. +func placeholders(n int) string { + if n <= 0 { + return "NULL" + } + b := make([]byte, 0, n*3) + for i := 0; i < n; i++ { + if i > 0 { + b = append(b, ',', ' ') + } + b = append(b, '?') + } + return string(b) +}