REST fill/clear on objects; clear-bed becomes one change set (#82)
FillRegion and ClearObject were reachable only through the agent toolbox, so
on an instance with no model configured the most valuable bulk operation in a
garden planner — and the one carrying the most carefully reasoned geometry in
the codebase — did not exist at all. internal/service/ops.go said these lived
on *Service so "any future REST surface" would inherit the ACL checks; this is
that surface.
POST /objects/:id/fill takes a region EITHER by compass name ("all", "ne",
"south half") or as an explicit rect in the object's local frame, and refuses
both-or-neither: silently preferring one would make a client bug look like a
geometry bug. It answers 200 rather than 201 because a fill can legitimately
create nothing (the region is already planted) and there is no single resource
to point a Location at.
POST /objects/:id/clear replaces a client-side loop of PATCHes. That loop
wrote one change set per plop, so clearing a 40-plop bed took 40 presses of
Undo to put back — while the agent's clear_object, for the identical
user-facing action, undid in one click. CLAUDE.md states the rule it violated:
multi-row operations record together so they undo as one unit. Moving it
server-side also removes the partial-failure case the loop had to reconcile.
TestClearObjectIsOneChangeSetAPI pins that: fill a bed, count change sets,
clear it, assert the count rose by exactly one.
DESIGN.md's API block gains the two new routes, and the four it was already
missing: GET/POST/DELETE /gardens/:id/share-link and the unauthenticated
GET /public/gardens/:token. An unauthenticated surface absent from the
architecture doc is the one worth fixing on sight.
Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
Claude-Session: https://claude.ai/code/session_01H3zbym8Doka2d7D48maSgZ
This commit is contained in:
@@ -0,0 +1,211 @@
|
||||
package api
|
||||
|
||||
import (
|
||||
"net/http"
|
||||
"testing"
|
||||
|
||||
"github.com/gin-gonic/gin"
|
||||
)
|
||||
|
||||
func fillPath(id int64) string { return objectPath(id) + "/fill" }
|
||||
func clearPath(id int64) string { return objectPath(id) + "/clear" }
|
||||
|
||||
// makeFillPlant creates a custom plant and returns its id.
|
||||
//
|
||||
// Deliberately named apart from the seed-lot tests' helper: that one arrives on
|
||||
// a separate branch, and two files declaring the same helper would collide on
|
||||
// whichever merged second.
|
||||
func makeFillPlant(t *testing.T, r *gin.Engine, cookie *http.Cookie, name string, spacing float64) int64 {
|
||||
t.Helper()
|
||||
w := doJSON(t, r, http.MethodPost, "/api/v1/plants", map[string]any{
|
||||
"name": name, "category": "vegetable", "spacingCm": spacing, "color": "#4a7c3f", "icon": "🌱",
|
||||
}, cookie)
|
||||
if w.Code != http.StatusCreated {
|
||||
t.Fatalf("create plant %q: status %d, body %s", name, w.Code, w.Body.String())
|
||||
}
|
||||
return int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
|
||||
}
|
||||
|
||||
// seedFillableBed makes a garden with one plantable bed and a custom plant,
|
||||
// returning (gardenID, objectID, plantID).
|
||||
func seedFillableBed(t *testing.T, r *gin.Engine, cookie *http.Cookie, w, h, spacing float64) (int64, int64, int64) {
|
||||
t.Helper()
|
||||
gid := createGardenAPI(t, r, cookie, "G")
|
||||
rec := doJSON(t, r, http.MethodPost, objectsPath(gid), map[string]any{
|
||||
"kind": "bed", "widthCm": w, "heightCm": h, "plantable": true,
|
||||
}, cookie)
|
||||
if rec.Code != http.StatusCreated {
|
||||
t.Fatalf("create bed: status %d, body %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
objID := int64(decodeMap(t, rec.Body.Bytes())["id"].(float64))
|
||||
plantID := makeFillPlant(t, r, cookie, "Fillable", spacing)
|
||||
return gid, objID, plantID
|
||||
}
|
||||
|
||||
// countChangeSets reads the history page and reports how many change sets exist.
|
||||
func countChangeSets(t *testing.T, r *gin.Engine, cookie *http.Cookie, gardenID int64) int {
|
||||
t.Helper()
|
||||
w := doJSON(t, r, http.MethodGet, historyPath(gardenID), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("history: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
sets, _ := decodeMap(t, w.Body.Bytes())["changeSets"].([]any)
|
||||
return len(sets)
|
||||
}
|
||||
|
||||
// TestFillAndClearAPI covers the two routes end to end through the router.
|
||||
//
|
||||
// These exist because both operations were previously reachable ONLY through the
|
||||
// agent toolbox, so on an instance with no model configured the most valuable
|
||||
// bulk operation in the app did not exist at all.
|
||||
func TestFillAndClearAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
gid, objID, plantID := seedFillableBed(t, r, cookie, 200, 200, 20)
|
||||
|
||||
// Fill by compass name.
|
||||
w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{
|
||||
"plantId": plantID, "region": "all",
|
||||
}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("fill: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
body := decodeMap(t, w.Body.Bytes())
|
||||
created := int(body["created"].(float64))
|
||||
if created == 0 {
|
||||
t.Fatalf("fill created nothing: %s", w.Body.String())
|
||||
}
|
||||
if plops, _ := body["plantings"].([]any); len(plops) != created {
|
||||
t.Errorf("created=%d but returned %d plantings", created, len(plops))
|
||||
}
|
||||
|
||||
// Clear it: one call, and it reports what it removed.
|
||||
w = doJSON(t, r, http.MethodPost, clearPath(objID), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("clear: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
if n := int(decodeMap(t, w.Body.Bytes())["cleared"].(float64)); n != created {
|
||||
t.Errorf("cleared %d, want %d (everything the fill made)", n, created)
|
||||
}
|
||||
|
||||
// Clearing an already-empty bed is a no-op, not an error.
|
||||
w = doJSON(t, r, http.MethodPost, clearPath(objID), nil, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("second clear: status %d", w.Code)
|
||||
}
|
||||
if n := int(decodeMap(t, w.Body.Bytes())["cleared"].(float64)); n != 0 {
|
||||
t.Errorf("second clear removed %d, want 0", n)
|
||||
}
|
||||
_ = gid
|
||||
}
|
||||
|
||||
// TestFillRegionSelectionAPI: exactly one of region/rect, and a rect fills only
|
||||
// its own corner of the bed.
|
||||
func TestFillRegionSelectionAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
_, objID, plantID := seedFillableBed(t, r, cookie, 400, 400, 20)
|
||||
|
||||
// Neither → 400. Both → 400. Accepting both and silently preferring one
|
||||
// would make a client bug look like a geometry bug.
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{"plantId": plantID}, cookie); w.Code != http.StatusBadRequest {
|
||||
t.Errorf("no region = %d, want 400", w.Code)
|
||||
}
|
||||
both := map[string]any{
|
||||
"plantId": plantID, "region": "all",
|
||||
"rect": map[string]any{"minXCm": -50, "minYCm": -50, "maxXCm": 50, "maxYCm": 50},
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), both, cookie); w.Code != http.StatusBadRequest {
|
||||
t.Errorf("both region and rect = %d, want 400", w.Code)
|
||||
}
|
||||
// An unknown compass name is rejected rather than silently filling nothing.
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{
|
||||
"plantId": plantID, "region": "middle-ish",
|
||||
}, cookie); w.Code != http.StatusBadRequest {
|
||||
t.Errorf("bad region name = %d, want 400", w.Code)
|
||||
}
|
||||
|
||||
// A rect confined to the NE corner produces plops only there. Local frame:
|
||||
// +x east, -y north.
|
||||
w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{
|
||||
"plantId": plantID,
|
||||
"rect": map[string]any{"minXCm": 0, "minYCm": -200, "maxXCm": 200, "maxYCm": 0},
|
||||
}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("rect fill: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
plops, _ := decodeMap(t, w.Body.Bytes())["plantings"].([]any)
|
||||
if len(plops) == 0 {
|
||||
t.Fatal("rect fill created nothing")
|
||||
}
|
||||
for _, raw := range plops {
|
||||
p := raw.(map[string]any)
|
||||
if x, y := p["xCm"].(float64), p["yCm"].(float64); x < 0 || y > 0 {
|
||||
t.Errorf("plop at (%v,%v) outside the NE rect", x, y)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// TestClearObjectIsOneChangeSetAPI is the regression test for the behaviour this
|
||||
// endpoint exists to restore.
|
||||
//
|
||||
// The UI used to clear a bed with a loop of PATCHes, and since every service
|
||||
// mutation auto-scopes its own change set, clearing a 40-plop bed wrote 40 of
|
||||
// them — 40 presses of Undo to put the bed back. CLAUDE.md states the rule
|
||||
// directly: multi-row operations record together so they undo as one unit.
|
||||
func TestClearObjectIsOneChangeSetAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
cookie := registerAndCookie(t, r, "[email protected]")
|
||||
gid, objID, plantID := seedFillableBed(t, r, cookie, 300, 300, 20)
|
||||
|
||||
w := doJSON(t, r, http.MethodPost, fillPath(objID), map[string]any{
|
||||
"plantId": plantID, "region": "all",
|
||||
}, cookie)
|
||||
if w.Code != http.StatusOK {
|
||||
t.Fatalf("fill: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
created := int(decodeMap(t, w.Body.Bytes())["created"].(float64))
|
||||
if created < 4 {
|
||||
t.Fatalf("need several plops to make this meaningful, got %d", created)
|
||||
}
|
||||
|
||||
before := countChangeSets(t, r, cookie, gid)
|
||||
if w := doJSON(t, r, http.MethodPost, clearPath(objID), nil, cookie); w.Code != http.StatusOK {
|
||||
t.Fatalf("clear: status %d", w.Code)
|
||||
}
|
||||
if after := countChangeSets(t, r, cookie, gid); after != before+1 {
|
||||
t.Errorf("clearing %d plops added %d change sets, want exactly 1", created, after-before)
|
||||
}
|
||||
}
|
||||
|
||||
// TestFillClearPermissionsAPI: a viewer may look but not fill or clear, and a
|
||||
// stranger gets 404 because existence is masked.
|
||||
func TestFillClearPermissionsAPI(t *testing.T) {
|
||||
r := authEngine(t, localCfg())
|
||||
owner := registerAndCookie(t, r, "[email protected]")
|
||||
viewer := registerAndCookie(t, r, "[email protected]")
|
||||
stranger := registerAndCookie(t, r, "[email protected]")
|
||||
|
||||
gid, objID, plantID := seedFillableBed(t, r, owner, 200, 200, 20)
|
||||
if w := doJSON(t, r, http.MethodPost, sharesPath(gid),
|
||||
map[string]any{"email": "[email protected]", "role": "viewer"}, owner); w.Code != http.StatusCreated {
|
||||
t.Fatalf("share as viewer: status %d, body %s", w.Code, w.Body.String())
|
||||
}
|
||||
|
||||
fillBody := map[string]any{"plantId": plantID, "region": "all"}
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), fillBody, viewer); w.Code != http.StatusForbidden {
|
||||
t.Errorf("viewer fill = %d, want 403 (they can see it but may not do that)", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, clearPath(objID), nil, viewer); w.Code != http.StatusForbidden {
|
||||
t.Errorf("viewer clear = %d, want 403", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), fillBody, stranger); w.Code != http.StatusNotFound {
|
||||
t.Errorf("stranger fill = %d, want 404 (existence masked)", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, clearPath(objID), nil, stranger); w.Code != http.StatusNotFound {
|
||||
t.Errorf("stranger clear = %d, want 404", w.Code)
|
||||
}
|
||||
if w := doJSON(t, r, http.MethodPost, fillPath(objID), fillBody, nil); w.Code != http.StatusUnauthorized {
|
||||
t.Errorf("anonymous fill = %d, want 401", w.Code)
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user