diff --git a/internal/service/ops.go b/internal/service/ops.go index f1e18be..4e3cab7 100644 --- a/internal/service/ops.go +++ b/internal/service/ops.go @@ -92,10 +92,11 @@ func defaultPlopRadius(spacingCM float64) float64 { // FillRegion lays a hex-packed field of plops of one plant across a region of a // plantable object the actor can edit. Plop radius comes from the plant's spacing // (or spacingOverride) via defaultPlopRadius; centers sit on a hex lattice at 2× -// radius pitch, centered in the region with a half-pitch inset at each edge (see -// hexCenters for why that inset and not a full one). A candidate is skipped when -// its plop would sit entirely inside an existing active plop (so re-filling -// doesn't stack duplicates). Returns the plops it created. +// radius pitch, centered in the region, and set in from each edge by the plop's +// radius less half a spacing — see hexCenters for why that half-spacing is what +// the edge is owed. A candidate is skipped when its plop would sit entirely +// inside an existing active plop (so re-filling doesn't stack duplicates). +// Returns the plops it created. func (s *Service) FillRegion(ctx context.Context, actorID, objectID int64, region Region, plantID int64, spacingOverride *float64) ([]domain.Planting, error) { o, _, err := s.objectForRole(ctx, actorID, objectID, roleEditor) if err != nil { @@ -128,6 +129,18 @@ func (s *Service) fillLoaded(ctx context.Context, actorID int64, o *domain.Garde return nil, domain.ErrInvalidInput } + // A caller-supplied region is arbitrary floats, and non-finite ones survive + // everything downstream: clamping keeps them, the inverted-region guard can't + // see NaN (it compares false both ways), and fitAxis centres on them happily. + // Nothing corrupt reaches the table — SQLite stores NaN as NULL and the NOT + // NULL constraint refuses it — but the caller gets an opaque store error for + // NaN, and for +Inf a silent zero-plop success. Both are lies about what went + // wrong; say "bad input" here instead. + if !isFinite(region.MinX) || !isFinite(region.MinY) || + !isFinite(region.MaxX) || !isFinite(region.MaxY) { + return nil, domain.ErrInvalidInput + } + region = region.clampTo(o.WidthCM/2, o.HeightCM/2) centers := hexCenters(region, radius, spacing) if len(centers) > maxFillPlops { diff --git a/internal/service/ops_test.go b/internal/service/ops_test.go index 18e6939..af84de1 100644 --- a/internal/service/ops_test.go +++ b/internal/service/ops_test.go @@ -154,10 +154,55 @@ func TestHexCentersEdgeInset(t *testing.T) { // TestHexCentersTinyRegion covers a region too small to hold a plop at the // half-pitch inset: planting one in the middle beats refusing to plant at all. +// +// The off-centre case earns its place — a region symmetric about the origin +// can't tell "the middle of the region" from "the origin", so on its own it +// would pass for an implementation that just returned (0,0). func TestHexCentersTinyRegion(t *testing.T) { - pts := hexCenters(rect(-5, -5, 5, 5), 15, 10) - if len(pts) != 1 || pts[0].x != 0 || pts[0].y != 0 { - t.Errorf("tiny region gave %+v, want a single centered plop", pts) + for _, tc := range []struct { + name string + r Region + wantX, wantY float64 + }{ + {"centred on the origin", rect(-5, -5, 5, 5), 0, 0}, + {"off in a corner", rect(20, -40, 30, -30), 25, -35}, + } { + t.Run(tc.name, func(t *testing.T) { + pts := hexCenters(tc.r, 15, 10) + if len(pts) != 1 || pts[0].x != tc.wantX || pts[0].y != tc.wantY { + t.Errorf("got %+v, want one plop at (%v,%v)", pts, tc.wantX, tc.wantY) + } + }) + } +} + +// TestFillRegionRejectsNonFiniteRegion: non-finite bounds survive clamping and +// the inverted-region guard (NaN compares false both ways). Without the explicit +// check, NaN surfaced as a raw store error ("NOT NULL constraint failed") and +// +Inf as a silent success that planted nothing — neither of which tells the +// caller what it actually did wrong. +func TestFillRegionRejectsNonFiniteRegion(t *testing.T) { + ctx := context.Background() + s := newTestService(t, openConfig()) + owner := seedUser(t, s, "a@example.com") + g, _ := s.CreateGarden(ctx, owner, GardenInput{Name: "Big", WidthCM: 2000, HeightCM: 2000}) + bed := seedFillBed(t, s, owner, g.ID, 100, 100) + plant := seedOwnPlant(t, s, owner, 10) + + nan := math.NaN() + for _, r := range []Region{ + {MinX: nan, MinY: -50, MaxX: 50, MaxY: 50}, + {MinX: -50, MinY: -50, MaxX: 50, MaxY: math.Inf(1)}, + } { + created, err := s.FillRegion(ctx, owner, bed.ID, r, plant.ID, nil) + if !errors.Is(err, domain.ErrInvalidInput) { + t.Errorf("FillRegion(%+v) err = %v, want ErrInvalidInput", r, err) + } + for _, p := range created { + if !isFinite(p.XCM) || !isFinite(p.YCM) { + t.Errorf("persisted a plop with non-finite coordinates: %+v", p) + } + } } }