fix(landscape): cast run status parameter so SetRunStatus type-checks in Postgres
The status parameter was used both as the assigned value (varchar column) and in CASE comparisons against text literals, so Postgres deduced two types for $2 and every run failed at SetRunStatus(running) with SQLSTATE 42P08; the failure also prevented marking the run failed, leaving it queued. Cast every use to text. Add TEST_DB_URL-gated repository tests (skipped without a database) that exercise the landscape run/node SQL and the new dataset period/group queries against a real Postgres, and document them in the README. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This commit is contained in:
11
README.md
11
README.md
@@ -364,6 +364,17 @@ make lint # golangci-lint (if installed)
|
|||||||
CI (`.github/workflows/ci.yml`) runs build, vet, `go test -race`, and golangci-lint
|
CI (`.github/workflows/ci.yml`) runs build, vet, `go test -race`, and golangci-lint
|
||||||
on every push and pull request.
|
on every push and pull request.
|
||||||
|
|
||||||
|
Repository tests in `internal/repository/postgres` run against a real database
|
||||||
|
when `TEST_DB_URL` points at one with migrations applied, and are skipped
|
||||||
|
otherwise. They catch Postgres-only failures (parameter type deduction, casts,
|
||||||
|
constraints) that in-memory stubs cannot:
|
||||||
|
|
||||||
|
```sh
|
||||||
|
docker run -d --name gis-test-pg -e POSTGRES_USER=gis -e POSTGRES_PASSWORD=gis -e POSTGRES_DB=gis -p 5434:5432 postgis/postgis:17-3.5
|
||||||
|
DB_URL='postgres://gis:gis@127.0.0.1:5434/gis?sslmode=disable' S3_ENDPOINT=x S3_ACCESS_KEY=x S3_SECRET_KEY=x RABBITMQ_URL=amqp://x go run ./cmd/gis migrate up
|
||||||
|
TEST_DB_URL='postgres://gis:gis@127.0.0.1:5434/gis?sslmode=disable' go test ./internal/repository/postgres/ -v
|
||||||
|
```
|
||||||
|
|
||||||
## Adding a feature
|
## Adding a feature
|
||||||
|
|
||||||
Each new domain is one vertical slice mirroring Category/Dataset:
|
Each new domain is one vertical slice mirroring Category/Dataset:
|
||||||
|
|||||||
115
internal/repository/postgres/dataset_db_test.go
Normal file
115
internal/repository/postgres/dataset_db_test.go
Normal file
@@ -0,0 +1,115 @@
|
|||||||
|
package postgres
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"gis/internal/domain"
|
||||||
|
|
||||||
|
"github.com/google/uuid"
|
||||||
|
)
|
||||||
|
|
||||||
|
// TestDatasetRepository_GroupsPeriodsAndLatest exercises the Postgres-only
|
||||||
|
// parts of the dataset/group repositories added with dataset periods: the
|
||||||
|
// ::date casts on *string parameters, the date filter clause, and the
|
||||||
|
// "freshest in group" ordering (end_date DESC NULLS LAST, created_at DESC).
|
||||||
|
func TestDatasetRepository_GroupsPeriodsAndLatest(t *testing.T) {
|
||||||
|
pool := testPool(t)
|
||||||
|
ctx := context.Background()
|
||||||
|
groups := NewDatasetGroupRepository(pool)
|
||||||
|
datasets := NewDatasetRepository(pool)
|
||||||
|
categories := NewCategoryRepository(pool)
|
||||||
|
|
||||||
|
suffix := uuid.NewString()[:8]
|
||||||
|
cat, err := categories.Create(ctx, domain.Category{Code: "dbtest-" + suffix, Name: "db test"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create category: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM categories WHERE id = $1`, cat.ID) })
|
||||||
|
|
||||||
|
group, err := groups.Create(ctx, domain.DatasetGroup{Code: "dbtest-group-" + suffix, Name: "db test group"})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create group: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM dataset_groups WHERE id = $1`, group.ID) })
|
||||||
|
|
||||||
|
s := func(v string) *string { return &v }
|
||||||
|
mk := func(name string, start, end *string) domain.Dataset {
|
||||||
|
d, err := datasets.Create(ctx, domain.Dataset{
|
||||||
|
CategoryID: cat.ID, DatasetGroupID: &group.ID, Name: name, Filename: name + ".tif",
|
||||||
|
StorageKey: "dbtest/" + suffix + "/" + name, FileType: domain.FileTypeRaster,
|
||||||
|
ContentType: "image/tiff", Status: domain.DatasetStatusReady,
|
||||||
|
StartDate: start, EndDate: end,
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("create dataset %s: %v", name, err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM datasets WHERE id = $1`, d.ID) })
|
||||||
|
time.Sleep(5 * time.Millisecond) // distinct created_at for the tie-break
|
||||||
|
return d
|
||||||
|
}
|
||||||
|
|
||||||
|
old := mk("old", s("2019-01-01"), s("2019-12-31"))
|
||||||
|
fresh := mk("fresh", s("2020-01-01"), s("2020-12-31"))
|
||||||
|
undated := mk("undated", nil, nil)
|
||||||
|
|
||||||
|
if old.StartDate == nil || *old.StartDate != "2019-01-01" || *old.EndDate != "2019-12-31" || old.DatasetGroupID == nil {
|
||||||
|
t.Fatalf("dates/group not round-tripped: %+v", old)
|
||||||
|
}
|
||||||
|
|
||||||
|
latest, err := datasets.LatestInGroup(ctx, group.ID, nil, nil)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("LatestInGroup: %v", err)
|
||||||
|
}
|
||||||
|
if latest.ID != fresh.ID {
|
||||||
|
t.Fatalf("latest = %s, want fresh %s (undated %s must sort last)", latest.Name, fresh.ID, undated.ID)
|
||||||
|
}
|
||||||
|
ft := domain.FileTypeVector
|
||||||
|
if _, err := datasets.LatestInGroup(ctx, group.ID, nil, &ft); !errors.Is(err, domain.ErrNotFound) {
|
||||||
|
t.Fatalf("no vector in group: err = %v, want ErrNotFound", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
page, err := datasets.ListSummaries(ctx, domain.DatasetFilter{
|
||||||
|
DatasetGroupID: &group.ID, EndDateFrom: s("2020-01-01"), EndDateTo: s("2020-12-31"),
|
||||||
|
}, 10, 0)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ListSummaries with date bounds: %v", err)
|
||||||
|
}
|
||||||
|
if len(page) != 1 || page[0].ID != fresh.ID {
|
||||||
|
t.Fatalf("date filter returned %+v, want only fresh", page)
|
||||||
|
}
|
||||||
|
n, err := datasets.Count(ctx, domain.DatasetFilter{DatasetGroupID: &group.ID, StartDateFrom: s("2019-06-01")})
|
||||||
|
if err != nil || n != 1 {
|
||||||
|
t.Fatalf("Count(start_date_from) = %d, %v; want 1", n, err)
|
||||||
|
}
|
||||||
|
|
||||||
|
updated, err := datasets.UpdateFields(ctx, undated.ID, domain.DatasetFields{
|
||||||
|
CategoryID: cat.ID, Name: "undated-now-dated", DatasetGroupID: &group.ID,
|
||||||
|
StartDate: s("2021-01-01"), EndDate: s("2021-12-31"),
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("UpdateFields: %v", err)
|
||||||
|
}
|
||||||
|
if updated.EndDate == nil || *updated.EndDate != "2021-12-31" {
|
||||||
|
t.Fatalf("UpdateFields did not store the period: %+v", updated)
|
||||||
|
}
|
||||||
|
if _, err := datasets.UpdateFields(ctx, undated.ID, domain.DatasetFields{
|
||||||
|
CategoryID: cat.ID, Name: "bad", StartDate: s("2021-12-31"), EndDate: s("2021-01-01"),
|
||||||
|
}); !errors.Is(err, domain.ErrValidation) {
|
||||||
|
t.Fatalf("reversed period must hit the CHECK constraint: err = %v", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := groups.Delete(ctx, group.ID); !errors.Is(err, domain.ErrConflict) {
|
||||||
|
t.Fatalf("deleting a referenced group: err = %v, want ErrConflict", err)
|
||||||
|
}
|
||||||
|
g2, err := groups.SetSLD(ctx, group.ID, "k", "style.sld", "application/xml")
|
||||||
|
if err != nil || g2.SldStorageKey == nil || *g2.SldStorageKey != "k" {
|
||||||
|
t.Fatalf("SetSLD: %+v, %v", g2, err)
|
||||||
|
}
|
||||||
|
byCode, err := groups.GetByCode(ctx, group.Code)
|
||||||
|
if err != nil || byCode.ID != group.ID {
|
||||||
|
t.Fatalf("GetByCode: %+v, %v", byCode, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
@@ -165,9 +165,12 @@ func (r *LandscapeRepository) CountRuns(ctx context.Context) (int, error) {
|
|||||||
// Returns domain.ErrNotFound if the run does not exist.
|
// Returns domain.ErrNotFound if the run does not exist.
|
||||||
func (r *LandscapeRepository) SetRunStatus(ctx context.Context, id uuid.UUID, status domain.LPRunStatus, errMsg *string) error {
|
func (r *LandscapeRepository) SetRunStatus(ctx context.Context, id uuid.UUID, status domain.LPRunStatus, errMsg *string) error {
|
||||||
tag, err := r.pool.Exec(ctx,
|
tag, err := r.pool.Exec(ctx,
|
||||||
`UPDATE lp_runs SET status = $2, error = $3,
|
// $2 is used both as the assigned value (varchar column) and in the CASE
|
||||||
started_at = CASE WHEN $2 = 'running' THEN now() ELSE started_at END,
|
// comparisons (text literals); the explicit ::text casts keep Postgres
|
||||||
finished_at = CASE WHEN $2 IN ('succeeded','failed') THEN now() ELSE finished_at END
|
// from deducing two different types for one parameter (SQLSTATE 42P08).
|
||||||
|
`UPDATE lp_runs SET status = $2::text, error = $3,
|
||||||
|
started_at = CASE WHEN $2::text = 'running' THEN now() ELSE started_at END,
|
||||||
|
finished_at = CASE WHEN $2::text IN ('succeeded','failed') THEN now() ELSE finished_at END
|
||||||
WHERE id = $1`,
|
WHERE id = $1`,
|
||||||
id, status, errMsg,
|
id, status, errMsg,
|
||||||
)
|
)
|
||||||
|
|||||||
151
internal/repository/postgres/landscape_db_test.go
Normal file
151
internal/repository/postgres/landscape_db_test.go
Normal file
@@ -0,0 +1,151 @@
|
|||||||
|
package postgres
|
||||||
|
|
||||||
|
import (
|
||||||
|
"context"
|
||||||
|
"errors"
|
||||||
|
"os"
|
||||||
|
"testing"
|
||||||
|
"time"
|
||||||
|
|
||||||
|
"gis/internal/domain"
|
||||||
|
|
||||||
|
"github.com/google/uuid"
|
||||||
|
"github.com/jackc/pgx/v5/pgxpool"
|
||||||
|
)
|
||||||
|
|
||||||
|
// testPool connects to the database named by TEST_DB_URL (migrations already
|
||||||
|
// applied) or skips the test. Repository SQL is otherwise only exercised by
|
||||||
|
// hand, so these tests are the guard against Postgres-only failures such as
|
||||||
|
// parameter type deduction (SQLSTATE 42P08) that no stub can reproduce.
|
||||||
|
func testPool(t *testing.T) *pgxpool.Pool {
|
||||||
|
t.Helper()
|
||||||
|
url := os.Getenv("TEST_DB_URL")
|
||||||
|
if url == "" {
|
||||||
|
t.Skip("TEST_DB_URL not set; skipping database test")
|
||||||
|
}
|
||||||
|
ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
|
||||||
|
defer cancel()
|
||||||
|
pool, err := Connect(ctx, url)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("connect: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(pool.Close)
|
||||||
|
return pool
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLandscapeRepository_SetRunStatus_AllTransitions(t *testing.T) {
|
||||||
|
pool := testPool(t)
|
||||||
|
repo := NewLandscapeRepository(pool)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
run, err := repo.CreateRun(ctx, domain.LPRun{
|
||||||
|
Status: domain.LPRunQueued,
|
||||||
|
Params: []byte(`{}`),
|
||||||
|
Nodes: []domain.LPRunNode{{NodeCode: "00", Stage: 0, Status: domain.LPNodePending}},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateRun: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM lp_runs WHERE id = $1`, run.ID) })
|
||||||
|
|
||||||
|
if err := repo.SetRunStatus(ctx, run.ID, domain.LPRunRunning, nil); err != nil {
|
||||||
|
t.Fatalf("SetRunStatus(running): %v", err)
|
||||||
|
}
|
||||||
|
got, err := repo.GetRun(ctx, run.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetRun: %v", err)
|
||||||
|
}
|
||||||
|
if got.Status != domain.LPRunRunning || got.StartedAt == nil || got.FinishedAt != nil {
|
||||||
|
t.Fatalf("after running: status=%s started=%v finished=%v", got.Status, got.StartedAt, got.FinishedAt)
|
||||||
|
}
|
||||||
|
|
||||||
|
msg := "boom"
|
||||||
|
if err := repo.SetRunStatus(ctx, run.ID, domain.LPRunFailed, &msg); err != nil {
|
||||||
|
t.Fatalf("SetRunStatus(failed): %v", err)
|
||||||
|
}
|
||||||
|
got, err = repo.GetRun(ctx, run.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetRun: %v", err)
|
||||||
|
}
|
||||||
|
if got.Status != domain.LPRunFailed || got.FinishedAt == nil || got.Error == nil || *got.Error != msg {
|
||||||
|
t.Fatalf("after failed: status=%s finished=%v error=%v", got.Status, got.FinishedAt, got.Error)
|
||||||
|
}
|
||||||
|
|
||||||
|
if err := repo.SetRunStatus(ctx, run.ID, domain.LPRunSucceeded, nil); err != nil {
|
||||||
|
t.Fatalf("SetRunStatus(succeeded): %v", err)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestLandscapeRepository_NodeLifecycleAndReuse(t *testing.T) {
|
||||||
|
pool := testPool(t)
|
||||||
|
repo := NewLandscapeRepository(pool)
|
||||||
|
ctx := context.Background()
|
||||||
|
|
||||||
|
newRun := func() domain.LPRun {
|
||||||
|
run, err := repo.CreateRun(ctx, domain.LPRun{
|
||||||
|
Status: domain.LPRunQueued,
|
||||||
|
Params: []byte(`{"window_size":2048}`),
|
||||||
|
Overrides: map[string]uuid.UUID{"lp-e": uuid.New()},
|
||||||
|
Nodes: []domain.LPRunNode{{NodeCode: "00", Stage: 0, Status: domain.LPNodePending}},
|
||||||
|
})
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("CreateRun: %v", err)
|
||||||
|
}
|
||||||
|
t.Cleanup(func() { _, _ = pool.Exec(ctx, `DELETE FROM lp_runs WHERE id = $1`, run.ID) })
|
||||||
|
return run
|
||||||
|
}
|
||||||
|
|
||||||
|
inputs := []domain.LPDatasetRef{{Group: "lp-z-wr", DatasetID: uuid.New()}}
|
||||||
|
outputs := []domain.LPDatasetRef{{Group: "lp-z-integrated", DatasetID: uuid.New()}}
|
||||||
|
|
||||||
|
first := newRun()
|
||||||
|
if err := repo.SetNodeStarted(ctx, first.ID, "00", "fp-1", inputs); err != nil {
|
||||||
|
t.Fatalf("SetNodeStarted: %v", err)
|
||||||
|
}
|
||||||
|
if err := repo.SetNodeFinished(ctx, first.ID, "00", domain.LPNodeSucceeded, outputs, nil); err != nil {
|
||||||
|
t.Fatalf("SetNodeFinished: %v", err)
|
||||||
|
}
|
||||||
|
got, err := repo.GetRun(ctx, first.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("GetRun: %v", err)
|
||||||
|
}
|
||||||
|
if len(got.Nodes) != 1 || got.Nodes[0].Status != domain.LPNodeSucceeded ||
|
||||||
|
len(got.Nodes[0].Inputs) != 1 || got.Nodes[0].Inputs[0] != inputs[0] ||
|
||||||
|
len(got.Nodes[0].Outputs) != 1 || got.Nodes[0].Outputs[0] != outputs[0] ||
|
||||||
|
got.Nodes[0].StartedAt == nil || got.Nodes[0].FinishedAt == nil {
|
||||||
|
t.Fatalf("node row not round-tripped: %+v", got.Nodes)
|
||||||
|
}
|
||||||
|
if len(got.Overrides) != 1 {
|
||||||
|
t.Fatalf("overrides not round-tripped: %+v", got.Overrides)
|
||||||
|
}
|
||||||
|
|
||||||
|
second := newRun()
|
||||||
|
reused, err := repo.FindReusableOutputs(ctx, "00", "fp-1", second.ID)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("FindReusableOutputs: %v", err)
|
||||||
|
}
|
||||||
|
if len(reused) != 1 || reused[0] != outputs[0] {
|
||||||
|
t.Fatalf("reused = %+v, want %+v", reused, outputs)
|
||||||
|
}
|
||||||
|
if _, err := repo.FindReusableOutputs(ctx, "00", "fp-other", second.ID); !errors.Is(err, domain.ErrNotFound) {
|
||||||
|
t.Fatalf("unknown fingerprint: err = %v, want ErrNotFound", err)
|
||||||
|
}
|
||||||
|
if _, err := repo.FindReusableOutputs(ctx, "00", "fp-1", first.ID); !errors.Is(err, domain.ErrNotFound) {
|
||||||
|
t.Fatalf("own run must be excluded: err = %v, want ErrNotFound", err)
|
||||||
|
}
|
||||||
|
|
||||||
|
failMsg := "grid mismatch"
|
||||||
|
if err := repo.SetNodeFinished(ctx, second.ID, "00", domain.LPNodeFailed, nil, &failMsg); err != nil {
|
||||||
|
t.Fatalf("SetNodeFinished(failed): %v", err)
|
||||||
|
}
|
||||||
|
runs, err := repo.ListRuns(ctx, 10, 0)
|
||||||
|
if err != nil {
|
||||||
|
t.Fatalf("ListRuns: %v", err)
|
||||||
|
}
|
||||||
|
if len(runs) < 2 || runs[0].ID != second.ID {
|
||||||
|
t.Fatalf("ListRuns must be newest first: %+v", runs)
|
||||||
|
}
|
||||||
|
if n, err := repo.CountRuns(ctx); err != nil || n < 2 {
|
||||||
|
t.Fatalf("CountRuns = %d, %v", n, err)
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in New Issue
Block a user