Plant catalog backend: CRUD + seeded built-ins (#12) #31

Merged
steve merged 2 commits from phase-4-plants-api into main 2026-07-19 01:48:23 +00:00
11 changed files with 871 additions and 4 deletions
Showing only changes of commit 78dbadf95c - Show all commits
+7
View File
@@ -90,6 +90,13 @@ func New(cfg *config.Config, svc *service.Service) *gin.Engine {
objects.PATCH("/:id", h.updateObject) objects.PATCH("/:id", h.updateObject)
objects.DELETE("/:id", h.deleteObject) objects.DELETE("/:id", h.deleteObject)
// Plant catalog: built-ins (seeded, read-only) plus the actor's own rows.
plants := v1.Group("/plants", h.requireAuth())
plants.GET("", h.listPlants)
plants.POST("", h.createPlant)
plants.PATCH("/:id", h.updatePlant)
plants.DELETE("/:id", h.deletePlant)
return r return r
} }
+2
View File
@@ -28,6 +28,8 @@ func writeServiceError(c *gin.Context, err error) {
writeAPIError(c, http.StatusForbidden, "FORBIDDEN", "you don't have access") writeAPIError(c, http.StatusForbidden, "FORBIDDEN", "you don't have access")
case errors.Is(err, domain.ErrVersionConflict): case errors.Is(err, domain.ErrVersionConflict):
writeAPIError(c, http.StatusConflict, "VERSION_CONFLICT", "the resource was modified; refetch and retry") writeAPIError(c, http.StatusConflict, "VERSION_CONFLICT", "the resource was modified; refetch and retry")
case errors.Is(err, domain.ErrPlantInUse):
writeAPIError(c, http.StatusConflict, "PLANT_IN_USE", "this plant is used by plantings and can't be deleted")
case errors.Is(err, domain.ErrInvalidCredentials): case errors.Is(err, domain.ErrInvalidCredentials):
writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password") writeAPIError(c, http.StatusUnauthorized, "INVALID_CREDENTIALS", "invalid email or password")
case errors.Is(err, domain.ErrEmailTaken): case errors.Is(err, domain.ErrEmailTaken):
+133
View File
@@ -0,0 +1,133 @@
package api
import (
"encoding/json"
"errors"
"net/http"
"github.com/gin-gonic/gin"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/service"
)
// plantCreateRequest is the body for POST /plants. spacingCm is centimeters;
// color is a hex string; icon is an emoji. daysToMaturity is optional.
type plantCreateRequest struct {
Name string `json:"name" binding:"required"`
Category string `json:"category" binding:"required"`
SpacingCM float64 `json:"spacingCm"`
Color string `json:"color" binding:"required"`
Icon string `json:"icon" binding:"required"`
DaysToMaturity *int `json:"daysToMaturity"`
Notes string `json:"notes"`
}
func (r plantCreateRequest) toInput() service.PlantInput {
return service.PlantInput{
Name: r.Name, Category: r.Category, SpacingCM: r.SpacingCM,
Color: r.Color, Icon: r.Icon, DaysToMaturity: r.DaysToMaturity, Notes: r.Notes,
}
}
// plantUpdateRequest is the body for PATCH /plants/:id: every field optional
// (absent = unchanged), plus the required current version. daysToMaturity is
// json.RawMessage so an explicit null (clear it) is distinct from an absent
// field (unchanged).
type plantUpdateRequest struct {
Name *string `json:"name"`
Category *string `json:"category"`
SpacingCM *float64 `json:"spacingCm"`
Color *string `json:"color"`
Icon *string `json:"icon"`
DaysToMaturity json.RawMessage `json:"daysToMaturity"`
Notes *string `json:"notes"`
Version int64 `json:"version" binding:"required,min=1"`
}
func (r plantUpdateRequest) toPatch() (service.PlantPatch, error) {
days, setDays, err := parseNullableInt(r.DaysToMaturity)
if err != nil {
return service.PlantPatch{}, err
}
return service.PlantPatch{
Name: r.Name, Category: r.Category, SpacingCM: r.SpacingCM,
Color: r.Color, Icon: r.Icon,
SetDays: setDays, DaysToMaturity: days, Notes: r.Notes,
}, nil
}
// parseNullableInt decodes a JSON number|null field into (value, present).
// Absent → (nil, false); null → (nil, true); 42 → (&42, true). A non-integer
// value is an error.
func parseNullableInt(raw json.RawMessage) (value *int, present bool, err error) {
Review

🟡 parseNullableInt duplicates parseNullableString (objects.go) almost verbatim; should be a shared generic helper

maintainability · flagged by 1 model

  • internal/api/plants.go:63-69 (parseNullableInt) is a byte-for-byte structural duplicate of internal/api/objects.go:87-95 (parseNullableString) — same "absent/null/value" decode logic, differing only by pointer type (*int vs *string). There's now a third near-twin, parseNullableJSON (internal/api/objects.go:100-109), which returns the raw JSON text instead of unmarshaling. Verified by reading both files directly: the module targets Go 1.26 (go.mod), so this is exactly the kind…

🪰 Gadfly · advisory

🟡 **parseNullableInt duplicates parseNullableString (objects.go) almost verbatim; should be a shared generic helper** _maintainability · flagged by 1 model_ - `internal/api/plants.go:63-69` (`parseNullableInt`) is a byte-for-byte structural duplicate of `internal/api/objects.go:87-95` (`parseNullableString`) — same "absent/null/value" decode logic, differing only by pointer type (`*int` vs `*string`). There's now a third near-twin, `parseNullableJSON` (`internal/api/objects.go:100-109`), which returns the raw JSON text instead of unmarshaling. Verified by reading both files directly: the module targets Go 1.26 (`go.mod`), so this is exactly the kind… <sub>🪰 Gadfly · advisory</sub>
if len(raw) == 0 {
return nil, false, nil
}
if err := json.Unmarshal(raw, &value); err != nil {
return nil, false, err
}
return value, true, nil
}
func (h *handlers) listPlants(c *gin.Context) {
plants, err := h.svc.ListPlants(c.Request.Context(), mustActor(c).ID)
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, plants)
}
func (h *handlers) createPlant(c *gin.Context) {
var req plantCreateRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "name, category, color and icon are required")
return
}
p, err := h.svc.CreatePlant(c.Request.Context(), mustActor(c).ID, req.toInput())
if err != nil {
writeServiceError(c, err)
return
}
c.JSON(http.StatusCreated, p)
}
func (h *handlers) updatePlant(c *gin.Context) {
id, ok := parseIDParam(c, "id")
if !ok {
return
}
var req plantUpdateRequest
if err := c.ShouldBindJSON(&req); err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "a current version is required")
return
}
patch, err := req.toPatch()
if err != nil {
writeAPIError(c, http.StatusBadRequest, "INVALID_INPUT", "invalid update payload")
return
}
p, err := h.svc.UpdatePlant(c.Request.Context(), mustActor(c).ID, id, patch, req.Version)
if err != nil {
if errors.Is(err, domain.ErrVersionConflict) {
writeVersionConflict(c, p)
return
}
writeServiceError(c, err)
return
}
c.JSON(http.StatusOK, p)
}
func (h *handlers) deletePlant(c *gin.Context) {
id, ok := parseIDParam(c, "id")
if !ok {
return
}
if err := h.svc.DeletePlant(c.Request.Context(), mustActor(c).ID, id); err != nil {
writeServiceError(c, err)
return
}
c.Status(http.StatusNoContent)
}
+100
View File
@@ -0,0 +1,100 @@
package api
import (
"encoding/json"
"net/http"
"strconv"
"testing"
)
func plantPath(id int64) string {
return "/api/v1/plants/" + strconv.FormatInt(id, 10)
}
// firstBuiltinID returns the id of a seeded built-in (no ownerId key) from a
// /plants list response, plus the total count.
func firstBuiltinID(t *testing.T, body []byte) (int64, int) {
t.Helper()
var list []map[string]any
if err := json.Unmarshal(body, &list); err != nil {
t.Fatalf("decode plants list: %v (body %s)", err, body)
}
for _, p := range list {
if _, owned := p["ownerId"]; !owned {
return int64(p["id"].(float64)), len(list)
}
}
t.Fatal("no built-in plant in list")
return 0, 0
}
func TestPlantCatalogFlow(t *testing.T) {
r := authEngine(t, localCfg())
cookie := registerAndCookie(t, r, "[email protected]")
// List → seeded built-ins present.
w := doJSON(t, r, http.MethodGet, "/api/v1/plants", nil, cookie)
if w.Code != http.StatusOK {
t.Fatalf("list status = %d, body %s", w.Code, w.Body.String())
}
builtinID, count := firstBuiltinID(t, w.Body.Bytes())
if count < 30 {
t.Errorf("catalog = %d plants, want >= 30 seeded", count)
}
// Create a custom plant → 201.
w = doJSON(t, r, http.MethodPost, "/api/v1/plants",
map[string]any{"name": "My tomato", "category": "vegetable", "spacingCm": 55, "color": "#e53935", "icon": "🍅"}, cookie)
if w.Code != http.StatusCreated {
t.Fatalf("create status = %d, body %s", w.Code, w.Body.String())
}
created := decodeMap(t, w.Body.Bytes())
id := int64(created["id"].(float64))
if created["ownerId"] == nil {
t.Error("custom plant should carry ownerId")
}
// Patch the custom plant with its version → 200, bumped.
w = doJSON(t, r, http.MethodPatch, plantPath(id), map[string]any{"spacingCm": 50, "version": 1}, cookie)
if w.Code != http.StatusOK {
t.Fatalf("patch status = %d, body %s", w.Code, w.Body.String())
}
if decodeMap(t, w.Body.Bytes())["version"].(float64) != 2 {
t.Error("patch didn't bump version")
}
// A built-in is read-only: patch and delete both → 403.
if w := doJSON(t, r, http.MethodPatch, plantPath(builtinID), map[string]any{"spacingCm": 99, "version": 1}, cookie); w.Code != http.StatusForbidden {
t.Errorf("patch built-in = %d, want 403", w.Code)
}
if w := doJSON(t, r, http.MethodDelete, plantPath(builtinID), nil, cookie); w.Code != http.StatusForbidden {
t.Errorf("delete built-in = %d, want 403", w.Code)
}
// Delete the custom plant → 204.
if w := doJSON(t, r, http.MethodDelete, plantPath(id), nil, cookie); w.Code != http.StatusNoContent {
t.Errorf("delete custom = %d, want 204", w.Code)
}
}
func TestPlantVisibilityAcrossUsersAPI(t *testing.T) {
r := authEngine(t, localCfg())
alice := registerAndCookie(t, r, "[email protected]")
bob := registerAndCookie(t, r, "[email protected]")
w := doJSON(t, r, http.MethodPost, "/api/v1/plants",
map[string]any{"name": "Alice mint", "category": "herb", "spacingCm": 30, "color": "#66bb6a", "icon": "🌿"}, alice)
id := int64(decodeMap(t, w.Body.Bytes())["id"].(float64))
// Bob can't see it → editing masks as 404, not 403.
if w := doJSON(t, r, http.MethodPatch, plantPath(id), map[string]any{"name": "x", "version": 1}, bob); w.Code != http.StatusNotFound {
t.Errorf("bob patch alice's plant = %d, want 404", w.Code)
}
}
func TestPlantsRequireAuth(t *testing.T) {
r := authEngine(t, localCfg())
if w := doJSON(t, r, http.MethodGet, "/api/v1/plants", nil, nil); w.Code != http.StatusUnauthorized {
t.Errorf("unauthenticated list = %d, want 401", w.Code)
}
}
+3
View File
@@ -17,6 +17,9 @@ var (
// ErrVersionConflict means the row's version did not match the one supplied // ErrVersionConflict means the row's version did not match the one supplied
// with a PATCH/DELETE; the caller should refetch and retry. // with a PATCH/DELETE; the caller should refetch and retry.
ErrVersionConflict = errors.New("version conflict") ErrVersionConflict = errors.New("version conflict")
// ErrPlantInUse means a plant can't be deleted because plantings still
// reference it (the FK is ON DELETE RESTRICT). Mapped to 409.
ErrPlantInUse = errors.New("plant is referenced by plantings")
// ErrInvalidInput means the caller supplied structurally invalid data (empty // ErrInvalidInput means the caller supplied structurally invalid data (empty
// required field, malformed value). Mapped to 400. // required field, malformed value). Mapped to 400.
+188
View File
@@ -0,0 +1,188 @@
package service
import (
"context"
"strings"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
// Plant field bounds. Spacing is mature in-row spacing in cm; the ceiling is a
// generous sanity limit (also rejecting NaN/Inf). Name/notes are length-capped so
// untrusted input can't balloon storage; icon holds one emoji (possibly a
// multi-codepoint ZWJ sequence, hence a byte cap rather than length 1).
const (
maxPlantNameLen = 200
maxPlantNotesLen = 10_000
maxPlantIconLen = 32
minPlantSpacingCM = 0.1
maxPlantSpacingCM = 10_000 // 100 m — headroom for large trees/shrubs
maxDaysToMaturity = 3650 // ~10 years
)
// plantCategories is the set of valid category enum values (mirrors the schema
// CHECK constraint in 0001_init.sql).
var plantCategories = map[string]struct{}{
domain.CategoryVegetable: {},
domain.CategoryHerb: {},
domain.CategoryFlower: {},
domain.CategoryFruit: {},
domain.CategoryTreeShrub: {},
domain.CategoryCover: {},
}
// PlantInput is the mutable field set for creating a plant. DaysToMaturity is
// optional (nil = unknown).
type PlantInput struct {
Name string
Category string
SpacingCM float64
Color string
Icon string
DaysToMaturity *int
Notes string
}
// PlantPatch is a partial update: nil fields are left unchanged. DaysToMaturity
// is itself nullable, so SetDays distinguishes "clear to NULL" (SetDays=true,
// value nil) from "leave unchanged" (SetDays=false) — a plain nil can't.
type PlantPatch struct {
Name *string
Category *string
SpacingCM *float64
Color *string
Icon *string
SetDays bool
DaysToMaturity *int
Notes *string
}
// ListPlants returns the plants the actor can see: built-ins plus their own.
func (s *Service) ListPlants(ctx context.Context, actorID int64) ([]domain.Plant, error) {
return s.store.ListPlantsForActor(ctx, actorID)
}
// CreatePlant adds a plant owned by the actor. To "clone" a built-in the client
// simply POSTs a copy — there is no dedicated endpoint, and the copy is owned by
// (and editable by) the actor.
func (s *Service) CreatePlant(ctx context.Context, actorID int64, in PlantInput) (*domain.Plant, error) {
p := &domain.Plant{
Name: strings.TrimSpace(in.Name),
Category: in.Category,
SpacingCM: in.SpacingCM,
Color: in.Color,
Icon: strings.TrimSpace(in.Icon),
DaysToMaturity: in.DaysToMaturity,
Notes: strings.TrimSpace(in.Notes),
}
if err := finalizePlant(p); err != nil {
return nil, err
}
owner := actorID
p.OwnerID = &owner
return s.store.CreatePlant(ctx, p)
}
// writablePlant loads a plant and enforces that the actor may modify it. It is
// THE authorization point for plant mutation. Built-in rows (owner_id nil) are
// visible to everyone but read-only → ErrForbidden. A row owned by someone else
// is invisible → ErrNotFound (masking existence, as gardens/objects do). Only the
// actor's own rows are writable.
func (s *Service) writablePlant(ctx context.Context, actorID, plantID int64) (*domain.Plant, error) {
p, err := s.store.GetPlant(ctx, plantID)
if err != nil {
return nil, err // ErrNotFound
}
if p.OwnerID == nil {
return nil, domain.ErrForbidden // built-in: seeded and read-only
}
if *p.OwnerID != actorID {
return nil, domain.ErrNotFound // another user's plant: not visible
}
return p, nil
}
// UpdatePlant applies a partial, version-guarded patch to the actor's own plant.
// On a version mismatch it returns (current, ErrVersionConflict).
func (s *Service) UpdatePlant(ctx context.Context, actorID, plantID int64, patch PlantPatch, version int64) (*domain.Plant, error) {
p, err := s.writablePlant(ctx, actorID, plantID)
if err != nil {
return nil, err
}
applyPlantPatch(p, patch)
if err := finalizePlant(p); err != nil {
return nil, err
}
p.Version = version
return s.store.UpdatePlant(ctx, p)
}
// DeletePlant removes the actor's own plant. A plant still referenced by any
// planting (even a removed one — the FK is RESTRICT) is refused with
// ErrPlantInUse rather than orphaning history; the client clones-and-edits or
// clears the plantings first.
func (s *Service) DeletePlant(ctx context.Context, actorID, plantID int64) error {
if _, err := s.writablePlant(ctx, actorID, plantID); err != nil {
return err
}
n, err := s.store.CountPlantingsForPlant(ctx, plantID)
if err != nil {
return err
}
if n > 0 {
return domain.ErrPlantInUse
}
return s.store.DeletePlant(ctx, plantID)
}
// applyPlantPatch mutates p with each provided (non-nil) patch field.
func applyPlantPatch(p *domain.Plant, patch PlantPatch) {
if patch.Name != nil {
p.Name = strings.TrimSpace(*patch.Name)
}
if patch.Category != nil {
p.Category = *patch.Category
}
if patch.SpacingCM != nil {
p.SpacingCM = *patch.SpacingCM
}
if patch.Color != nil {
p.Color = *patch.Color
}
if patch.Icon != nil {
p.Icon = strings.TrimSpace(*patch.Icon)
}
if patch.SetDays {
p.DaysToMaturity = patch.DaysToMaturity
}
if patch.Notes != nil {
p.Notes = strings.TrimSpace(*patch.Notes)
}
}
// finalizePlant validates a built/merged plant. Shared by create and update so
// both enforce the same invariants. isFinite/isHexColor live in objects.go.
func finalizePlant(p *domain.Plant) error {
if p.Name == "" || len(p.Name) > maxPlantNameLen {
return domain.ErrInvalidInput
}
if _, ok := plantCategories[p.Category]; !ok {
return domain.ErrInvalidInput
}
if !isFinite(p.SpacingCM) || p.SpacingCM < minPlantSpacingCM || p.SpacingCM > maxPlantSpacingCM {
return domain.ErrInvalidInput
}
if !isHexColor(p.Color) {
return domain.ErrInvalidInput
}
if p.Icon == "" || len(p.Icon) > maxPlantIconLen {
return domain.ErrInvalidInput
}
if p.DaysToMaturity != nil && (*p.DaysToMaturity < 1 || *p.DaysToMaturity > maxDaysToMaturity) {
return domain.ErrInvalidInput
}
if len(p.Notes) > maxPlantNotesLen {
return domain.ErrInvalidInput
}
return nil
}
+239
View File
@@ -0,0 +1,239 @@
package service
import (
"context"
"errors"
"strings"
"testing"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
)
func intPtr(n int) *int { return &n }
Outdated
Review

🟡 Duplicate pointer helper intPtr should be consolidated with existing ptrBool/strPtr in same package

maintainability · flagged by 2 models

  • internal/service/plants_test.go:12intPtr duplicates the pointer-helper pattern already present in internal/service/objects_test.go (ptrBool, strPtr). Three tiny one-liners are now scattered across test files in the same package with inconsistent naming (ptrBool vs strPtr vs intPtr). This makes the helpers hard to discover and invites further duplication. Fix: consolidate them into a shared helpers_test.go in internal/service/ with a consistent scheme (e.g. `ptrBo…

🪰 Gadfly · advisory

🟡 **Duplicate pointer helper intPtr should be consolidated with existing ptrBool/strPtr in same package** _maintainability · flagged by 2 models_ - **`internal/service/plants_test.go:12`** — `intPtr` duplicates the pointer-helper pattern already present in `internal/service/objects_test.go` (`ptrBool`, `strPtr`). Three tiny one-liners are now scattered across test files in the same package with inconsistent naming (`ptrBool` vs `strPtr` vs `intPtr`). This makes the helpers hard to discover and invites further duplication. **Fix:** consolidate them into a shared `helpers_test.go` in `internal/service/` with a consistent scheme (e.g. `ptrBo… <sub>🪰 Gadfly · advisory</sub>
// validPlant is a well-formed PlantInput for tests to tweak.
func validPlant(name string) PlantInput {
return PlantInput{Name: name, Category: domain.CategoryHerb, SpacingCM: 25, Color: "#4a7c3f", Icon: "🌿"}
}
func TestListPlantsIncludesSeededBuiltins(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
plants, err := s.ListPlants(context.Background(), owner)
if err != nil {
t.Fatalf("ListPlants: %v", err)
}
// The 0003 seed ships a catalog; assert it's there and all built-ins.
if len(plants) < 30 {
t.Fatalf("seeded catalog = %d plants, want >= 30", len(plants))
}
for _, p := range plants {
if p.OwnerID != nil {
t.Fatalf("unexpected non-builtin in a fresh catalog: %+v", p)
}
}
// Find a known one.
var found bool
for _, p := range plants {
if p.Name == "Garlic" {
found = true
if p.SpacingCM != 15 || p.Icon != "🧄" {
t.Errorf("Garlic seeded wrong: %+v", p)
}
}
}
if !found {
t.Error("expected seeded Garlic in the catalog")
}
}
func TestCreatePlantIsOwnedAndTrimmed(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
in := validPlant(" Purple basil ")
in.DaysToMaturity = intPtr(60)
p, err := s.CreatePlant(context.Background(), owner, in)
if err != nil {
t.Fatalf("CreatePlant: %v", err)
}
if p.OwnerID == nil || *p.OwnerID != owner {
t.Errorf("owner = %v, want %d", p.OwnerID, owner)
}
if p.Name != "Purple basil" {
t.Errorf("name = %q, want trimmed", p.Name)
}
if p.Version != 1 || p.DaysToMaturity == nil || *p.DaysToMaturity != 60 {
t.Errorf("unexpected plant: %+v", p)
}
}
func TestPlantVisibilityAcrossUsers(t *testing.T) {
s := newTestService(t, openConfig())
alice := seedUser(t, s, "[email protected]")
bob := seedUser(t, s, "[email protected]")
p, err := s.CreatePlant(context.Background(), alice, validPlant("Alice's mint"))
if err != nil {
t.Fatalf("CreatePlant: %v", err)
}
// Bob's catalog: built-ins only, none of Alice's.
bobList, err := s.ListPlants(context.Background(), bob)
if err != nil {
t.Fatalf("ListPlants(bob): %v", err)
}
for _, bp := range bobList {
if bp.ID == p.ID {
t.Fatal("bob can see alice's custom plant")
}
}
// Bob can neither edit nor delete it — invisibility masks it as ErrNotFound.
nm := "hijack"
if _, err := s.UpdatePlant(context.Background(), bob, p.ID, PlantPatch{Name: &nm}, p.Version); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("bob update err = %v, want ErrNotFound", err)
}
if err := s.DeletePlant(context.Background(), bob, p.ID); !errors.Is(err, domain.ErrNotFound) {
t.Errorf("bob delete err = %v, want ErrNotFound", err)
}
}
func TestBuiltinPlantsAreReadOnly(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
plants, _ := s.ListPlants(context.Background(), owner)
builtin := plants[0] // seeded, owner_id nil
if builtin.OwnerID != nil {
t.Fatalf("expected a built-in, got %+v", builtin)
}
nm := "tweaked"
if _, err := s.UpdatePlant(context.Background(), owner, builtin.ID, PlantPatch{Name: &nm}, builtin.Version); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("edit built-in err = %v, want ErrForbidden", err)
}
if err := s.DeletePlant(context.Background(), owner, builtin.ID); !errors.Is(err, domain.ErrForbidden) {
t.Errorf("delete built-in err = %v, want ErrForbidden", err)
}
}
func TestCloneBuiltinThenEdit(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
plants, _ := s.ListPlants(context.Background(), owner)
src := plants[0]
// "Clone" = POST a copy; it's now owned and editable.
clone, err := s.CreatePlant(context.Background(), owner, PlantInput{
Name: src.Name + " (mine)", Category: src.Category, SpacingCM: src.SpacingCM, Color: src.Color, Icon: src.Icon,
})
if err != nil {
t.Fatalf("clone: %v", err)
}
sp := 12.0
updated, err := s.UpdatePlant(context.Background(), owner, clone.ID, PlantPatch{SpacingCM: &sp}, clone.Version)
if err != nil {
t.Fatalf("edit clone: %v", err)
}
if updated.SpacingCM != 12 || updated.Version != clone.Version+1 {
t.Errorf("clone edit didn't apply: %+v", updated)
}
}
func TestUpdatePlantClearsDays(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
in := validPlant("Dill")
in.DaysToMaturity = intPtr(50)
p, _ := s.CreatePlant(context.Background(), owner, in)
// SetDays with nil clears; absent (SetDays=false) would leave it.
cleared, err := s.UpdatePlant(context.Background(), owner, p.ID, PlantPatch{SetDays: true, DaysToMaturity: nil}, p.Version)
if err != nil {
t.Fatalf("clear days: %v", err)
}
if cleared.DaysToMaturity != nil {
t.Errorf("daysToMaturity = %v, want nil", *cleared.DaysToMaturity)
}
}
func TestPlantVersionConflict(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
p, _ := s.CreatePlant(context.Background(), owner, validPlant("Sage"))
sp := 30.0
if _, err := s.UpdatePlant(context.Background(), owner, p.ID, PlantPatch{SpacingCM: &sp}, p.Version); err != nil {
t.Fatalf("first update: %v", err)
}
// Stale version → conflict + current row.
current, err := s.UpdatePlant(context.Background(), owner, p.ID, PlantPatch{SpacingCM: &sp}, p.Version)
if !errors.Is(err, domain.ErrVersionConflict) {
t.Fatalf("stale update err = %v, want ErrVersionConflict", err)
}
if current == nil || current.Version != 2 {
t.Errorf("conflict didn't return current row: %+v", current)
}
}
func TestCreatePlantValidation(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
bad := []PlantInput{
{Name: "", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿"}, // empty name
{Name: "x", Category: "spaceship", SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿"}, // bad category
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 0, Color: "#4a7c3f", Icon: "🌿"}, // zero spacing
{Name: "x", Category: domain.CategoryHerb, SpacingCM: -5, Color: "#4a7c3f", Icon: "🌿"}, // negative spacing
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "nope", Icon: "🌿"}, // bad color
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: ""}, // empty icon
{Name: strings.Repeat("x", maxPlantNameLen+1), Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿"}, // name too long
{Name: "x", Category: domain.CategoryHerb, SpacingCM: 10, Color: "#4a7c3f", Icon: "🌿", DaysToMaturity: intPtr(0)}, // days must be >= 1
}
for i, in := range bad {
if _, err := s.CreatePlant(context.Background(), owner, in); !errors.Is(err, domain.ErrInvalidInput) {
t.Errorf("case %d: err = %v, want ErrInvalidInput", i, err)
}
}
// A shorthand hex (#rgb) and a set days value pass.
ok := validPlant("Thyme")
ok.Color = "#0a0"
ok.DaysToMaturity = intPtr(90)
if _, err := s.CreatePlant(context.Background(), owner, ok); err != nil {
t.Errorf("valid plant rejected: %v", err)
}
}
func TestDeletePlantInUseIsBlocked(t *testing.T) {
s := newTestService(t, openConfig())
owner := seedUser(t, s, "[email protected]")
g := seedGarden(t, s, owner)
bed, _ := s.CreateObject(context.Background(), owner, g.ID, ObjectInput{
Kind: domain.KindBed, XCM: 500, YCM: 500, WidthCM: 200, HeightCM: 200,
})
plant, _ := s.CreatePlant(context.Background(), owner, validPlant("Basil"))
// Plantings CRUD is #14; insert a plop directly so the plant is referenced.
if _, err := s.store.SQL().ExecContext(context.Background(),
`INSERT INTO plantings (object_id, plant_id, x_cm, y_cm, radius_cm) VALUES (?, ?, 0, 0, 10)`,
bed.ID, plant.ID,
); err != nil {
t.Fatalf("seed planting: %v", err)
}
if err := s.DeletePlant(context.Background(), owner, plant.ID); !errors.Is(err, domain.ErrPlantInUse) {
t.Fatalf("delete in-use plant err = %v, want ErrPlantInUse", err)
}
// Clearing the reference frees the delete.
if _, err := s.store.SQL().ExecContext(context.Background(), `DELETE FROM plantings WHERE plant_id = ?`, plant.ID); err != nil {
t.Fatalf("clear planting: %v", err)
}
if err := s.DeletePlant(context.Background(), owner, plant.ID); err != nil {
t.Errorf("delete freed plant: %v", err)
}
}
@@ -0,0 +1,46 @@
-- 0003_seed_plants.sql — built-in plant catalog.
--
-- owner_id NULL marks a read-only built-in (see 0001_init.sql § plants). These
-- seed once: the migration runner records this version in schema_migrations, so
-- re-running Migrate is a no-op and never duplicates them. Users clone a built-in
-- (POST a copy owned by themselves) to customize; the originals stay immutable.
--
-- spacing_cm is mature in-row spacing and drives derived plop counts (#14).
-- color is a display hex; icon is an emoji (v1 ships zero image assets).
INSERT INTO plants (owner_id, name, category, spacing_cm, color, icon) VALUES
-- Alliums & vegetables
(NULL, 'Garlic', 'vegetable', 15, '#d9d2c5', '🧄'),
(NULL, 'Onion', 'vegetable', 10, '#b07fc7', '🧅'),
(NULL, 'Bush bean', 'vegetable', 10, '#6b8e23', '🫘'),
(NULL, 'Pole bean', 'vegetable', 15, '#4e7a27', '🫘'),
(NULL, 'Pea', 'vegetable', 8, '#8bc34a', '🫛'),
(NULL, 'Tomato', 'vegetable', 60, '#e53935', '🍅'),
(NULL, 'Pepper', 'vegetable', 45, '#d32f2f', '🌶️'),
(NULL, 'Cucumber', 'vegetable', 30, '#43a047', '🥒'),
(NULL, 'Zucchini', 'vegetable', 60, '#2e7d32', '🥒'),
(NULL, 'Winter squash', 'vegetable', 90, '#ef6c00', '🎃'),
(NULL, 'Carrot', 'vegetable', 8, '#f57c00', '🥕'),
(NULL, 'Radish', 'vegetable', 5, '#e91e63', '🌱'),
(NULL, 'Beet', 'vegetable', 10, '#8e24aa', '🌱'),
(NULL, 'Lettuce', 'vegetable', 20, '#7cb342', '🥬'),
(NULL, 'Spinach', 'vegetable', 10, '#33691e', '🥬'),
(NULL, 'Kale', 'vegetable', 45, '#2e5d34', '🥬'),
(NULL, 'Broccoli', 'vegetable', 45, '#3f7d3f', '🥦'),
(NULL, 'Cabbage', 'vegetable', 45, '#8bc98b', '🥬'),
(NULL, 'Potato', 'vegetable', 30, '#a1887f', '🥔'),
(NULL, 'Corn', 'vegetable', 25, '#fbc02d', '🌽'),
-- Herbs
(NULL, 'Basil', 'herb', 25, '#4a7c3f', '🌿'),
(NULL, 'Cilantro', 'herb', 10, '#6aa84f', '🌿'),
(NULL, 'Dill', 'herb', 15, '#7cb342', '🌿'),
(NULL, 'Parsley', 'herb', 15, '#388e3c', '🌿'),
(NULL, 'Oregano', 'herb', 25, '#558b2f', '🌿'),
(NULL, 'Thyme', 'herb', 20, '#7a9b6e', '🌿'),
(NULL, 'Rosemary', 'herb', 45, '#4b6b47', '🌿'),
(NULL, 'Sage', 'herb', 45, '#9caf88', '🌿'),
(NULL, 'Mint', 'herb', 30, '#66bb6a', '🌿'),
-- Fruit & flowers
(NULL, 'Strawberry', 'fruit', 30, '#e91e63', '🍓'),
(NULL, 'Sunflower', 'flower', 45, '#fdd835', '🌻'),
(NULL, 'Marigold', 'flower', 25, '#fb8c00', '🌼');
+131
View File
@@ -2,6 +2,8 @@ package store
import ( import (
"context" "context"
"database/sql"
"errors"
"fmt" "fmt"
"gitea.stevedudenhoeffer.com/steve/pansy/internal/domain" "gitea.stevedudenhoeffer.com/steve/pansy/internal/domain"
@@ -53,3 +55,132 @@ func (d *DB) ListReferencedPlants(ctx context.Context, gardenID int64) ([]domain
} }
return plants, nil return plants, nil
} }
// maxPlantsListed caps ListPlantsForActor as a defensive backstop. The seeded
// built-ins (~30) plus a user's own catalog stay tiny at household scale, so this
// is never hit; pagination is post-v1 if ever needed.
const maxPlantsListed = 5000
Review

ListPlantsForActor LIMIT 5000 silently truncates with no error if exceeded

error-handling · flagged by 1 model

  • internal/store/plants.go:170 (DeletePlant) — the isForeignKeyViolation(err) backstop relies on substring-matching the SQLite driver's error text ("FOREIGN KEY constraint failed", per internal/store/users.go:184-185). If the driver ever changes that wording, the concurrent-insert race between CountPlantingsForPlant and the DELETE would surface as a raw 500 INTERNAL instead of the intended 409 PLANT_IN_USE. This mirrors the pre-existing isUniqueViolation pattern, so it's reus…

🪰 Gadfly · advisory

⚪ **ListPlantsForActor LIMIT 5000 silently truncates with no error if exceeded** _error-handling · flagged by 1 model_ - `internal/store/plants.go:170` (`DeletePlant`) — the `isForeignKeyViolation(err)` backstop relies on substring-matching the SQLite driver's error text (`"FOREIGN KEY constraint failed"`, per `internal/store/users.go:184-185`). If the driver ever changes that wording, the concurrent-insert race between `CountPlantingsForPlant` and the `DELETE` would surface as a raw `500 INTERNAL` instead of the intended `409 PLANT_IN_USE`. This mirrors the pre-existing `isUniqueViolation` pattern, so it's reus… <sub>🪰 Gadfly · advisory</sub>
// ListPlantsForActor returns the plants a user can see: every built-in
// (owner_id NULL) plus the user's own rows, built-ins first then by name. Always
// a non-nil slice. This is the catalog list; ListReferencedPlants above is the
// narrower /full read.
func (d *DB) ListPlantsForActor(ctx context.Context, actorID int64) ([]domain.Plant, error) {
rows, err := d.sql.QueryContext(ctx,
`SELECT `+plantColumns+` FROM plants
WHERE owner_id IS NULL OR owner_id = ?
ORDER BY owner_id IS NOT NULL, name COLLATE NOCASE, id
LIMIT ?`,
actorID, maxPlantsListed,
)
if err != nil {
return nil, fmt.Errorf("store: list plants: %w", err)
}
defer rows.Close()
plants := []domain.Plant{}
for rows.Next() {
p, err := scanPlant(rows)
if err != nil {
return nil, fmt.Errorf("store: scan plant: %w", err)
}
plants = append(plants, *p)
}
if err := rows.Err(); err != nil {
return nil, fmt.Errorf("store: iterate plants: %w", err)
}
return plants, nil
}
// GetPlant returns the plant with the given id, or domain.ErrNotFound. Visibility
// (built-in vs owned) is the service layer's call, not this raw read's.
func (d *DB) GetPlant(ctx context.Context, id int64) (*domain.Plant, error) {
p, err := scanPlant(d.sql.QueryRowContext(ctx,
`SELECT `+plantColumns+` FROM plants WHERE id = ?`, id))
if errors.Is(err, sql.ErrNoRows) {
return nil, domain.ErrNotFound
}
if err != nil {
return nil, fmt.Errorf("store: get plant: %w", err)
}
return p, nil
}
// CreatePlant inserts a plant (fields already validated by the service) and
// returns the stored row. OwnerID is the creating user — built-ins are seeded by
// migration only, never through this path.
func (d *DB) CreatePlant(ctx context.Context, p *domain.Plant) (*domain.Plant, error) {
created, err := scanPlant(d.sql.QueryRowContext(ctx,
`INSERT INTO plants (owner_id, name, category, spacing_cm, color, icon, days_to_maturity, notes)
VALUES (?, ?, ?, ?, ?, ?, ?, ?)
RETURNING `+plantColumns,
p.OwnerID, p.Name, p.Category, p.SpacingCM, p.Color, p.Icon, p.DaysToMaturity, p.Notes,
))
if err != nil {
return nil, fmt.Errorf("store: insert plant: %w", err)
}
return created, nil
}
// UpdatePlant applies a version-guarded update of all mutable columns (the
// service merges partial patches first). Returns the updated row, or
// (current row, ErrVersionConflict) / ErrNotFound — the same contract as
// UpdateGarden/UpdateObject.
func (d *DB) UpdatePlant(ctx context.Context, p *domain.Plant) (*domain.Plant, error) {
updated, err := scanPlant(d.sql.QueryRowContext(ctx,
`UPDATE plants
SET name = ?, category = ?, spacing_cm = ?, color = ?, icon = ?,
days_to_maturity = ?, notes = ?,
version = version + 1,
updated_at = strftime('%Y-%m-%dT%H:%M:%SZ', 'now')
WHERE id = ? AND version = ?
RETURNING `+plantColumns,
p.Name, p.Category, p.SpacingCM, p.Color, p.Icon, p.DaysToMaturity, p.Notes,
p.ID, p.Version,
))
if errors.Is(err, sql.ErrNoRows) {
current, gerr := d.GetPlant(ctx, p.ID)
if gerr != nil {
return nil, gerr
}
return current, domain.ErrVersionConflict
}
if err != nil {
return nil, fmt.Errorf("store: update plant: %w", err)
}
return updated, nil
}
// CountPlantingsForPlant returns how many plantings reference a plant, INCLUDING
// removed ones — a removed_at row still holds the FK (ON DELETE RESTRICT), so any
// row blocks deletion. The service checks this to refuse a delete with a clear
// error before the constraint fires.
func (d *DB) CountPlantingsForPlant(ctx context.Context, plantID int64) (int, error) {
var n int
if err := d.sql.QueryRowContext(ctx,
`SELECT count(*) FROM plantings WHERE plant_id = ?`, plantID).Scan(&n); err != nil {
return 0, fmt.Errorf("store: count plantings for plant: %w", err)
}
return n, nil
}
// DeletePlant removes a plant. Returns domain.ErrNotFound if no row was deleted,
// or domain.ErrPlantInUse if the ON DELETE RESTRICT foreign key from plantings
// fires (a backstop for the service's own count check under a concurrent insert).
func (d *DB) DeletePlant(ctx context.Context, id int64) error {
Review

🟡 FK-violation backstop uses fragile string-match on driver error; race could surface as 500 instead of 409

error-handling · flagged by 1 model

  • internal/store/plants.go:170 (DeletePlant) — the isForeignKeyViolation(err) backstop relies on substring-matching the SQLite driver's error text ("FOREIGN KEY constraint failed", per internal/store/users.go:184-185). If the driver ever changes that wording, the concurrent-insert race between CountPlantingsForPlant and the DELETE would surface as a raw 500 INTERNAL instead of the intended 409 PLANT_IN_USE. This mirrors the pre-existing isUniqueViolation pattern, so it's reus…

🪰 Gadfly · advisory

🟡 **FK-violation backstop uses fragile string-match on driver error; race could surface as 500 instead of 409** _error-handling · flagged by 1 model_ - `internal/store/plants.go:170` (`DeletePlant`) — the `isForeignKeyViolation(err)` backstop relies on substring-matching the SQLite driver's error text (`"FOREIGN KEY constraint failed"`, per `internal/store/users.go:184-185`). If the driver ever changes that wording, the concurrent-insert race between `CountPlantingsForPlant` and the `DELETE` would surface as a raw `500 INTERNAL` instead of the intended `409 PLANT_IN_USE`. This mirrors the pre-existing `isUniqueViolation` pattern, so it's reus… <sub>🪰 Gadfly · advisory</sub>
res, err := d.sql.ExecContext(ctx, `DELETE FROM plants WHERE id = ?`, id)
if err != nil {
if isForeignKeyViolation(err) {
return domain.ErrPlantInUse
}
return fmt.Errorf("store: delete plant: %w", err)
}
n, err := res.RowsAffected()
if err != nil {
return fmt.Errorf("store: plant delete rows: %w", err)
}
if n == 0 {
return domain.ErrNotFound
}
return nil
}
+15 -4
View File
@@ -38,12 +38,19 @@ func TestMigrateCreatesSchema(t *testing.T) {
} }
} }
// Derive the expected head version from the embedded files so adding a
// migration doesn't break this test.
migs, err := loadMigrations()
if err != nil {
t.Fatalf("loadMigrations: %v", err)
}
wantVersion := migs[len(migs)-1].version
var version int var version int
if err := db.SQL().QueryRow(`SELECT max(version) FROM schema_migrations`).Scan(&version); err != nil { if err := db.SQL().QueryRow(`SELECT max(version) FROM schema_migrations`).Scan(&version); err != nil {
t.Fatalf("read schema_migrations: %v", err) t.Fatalf("read schema_migrations: %v", err)
} }
if version != 2 { if version != wantVersion {
t.Errorf("schema version = %d, want 2", version) t.Errorf("schema version = %d, want %d", version, wantVersion)
} }
} }
@@ -54,12 +61,16 @@ func TestMigrateIsIdempotent(t *testing.T) {
if err := db.Migrate(context.Background()); err != nil { if err := db.Migrate(context.Background()); err != nil {
t.Fatalf("second Migrate: %v", err) t.Fatalf("second Migrate: %v", err)
} }
migs, err := loadMigrations()
if err != nil {
t.Fatalf("loadMigrations: %v", err)
}
var count int var count int
if err := db.SQL().QueryRow(`SELECT count(*) FROM schema_migrations`).Scan(&count); err != nil { if err := db.SQL().QueryRow(`SELECT count(*) FROM schema_migrations`).Scan(&count); err != nil {
t.Fatalf("count schema_migrations: %v", err) t.Fatalf("count schema_migrations: %v", err)
} }
if count != 2 { if count != len(migs) {
t.Errorf("schema_migrations rows = %d, want 2", count) t.Errorf("schema_migrations rows = %d, want %d", count, len(migs))
} }
} }
+7
View File
@@ -177,3 +177,10 @@ func boolToInt(b bool) int {
func isUniqueViolation(err error) bool { func isUniqueViolation(err error) bool {
return err != nil && strings.Contains(err.Error(), "UNIQUE constraint failed") return err != nil && strings.Contains(err.Error(), "UNIQUE constraint failed")
} }
// isForeignKeyViolation reports whether err is a SQLite FOREIGN KEY-constraint
// failure — e.g. deleting a plant a planting still references (ON DELETE
// RESTRICT). Same stable-message approach as isUniqueViolation.
func isForeignKeyViolation(err error) bool {
return err != nil && strings.Contains(err.Error(), "FOREIGN KEY constraint failed")
}