neon-sprawl/docs/plans/NEON-5-implementation-plan.md

# NEON-5 — Implementation plan

## Story reference

| Field | Value |
|--------|--------|
| **Key** | NEON-5 |
| **Title** | E1.M1: Persist character position (PostgreSQL) |
| **Jira** | [NEON-5](https://neon-sprawl.atlassian.net/browse/NEON-5) |
| **Parent context** | [NEON-9 — E1.M1 InputAndMovementRuntime](https://neon-sprawl.atlassian.net/browse/NEON-9) · [NEON-4](NEON-4-implementation-plan.md) (MoveCommand → `PositionState`) |

## Goal, scope, and out-of-scope

**Goal:** **`PositionState` survives server restart** via a thin PostgreSQL persistence path wired into the existing GET/POST endpoints.

**In scope**

- SQL migration or script for a minimal table: player identifier, position fields, monotonic **`sequence`**, **`updated_at`** (aligned with [`PositionSnapshot`](../../server/NeonSprawl.Server/Game/PositionState/PositionSnapshot.cs) and HTTP contract).
- Repository / store implementation used by **`IPositionStateStore`** so [`PositionStateApi`](../../server/NeonSprawl.Server/Game/PositionState/PositionStateApi.cs) stays unchanged.
- Reuse **[`docker-compose.yml`](../../docker-compose.yml)** Postgres from repo root / [root README](../../README.md).
- Document how the server picks the database (configuration / env vars).

**Out of scope (per Jira)**

- Full character creation flow, multiple zones, ORM/migration framework polish beyond what is needed for this slice.

**Persistence contract (columns vs JSONB)**

- Use **relational columns** (`pos_x`, `pos_y`, `pos_z`, `sequence`, `updated_at`) for the row — simple queries, clear mapping to `PositionSnapshot`, no JSON parsing. Note this choice in `server/README.md` when describing persistence.

## Acceptance criteria checklist

- [ ] Restart **`dotnet run`** with the same Postgres volume: last committed position is still returned from **`GET /game/players/{id}/position`** (after a prior **`POST …/move`**).
- [ ] Connection / database configuration documented (env vars or `ConnectionStrings`, aligned with ASP.NET conventions).
- [ ] No regression to the **in-memory** happy path: default **`dotnet test`** / CI without Postgres continues to behave as NEON-3/NEON-4 (see [Tests](#tests)).

## Technical approach

1. **Configuration**
   - Introduce a **connection string** (e.g. `ConnectionStrings:NeonSprawl` or `Default`) in `appsettings.Development.json` / environment overrides. Mirror docker-compose defaults already listed in [root `README.md`](../../README.md) (`localhost:5432`, `neon_sprawl`, `neon_sprawl` / `neon_sprawl_dev`).
   - **`AddPositionStateStore`**: when the connection string is **present and non-empty**, register a **PostgreSQL-backed** `IPositionStateStore`; otherwise keep **`InMemoryPositionStateStore`** (preserves CI and local tests without DB).

2. **Schema**
   - Add a versioned SQL file under the server tree (e.g. `server/db/migrations/V001__player_position.sql`) with `CREATE TABLE IF NOT EXISTS …` and indexes needed for lookups.
   - Table name suggestion: **`player_position`** (avoids implying a full **`characters`** model). Columns: `player_id` (text, primary key or unique), `pos_x`, `pos_y`, `pos_z` (`double precision`), `sequence` (`bigint`), `updated_at` (`timestamptz`). Case-insensitive id matching should match [`InMemoryPositionStateStore`](../../server/NeonSprawl.Server/Game/PositionState/InMemoryPositionStateStore.cs) (trim + ordinal case-insensitive semantics): implement with **`LOWER(TRIM(player_id))`** uniqueness / lookup pattern, or document that stored ids are normalized once — **pick one approach and apply consistently in SQL + code**.

3. **Store implementation**
   - New class (e.g. **`PostgresPositionStateStore`**) implementing **`IPositionStateStore`**, using **Npgsql** (`NpgsqlDataSource` or `NpgsqlConnection` via DI) — add package to [`NeonSprawl.Server.csproj`](../../server/NeonSprawl.Server/NeonSprawl.Server.csproj).
   - **Seed dev player:** on construction (or first DB touch), ensure the row for **`Game:DevPlayerId`** exists with **`Game:DefaultPosition`** and **`sequence = 0`**, analogous to in-memory startup seeding.
   - **`TryGetPosition`:** read from DB; return **`false`** if no row (unknown player).
   - **`TryApplyMoveTarget`:** single transactional **`UPDATE`** that sets position, increments **`sequence`**, bumps **`updated_at`**, and returns the new row (or use a concurrency-safe pattern). Unknown player → **`false`**. Preserve “404 for unknown id” behavior.

4. **Startup / DDL**
   - Either run idempotent DDL once at startup (lightweight for this story) **or** require `psql -f` / compose init — **prefer idempotent startup ensure** so `docker compose up` + `dotnet run` works without a manual step; keep the SQL file in repo as the source of truth.

5. **Documentation**
   - Update **[`server/README.md`](../../server/README.md):** persistence subsection (Postgres vs in-memory toggle), link to SQL file, note restart verification steps, document **env vars** (e.g. `ConnectionStrings__NeonSprawl` for Docker / cloud) in addition to the literals already on the root README.

## Files to add

| Path | Purpose |
|------|--------|
| `server/db/migrations/V001__player_position.sql` (or similar) | Idempotent DDL for `player_position` (+ comments). |
| `server/NeonSprawl.Server/Game/PositionState/PostgresPositionStateStore.cs` (name may vary) | Npgsql-backed `IPositionStateStore` + seed + read/update. |
| `server/NeonSprawl.Server.Tests/Game/PositionState/PostgresPositionStateIntegrationTests.cs` (name may vary) | xUnit integration tests against **real PostgreSQL** (HTTP + persistence boundary; see [Tests](#tests)). |
| Optional: `.../PositionState/DatabasePositionOptions.cs` | Section binding for connection string name / feature flag if needed. |

## Files to modify

| Path | Rationale |
|------|-----------|
| [`server/NeonSprawl.Server/NeonSprawl.Server.csproj`](../../server/NeonSprawl.Server/NeonSprawl.Server.csproj) | Add **Npgsql** (and any needed packages). |
| [`server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs`](../../server/NeonSprawl.Server/Game/PositionState/PositionStateServiceCollectionExtensions.cs) | Register Postgres vs in-memory from configuration. |
| [`server/NeonSprawl.Server/appsettings.json`](../../server/NeonSprawl.Server/appsettings.json) / `appsettings.Development.json` | Optional default connection string for local dev (or leave empty to default in-memory). |
| [`server/README.md`](../../server/README.md) | Postgres persistence, env vars, manual restart acceptance check, how to run **Postgres integration tests** locally. |
| [`.github/workflows/dotnet.yml`](../../.github/workflows/dotnet.yml) | Add a **PostgreSQL service container** (same image/credentials as [`docker-compose.yml`](../../docker-compose.yml)) and pass a **connection string** into the **Test** step so Postgres integration tests run on every push/PR that touches the server. |

## Tests

- **Existing** [`MoveCommandApiTests`](../../server/NeonSprawl.Server.Tests/Game/PositionState/MoveCommandApiTests.cs) and [`PositionStateApiTests`](../../server/NeonSprawl.Server.Tests/Game/PositionState/PositionStateApiTests.cs): unchanged setup — **no** connection string so the host keeps **`InMemoryPositionStateStore`** — satisfies “no regression to in-memory happy path.”
- **New (required):** **PostgreSQL integration tests** per [`.cursor/rules/testing-expectations.md`](../../.cursor/rules/testing-expectations.md): use **`WebApplicationFactory<Program>`** with configuration (or environment) that sets the same **connection string** the app uses for the durable store, pointing at real Postgres.
  - Cover at least: **POST move** then **GET** sees stored position and **`sequence`**; **persistence across host restarts** simulated by **disposing** the first factory and creating a **second** `WebApplicationFactory` with the same connection string, then **GET** still returns the last committed state (matches Jira “restart server + DB” without a literal process kill).
  - Optionally add a **failure-path** case (e.g. unknown player **404** on move when row absent) if the implementation keeps that behavior for Postgres.
- **CI:** [.github/workflows/dotnet.yml](../../.github/workflows/dotnet.yml) must run these tests **always** in the existing job (service container + env), not as an optional manual job — so the durable path is gated like the rest of the suite.
- **Local:** document in **`server/README.md`**: run **`docker compose up -d`** (or any Postgres with matching credentials) and set the same connection string env vars the workflow uses, then `dotnet test`.

**Manual verification (Jira AC, optional double-check):** `docker compose up -d` → `dotnet run` with Postgres enabled → `POST …/move` → stop server → `dotnet run` again → `GET …/position` matches last move.

## Open questions / risks

- **Case sensitivity:** DB must match current **trim + case-insensitive** player id semantics; enforce via normalized column or `LOWER` constraints to avoid subtle 404 vs memory mismatches.
- **Service container vs Testcontainers:** Prefer **GitHub Actions `services: postgres`** for CI (no new test dependency). If flakiness appears, consider **Testcontainers** later; initial implementation should not defer integration tests.

## Pull request description (draft)

**NEON-5:** Persist `PositionState` in PostgreSQL when a connection string is configured; keep in-memory store when not. Adds DDL + Npgsql-backed `IPositionStateStore`, **PostgreSQL integration tests** (including simulating restart via a second web factory), **Postgres service in `dotnet.yml`**, and README notes for env vars and local test runs.