From 06d160daf0a50093a8ddc6ad8a66d70ea59f9145 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Fri, 22 May 2026 13:00:34 +0300 Subject: [PATCH] [AZ-808] [AZ-809] [AZ-810] [AZ-811] [AZ-812] Cycle 8 Step 9 queued Step 9 (New Task) closure for cycle 8. Queues 5 task specs under the AZ-795 strict-validation umbrella + OSM-naming harmonization: - AZ-808 region-request validator (POST /api/satellite/request) 3 pts - AZ-809 route-creation validator (POST /api/satellite/route) 5 pts - AZ-810 UAV upload metadata validator (POST /api/satellite/upload) 5 pts - AZ-811 lat/lon GET validator (GET /api/satellite/tiles/latlon) 2 pts - AZ-812 Region DTO rename latitude/longitude -> lat/lon 3 pts Total 18 SP. Origin: cross-repo request from gps-denied-onboard agent (2026-05-22) after AZ-777 Phase 2 black-box probe of the Region API surfaced silent-coercion behavior + the lone OSM-deviating coord naming convention left in the producer's public surface. Ordering recorded (per /autodev Step 10 dirty-tree decision): AZ-812 ships first so AZ-808 validator + contract doc + integration tests are written against the final lat/lon names. AZ-809/AZ-810/AZ-811 are independent of AZ-812 (their DTOs already use OSM short form). Deps table updated: cycle-8b (AZ-812) folded into cycle-8 ordering as step 1; AZ-808 dependency upgraded SOFT -> HARD on AZ-812. Co-authored-by: Cursor --- _docs/02_tasks/_dependencies_table.md | 45 +++++ .../todo/AZ-808_region_endpoint_validation.md | 132 +++++++++++++++ .../todo/AZ-809_route_endpoint_validation.md | 156 ++++++++++++++++++ .../todo/AZ-810_upload_metadata_validation.md | 147 +++++++++++++++++ .../AZ-811_latlon_get_endpoint_validation.md | 105 ++++++++++++ .../todo/AZ-812_region_field_rename_to_osm.md | 117 +++++++++++++ 6 files changed, 702 insertions(+) create mode 100644 _docs/02_tasks/todo/AZ-808_region_endpoint_validation.md create mode 100644 _docs/02_tasks/todo/AZ-809_route_endpoint_validation.md create mode 100644 _docs/02_tasks/todo/AZ-810_upload_metadata_validation.md create mode 100644 _docs/02_tasks/todo/AZ-811_latlon_get_endpoint_validation.md create mode 100644 _docs/02_tasks/todo/AZ-812_region_field_rename_to_osm.md diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 1c10390..87fd9e5 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -127,6 +127,31 @@ Adopted into satellite-provider cycle 7 with the recommended ordering: shared va | AZ-795 | Strict input validation across all public endpoints (FluentValidation + ProblemDetails) — **Epic with shared-infra ship** | — (children gated on shared infra landing first) | — (epic; shared-infra estimate 5–8 pts; per-endpoint children ~3 pts each) | Done — shared infra shipped (cycle 7); future per-endpoint child tasks open | | AZ-796 | Strict validation for inventory endpoint (POST /api/satellite/tiles/inventory) | AZ-795 (HARD — shared infra); coordinate with AZ-794 | 3 | Done (cycle 7) | +### Step 9 cycle 8 — Per-endpoint validation children of AZ-795 (cross-repo follow-up) + +Source: cross-repo request from `gps-denied-onboard` agent (2026-05-22). After AZ-795 shipped the shared infra (FluentValidation + GlobalExceptionHandler + UnmappedMemberHandling.Disallow + ValidationEndpointFilter) and AZ-796 shipped the inventory endpoint as the first concrete child, four additional public endpoints remain silent-coercion-permissive: `POST /api/satellite/request` (region onboarding), `POST /api/satellite/route` (route creation), `POST /api/satellite/upload` (UAV metadata layer; the file-level quality gate from AZ-488 stays), `GET /api/satellite/tiles/latlon` (single-tile download). All four are queued here as cycle-8 candidates, each mirroring the AZ-796 reference implementation pattern with endpoint-specific adaptations. + +**Cross-repo context**: AZ-808 + AZ-809 are blocking dependencies for gps-denied-onboard AZ-777 Phase 2 (Derkachi reference tile catalog seeding). AZ-810 is a defense-in-depth tightening for the existing AZ-488 UAV upload path. AZ-811 is the smallest item, included for completeness of the per-endpoint surface. + +| Task | Title | Depends On | Points | Status | +|------|-------|-----------|--------|--------| +| AZ-808 | Strict validation for region-request endpoint (POST /api/satellite/request) | AZ-795 (HARD — shared infra); AZ-796 (reference); AZ-812 (HARD — ships first in cycle 8 per /autodev step 10 user decision 2026-05-22; AZ-808 spec field references rewrite from `latitude`/`longitude` → `lat`/`lon` before validator implementation starts) | 3 | To Do | +| AZ-809 | Strict validation for route-creation endpoint (POST /api/satellite/route) | AZ-795 (HARD); AZ-796 (single-DTO reference); AZ-808 (no-prior-contract reference) | 5 | To Do | +| AZ-810 | Strict validation for UAV upload metadata (POST /api/satellite/upload) | AZ-795 (HARD); AZ-796 (single-DTO reference); AZ-809 (nested per-item reference); AZ-488 (must remain green); AZ-503 (flightId semantics) | 5 | To Do | +| AZ-811 | Strict validation for lat/lon tile GET endpoint (GET /api/satellite/tiles/latlon) | AZ-795 (HARD); AZ-796 (single-DTO reference); AZ-808 (no-prior-contract reference) | 2 | To Do | + +**Spec amendments (2026-05-22, post-probe)**: AZ-808 and AZ-809 specs were amended after a gps-denied-onboard black-box probe of the running producer surfaced two real silent-coercion gaps and one input/output naming asymmetry. Notable spec changes: (1) AZ-808 rule count 8 → 9 (added `Id` non-zero-Guid rule); (2) AZ-809 rule count 13 → 14 (added `Id` non-zero-Guid rule); (3) AZ-809 added AC-10 advisory documenting the input/output point-naming asymmetry on `RouteResponse.points[]`; (4) AZ-808 added field-naming coordination section pointing at AZ-812. Story-point estimates unchanged; the new rules were already implicit in the AZ-795 epic's mandate. + +### Step 9 cycle 8b — Region API field-name harmonization (cross-repo follow-up) + +Source: cross-repo request from `gps-denied-onboard` agent (2026-05-22). After the AZ-777 Phase 2 black-box probe of the Region API, the consumer attempted `{"lat":..,"lon":..}` against `POST /api/satellite/request` and received HTTP 400 with `UnmappedMemberHandling.Disallow` rejecting the unknown fields. The producer DTO uses verbose `latitude`/`longitude`, which is the **only** OSM-deviating coord convention left in the public API surface: the inventory endpoint already uses `z/x/y` (per AZ-794), the Route endpoint's `RoutePoint`/`GeoPoint` already use `lat`/`lon` (per existing `[JsonPropertyName]`), and the slippy-map URL uses `z/x/y`. AZ-812 closes the inconsistency by renaming Region to match. + +This is a separate cycle (8b) because it's a **wire-format rename** (mirror of AZ-794) rather than a validator add (mirror of AZ-796). The two operations are surgically distinct even though they touch the same DTO. + +| Task | Title | Depends On | Points | Status | +|------|-------|-----------|--------|--------| +| AZ-812 | satellite-provider: rename `RequestRegionRequest.{Latitude, Longitude}` → `{Lat, Lon}` (OSM convention) + harmonize cross-endpoint | — (coordinate release ordering with AZ-808) | 3 | To Do | + ## Execution Order ### Step 6 @@ -193,6 +218,24 @@ Adopted into cycle 7. Ordering: 3. AZ-796 (inventory validator) — first per-endpoint child; serves as reference implementation for sibling per-endpoint child tasks. 4. Sibling per-endpoint child tasks under AZ-795 — added by parent-suite team as they enumerate the surface from `/swagger/v1/swagger.json` (out of cycle 7 scope; future cycles). +### Step 9 cycle 8 (AZ-808 / AZ-809 / AZ-810 / AZ-811 / AZ-812) + +Ordering decision recorded 2026-05-22 (`/autodev` Step 10 dirty-tree resolution): **Option 1 (AZ-812 first, then AZ-808 against final lat/lon names)** — chosen to avoid AZ-808 double-migration on contract doc + integration tests. AZ-809, AZ-810, AZ-811 are independent of AZ-812 (their DTOs already use OSM short form). + +Execution order: + +1. AZ-812 (3 SP) — Region DTO rename `Latitude/Longitude` → `Lat/Lon`. Ships first; AZ-808 depends on its outcome. Own batch (wire-format change is atomic; independent rollback target). +2. AZ-811 (2 SP) — smallest validator unblocker; closes the simplest endpoint and validates the query-param filter pattern for any future query-string endpoints. Independent of AZ-812. +3. AZ-808 (3 SP) — region-request validator written against post-rename `lat/lon`; unblocks gps-denied-onboard AZ-777 Phase 2 bbox-based seeding path. Hard-depends on AZ-812. +4. AZ-809 (5 SP) — route-creation validator; unblocks gps-denied-onboard AZ-777 Phase 2 route-based (preferred) seeding path. Independent of AZ-812. +5. AZ-810 (5 SP) — UAV upload metadata validator; defense-in-depth for AZ-488 multipart endpoint. Independent of AZ-812. + +Parent-suite team may reorder steps 2–5 based on consumer priorities; step 1 (AZ-812) must remain first. + +### Step 9 cycle 8b (AZ-812 — folded into cycle 8 ordering above) + +Originally tracked as a separate cycle 8b because AZ-812 is a wire-format rename (mirror of AZ-794) rather than a validator add (mirror of AZ-796). After the /autodev Step 10 ordering decision above, cycle 8b folds into cycle 8 as step 1 of the execution order. Section retained for traceability — the cycle-8b table entry remains the authoritative spec marker for AZ-812. + ## Total Effort Step 6: 6 tasks, 17 story points @@ -205,6 +248,8 @@ Step 9 cycle 4: 1 task created (AZ-500 = 5 pts) Step 9 cycle 5: 3 tasks tracked (AZ-503 = 3 pts foundation-half, AZ-504 = 1 pt, AZ-505 = 3 pts split-off-deferred) — 4 pts committed to cycle 5, 3 pts deferred to cycle 6 Step 9 cycle 6: 1 task scheduled (AZ-505 = 3 pts) — consumed from cycle-5 deferral Step 9 cycle 7: 3 tasks adopted (AZ-794 = 3 pts rename, AZ-795 = epic with 5–8 pts shared-infra ship, AZ-796 = 3 pts first per-endpoint child) — total ~11–14 pts (over the 2–5 pts/cycle preference; AZ-795's shared-infra ship is the heavy item). Origin: gps-denied-onboard AZ-777 Phase 1 Jetson probe (2026-05-22). Sibling per-endpoint child tasks under AZ-795 to be added in future cycles as the parent-suite team enumerates the endpoint surface. +Step 9 cycle 8: 5 tasks queued (AZ-812 = 3 pts Region DTO rename, AZ-808 = 3 pts region validator, AZ-809 = 5 pts route, AZ-810 = 5 pts UAV upload metadata, AZ-811 = 2 pts lat/lon GET) — total 18 pts across 4 per-endpoint AZ-795 children + 1 OSM-naming harmonization. Origin: cross-repo request from gps-denied-onboard agent (2026-05-22) for completeness of validation surface after AZ-795/796 landed, plus AZ-777 Phase 2 black-box probe surfacing the Region DTO as the lone OSM hold-out. Ordering: AZ-812 first (per /autodev Step 10 user decision), then AZ-808/809/810/811 (independent of each other modulo AZ-812). AZ-808 and AZ-809 specs amended 2026-05-22 post-probe to add `Id` non-zero-Guid rule + Route AC-10 input/output naming asymmetry advisory. +Step 9 cycle 8b: folded into cycle 8 as step 1 (AZ-812). Section retained in dependency table for traceability. ## Coverage Verification diff --git a/_docs/02_tasks/todo/AZ-808_region_endpoint_validation.md b/_docs/02_tasks/todo/AZ-808_region_endpoint_validation.md new file mode 100644 index 0000000..3aafea3 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-808_region_endpoint_validation.md @@ -0,0 +1,132 @@ +# Strict validation for region-request endpoint (POST /api/satellite/request) + +**Task**: AZ-808_region_endpoint_validation +**Name**: Strict validation for region-request endpoint +**Description**: Add FluentValidation-backed strict input validation to `POST /api/satellite/request` (region onboarding — enqueues a square region of tiles for async Google-Maps backfill). Reject malformed payloads with RFC 7807 ValidationProblemDetails (HTTP 400). Second concrete child of AZ-795; reuses the shared infra wired in cycle 7. +**Complexity**: 3 points (7 validation rules — was 6 before the 2026-05-22 probe added the `Id` rule) +**Dependencies**: AZ-795 (HARD — shared infra already landed in cycle 7); AZ-796 (reference implementation pattern); AZ-812 (field-naming coordination — see below) +**Component**: SatelliteProvider.Api/Validators + SatelliteProvider.Common (RequestRegionRequest DTO) +**Tracker**: AZ-808 (https://denyspopov.atlassian.net/browse/AZ-808) +**Epic**: AZ-795 — Strict input validation across all public endpoints +**Originating ticket**: gps-denied-onboard AZ-777 Phase 2 (cross-repo, 2026-05-22) — consumer needs this endpoint to seed Derkachi reference tile catalog; black-box probe surfaced concrete silent-coercion behavior + +## Scope + +Add FluentValidation-backed strict input validation to `POST /api/satellite/request` (region onboarding — enqueues a square region of tiles for async Google-Maps backfill). Reject malformed payloads with **RFC 7807 ValidationProblemDetails** (HTTP 400) per the Epic's `error-shape.md` v1.0.0 contract. + +Originating discovery: AZ-777 Phase 2 (gps-denied-onboard) — the consumer needs to call this endpoint to seed the Derkachi reference tile catalog. A black-box probe (2026-05-22) confirmed real silent-coercion behavior that this task fixes (see *Probe-confirmed gaps* below). + +Jira AZ-808 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Probe-confirmed gaps (2026-05-22) + +A black-box probe of the running producer captured these concrete behaviors that this task must close: + +1. **`Id` silently coerces to zero-Guid when omitted.** Body `{"latitude":49.94,"longitude":36.31,"sizeMeters":200,"zoomLevel":18,"stitchTiles":false}` (no `id`) returned HTTP 200 with `"id":"00000000-0000-0000-0000-000000000000"` and `status:queued`. The `[Required]` DataAnnotation on `RequestRegionRequest.Id` is NOT enforced — the deserializer just yields the default Guid. This is the same silent-coercion class that motivated AZ-795. Validator must reject zero-Guid + missing-Id with the same RFC 7807 shape as the inventory validator. +2. **`UnmappedMemberHandling.Disallow` IS active for this endpoint.** Sending the wrong field name (`{"lat":49.94,...}`) returned HTTP 400 with the proper ValidationProblemDetails shape: `{"errors":{"lat":["The JSON property 'lat' could not be mapped to any .NET member contained in type 'SatelliteProvider.Common.DTO.RequestRegionRequest'."]}}`. So rule 8 (unknown-field rejection) is already covered by AZ-795 cycle-7 shared infra; this task only needs to verify it stays active after wiring `WithValidation()`. +3. **Happy path works end-to-end.** With the correct shape `{"id":"","latitude":..,"longitude":..,"sizeMeters":200,"zoomLevel":18,"stitchTiles":false}`: HTTP 200 + regionId + 9 tiles downloaded from Google Maps + accessible via `GET /tiles/{z}/{x}/{y}` (13 KB JPEG verified). Validator must NOT regress this path. + +## Field-naming coordination with AZ-812 + +This spec uses the **current wire format** (`latitude`, `longitude`) because that's what the DTO ships today and that's what the validator must reject malformed values for. **AZ-812** (mirror of AZ-794 for inventory) is filed to rename these to `lat`/`lon` for OSM-style consistency across all satellite-provider endpoints. + +If AZ-812 lands **before** this task, rewrite all field references in this spec from `latitude`/`longitude` to `lat`/`lon` before implementing. If AZ-812 lands **after** this task, AZ-812 must also update the validator + contract doc + integration tests. Pick the ordering during planning to avoid double migration. + +## Endpoint surface + +`POST /api/satellite/request` + +Current wire format (per `RequestRegionRequest.cs`, probe-confirmed 2026-05-22): + +```json +{ + "id": "", + "latitude": 50.10, + "longitude": 36.10, + "sizeMeters": 5000, + "zoomLevel": 18, + "stitchTiles": false +} +``` + +Response: HTTP 200 with `RegionStatusResponse` (id, status, csvFilePath, summaryFilePath, tilesDownloaded, tilesReused, createdAt, updatedAt). Async — the actual tile downloads happen in the background via `RegionProcessingService` (Flow F3). Caller polls `GET /api/satellite/region/{id}` until `status:completed`. + +## Required validations + +1. **Body present** — null/empty body → 400 (`errors.$`). +2. **`id` required, non-zero Guid** — NEW (probe-confirmed gap). Missing or `00000000-...` → 400 with `errors.id`. Use `RuleFor(x => x.Id).NotEmpty()` (FluentValidation's `NotEmpty()` rejects default-Guid). +3. **`latitude` required** — double, in `[-90.0, 90.0]`. Out-of-range or missing → 400 with `errors.latitude`. +4. **`longitude` required** — double, in `[-180.0, 180.0]`. Out-of-range or missing → 400 with `errors.longitude`. +5. **`sizeMeters` required** — double, in `[100.0, 10000.0]` (matches current inline check in `RequestRegion Handler` per `api_program.md`). Out-of-range or missing → 400 with `errors.sizeMeters`. +6. **`zoomLevel` required** — int, in `[0, 22]` (align with `TileCoordValidator` slippy-map range used by AZ-796 for the inventory endpoint). Out-of-range or missing → 400 with `errors.zoomLevel`. +7. **`stitchTiles` required** — bool. Missing → 400 with `errors.stitchTiles` (no defaulting to `false` — force the caller to declare intent). +8. **Unknown root fields rejected** — already covered by AZ-795's `UnmappedMemberHandling.Disallow` (probe-confirmed active). Verify it stays active after wiring `WithValidation()`. +9. **Type mismatch** — e.g. `"latitude": "fifty"` → 400 with `errors.latitude` ("could not be parsed"). Already covered by AZ-795's `GlobalExceptionHandler`; verify it triggers for this endpoint. + +## Implementation pattern (mirror AZ-796) + +1. New file: `SatelliteProvider.Api/Validators/RegionRequestValidator.cs` — `AbstractValidator` with rules 2–7. +2. Mark `RequestRegionRequest` props with `[JsonRequired]` (replacing or supplementing the existing `[Required]` DataAnnotation — the latter is not enforced by `System.Text.Json`, as the probe confirmed). Apply to `Id`, `Latitude`, `Longitude`, `SizeMeters`, `ZoomLevel`, `StitchTiles`. +3. Add `.WithValidation()` to the `MapPost("/api/satellite/request", ...)` chain in `Program.cs`. +4. Unit tests: `SatelliteProvider.Tests/Validators/RegionRequestValidatorTests.cs` — one test per `RuleFor(...)` (≥ 6 methods covering id, latitude, longitude, sizeMeters, zoomLevel, stitchTiles). +5. Integration tests: `SatelliteProvider.IntegrationTests/RegionRequestValidationTests.cs` (new file) — ≥ 9 methods (1 happy + 1 per failure-mode AC — including missing-id reproducing the probe's silent-coercion case). +6. Manual probe: `scripts/probe_region_validation.sh` (mirrors `scripts/probe_inventory_validation.sh` from AZ-796). MUST include the missing-id test case. + +## New contract doc + +Create `_docs/02_document/contracts/api/region-request.md` v1.0.0. The region endpoint has **no formal contract** today (only `system-flows.md` F2 + module docs). The contract doc must cover: + +- Endpoint, auth, request body, response body (use the actual `RegionStatusResponse` shape: id, status, csvFilePath, summaryFilePath, tilesDownloaded, tilesReused, createdAt, updatedAt), error shape (reference `error-shape.md` v1.0.0). +- Invariants (one regionId per request; client-provided non-zero Id; size cap; async semantics — caller must poll `GET /api/satellite/region/{id}`). +- Test cases mirroring the validator rules (same `Case | Input | Expected | Notes` table format as `tile-inventory.md` v2.0.0). MUST include the missing-id case. +- Cross-link to `RegionStatus` flow (F3) and the consumer-facing inventory contract (`tile-inventory.md` — callers seed via region, then read via inventory). +- Reference to AZ-812 (field-naming follow-up). + +## Coordination with sibling tickets + +- **Parent (AZ-795)**: depends on shared infra already landed in cycle 7. +- **AZ-796 (inventory)**: reference implementation — copy the validator + integration-test layout 1:1. +- **AZ-812 (region field rename)**: hard coordination on field names. See *Field-naming coordination with AZ-812* above. +- **AZ-777 (gps-denied-onboard)**: consumer-side dependency — Phase 2 cannot proceed safely until this validator lands AND the contract doc exists. Consumer has black-box-probed the endpoint and can use it today, but silent-coercion bugs make Phase 2 fragile until validation is in place. +- Sibling validation tasks created in the same batch: **AZ-809** (route), **AZ-810** (UAV upload metadata), **AZ-811** (lat/lon GET). + +## Acceptance criteria + +**AC-1**: Each of the 9 validations above rejects with HTTP 400 + ValidationProblemDetails (single-rule precision; unrelated rules NOT in the `errors` map). + +**AC-2**: Happy path unchanged — a valid body still returns HTTP 200 + `RegionStatusResponse`; background processing still runs; the probe's 9-tile Derkachi case (`{"id":"","latitude":49.94,"longitude":36.31,"sizeMeters":200,"zoomLevel":18,"stitchTiles":false}`) still completes in under 10 seconds. + +**AC-3**: `RegionRequestValidator` lives in its own file under `SatelliteProvider.Api/Validators/` and is unit-tested (≥ 1 test per `RuleFor`). + +**AC-4**: `SatelliteProvider.IntegrationTests/RegionRequestValidationTests.cs` covers happy + 8+ failure modes with full ValidationProblemDetails assertion (use the existing `ProblemDetailsAssertions` helper from AZ-795). MUST include `Post_WithMissingId_ReturnsBadRequest` (reproducing the 2026-05-22 probe's silent-coercion case). + +**AC-5**: `_docs/02_document/contracts/api/region-request.md` v1.0.0 created and published. + +**AC-6**: `_docs/02_document/system-flows.md` F2 updated to reference the new contract doc + error shape. + +**AC-7**: OpenAPI spec marks `RequestRegionRequest` fields `required`, declares ranges, and documents the 400 response (matches AZ-796 Swashbuckle annotations). + +**AC-8**: Manual probe script exercises each failure mode end-to-end via `curl` + JWT. + +## Out of scope + +- The Region API's processing semantics (Flow F3 — `RegionProcessingService`) — validation lives at the API layer only. +- Any change to `IRegionService.RequestRegionAsync` signature beyond accepting the validated DTO. +- `GET /api/satellite/region/{id}` status endpoint (separate task if path-parameter validation needed; current Guid binding is framework-handled). +- The field-name rename (`Latitude/Longitude` → `Lat/Lon`) — handled by AZ-812. +- Performance — validation overhead is negligible vs the async enqueue + Google Maps round-trip. + +## Constraints + +- **Breaking behavior change** — any consumer today omitting `id` (silently getting zero-Guid) or sending malformed values will start getting 400. Known consumer set: gps-denied-onboard (currently uses correct body shape with id, per black-box probe 2026-05-22). Other consumers TBD by parent-suite team. +- No regression in any existing `RegionRequestTests.cs` happy-path coverage. + +## References + +- Jira AZ-808: https://denyspopov.atlassian.net/browse/AZ-808 +- Parent Epic: AZ-795 (shared infra; error-shape contract) +- Reference implementation: AZ-796 (inventory endpoint) +- Coordination: AZ-812 (region field-name rename to OSM convention) +- Cycle-7 retro: `_docs/06_metrics/retro_2026-05-22_cycle7.md` (flagged this endpoint as next in line) +- Originating consumer discovery: gps-denied-onboard AZ-777 Phase 2 (2026-05-22 black-box probe) +- Related contract docs: `error-shape.md` v1.0.0, `tile-inventory.md` v2.0.0 (both produced by AZ-795+AZ-796 cycle 7) diff --git a/_docs/02_tasks/todo/AZ-809_route_endpoint_validation.md b/_docs/02_tasks/todo/AZ-809_route_endpoint_validation.md new file mode 100644 index 0000000..a0df5be --- /dev/null +++ b/_docs/02_tasks/todo/AZ-809_route_endpoint_validation.md @@ -0,0 +1,156 @@ +# Strict validation for route-creation endpoint (POST /api/satellite/route) + +**Task**: AZ-809_route_endpoint_validation +**Name**: Strict validation for route-creation endpoint +**Description**: Add FluentValidation-backed strict input validation to `POST /api/satellite/route` (route creation — client submits ordered waypoints + optional geofence polygons; producer interpolates intermediate points every ≈ 200 m and — if `requestMaps=true` — enqueues a region request per route point for async tile backfill). Reject malformed payloads with RFC 7807 ValidationProblemDetails (HTTP 400). Third concrete child of AZ-795; reuses the shared infra wired in cycle 7. +**Complexity**: 5 points (14 rules — was 13 before the 2026-05-22 probe added the `Id` rule; 3 validator classes; cross-field constraint; new contract doc) +**Dependencies**: AZ-795 (HARD — shared infra); AZ-796 (single-DTO reference); AZ-808 (no-prior-contract pattern, same batch) +**Component**: SatelliteProvider.Api/Validators + SatelliteProvider.Common (CreateRouteRequest, RoutePoint, GeofencePolygon, GeoPoint DTOs) +**Tracker**: AZ-809 (https://denyspopov.atlassian.net/browse/AZ-809) +**Epic**: AZ-795 — Strict input validation across all public endpoints +**Originating ticket**: gps-denied-onboard AZ-777 Phase 2 (cross-repo, 2026-05-22) — route-based seeding is the consumer's preferred imagery seeding path; black-box probe surfaced silent-coercion + input/output naming asymmetry + +## Scope + +Add FluentValidation-backed strict input validation to `POST /api/satellite/route`. Reject malformed payloads with **RFC 7807 ValidationProblemDetails** (HTTP 400) per the Epic's `error-shape.md` v1.0.0 contract. + +Originating discovery: AZ-777 Phase 2 (gps-denied-onboard) — the consumer's preferred imagery seeding path is route-based (flight-track waypoints) rather than bbox-based, so this endpoint is the primary integration target for the Derkachi reference tile catalog. A black-box probe (2026-05-22) confirmed real silent-coercion behavior and an input/output naming asymmetry (see *Probe-confirmed gaps* below). + +Jira AZ-809 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Probe-confirmed gaps (2026-05-22) + +A black-box probe of the running producer captured these concrete behaviors: + +1. **`Id` silently coerces to zero-Guid when omitted.** Same gap as the Region endpoint (AZ-808). `CreateRouteRequest.Id` has no `[Required]` and no `[JsonRequired]`, so the deserializer yields zero-Guid. Validator must reject missing/zero Id. +2. **Happy path works end-to-end** for both `requestMaps:false` (route storage only, instant) and `requestMaps:true` (route + background tile backfill, ~15s for a 2-point 132m route at z=18). Validator must NOT regress. +3. **Input/output naming asymmetry on points** (new finding). Input `points: [{"lat":..,"lon":..}]` (OSM short form, per `[JsonPropertyName("lat")]` on `RoutePoint`). But the **response** echoes points as `{"latitude":..,"longitude":..,"pointType":..,"sequenceNumber":..,"segmentIndex":..,"distanceFromPrevious":..}`. This is a DTO round-trip inconsistency on the same object type. NOT in scope for this validation task, but surfaced as **AC-10** (advisory) so the parent-suite team can decide whether to file a follow-up. +4. **`UnmappedMemberHandling.Disallow` is active globally** (verified via AZ-808 probe), so unknown-field rejection (rule 13) will work out-of-the-box once `WithValidation()` is wired. + +## Endpoint surface + +`POST /api/satellite/route` + +Current wire format (per `CreateRouteRequest`, probe-confirmed 2026-05-22): + +```json +{ + "id": "a1b2c3d4-...", + "name": "derkachi-flight-1", + "description": "AZ-777 Phase 2 seed route", + "regionSizeMeters": 1000, + "zoomLevel": 18, + "points": [ + { "lat": 50.10, "lon": 36.10 }, + { "lat": 50.11, "lon": 36.11 } + ], + "geofences": { + "polygons": [ + { "northWest": { "lat": 50.15, "lon": 36.05 }, + "southEast": { "lat": 50.05, "lon": 36.15 } } + ] + }, + "requestMaps": true, + "createTilesZip": false +} +``` + +Response (current, probe-confirmed): HTTP 200 with `RouteResponse` (id, name, description, regionSizeMeters, zoomLevel, totalDistanceMeters, totalPoints, points[], requestMaps, mapsReady, csvFilePath, summaryFilePath, stitchedImagePath, tilesZipPath, createdAt, updatedAt). Note response uses `latitude`/`longitude` for echoed points — see AC-10. + +Background processing per Flow F5 if `requestMaps=true`; client polls `GET /api/satellite/route/{id}` until `mapsReady:true`. + +## Required validations + +1. **Body present** — null/empty body → 400 (`errors.$`). +2. **`id` required, non-zero Guid** — NEW (probe-confirmed gap, same as AZ-808). Missing or `00000000-...` → 400 with `errors.id`. +3. **`name` required** — non-empty string, length `[1, 200]`. Missing/empty → 400 with `errors.name`. +4. **`description` optional** — if present, length `[0, 1000]`. Over cap → 400 with `errors.description`. +5. **`regionSizeMeters` required** — double, in `[100.0, 10000.0]` (align with region endpoint). Out-of-range or missing → 400 with `errors.regionSizeMeters`. +6. **`zoomLevel` required** — int, in `[0, 22]` (align with `TileCoordValidator`). Out-of-range or missing → 400 with `errors.zoomLevel`. +7. **`points` required, non-empty** — at least **2 entries** (current `Flow F4` precondition), at most **500 entries** (cap to prevent runaway region-enqueue — confirm cap with parent-suite team). Below 2 or above 500 → 400 with `errors.points`. +8. **Per-point**: `lat` required, double, in `[-90.0, 90.0]`; `lon` required, double, in `[-180.0, 180.0]`. Missing/out-of-range → 400 with `errors.points[i].lat` or `.lon`. +9. **`geofences` optional** — if present: + - `polygons` required, non-empty. + - Per-polygon: `northWest` + `southEast` both required, each with valid `lat`/`lon`. + - Cross-field invariant: `northWest.lat > southEast.lat` AND `northWest.lon < southEast.lon` (i.e. NW is genuinely north-of and west-of SE). + - Violations → 400 with `errors.geofences.polygons[i].`. +10. **`requestMaps` required** — bool. Missing → 400 with `errors.requestMaps`. +11. **`createTilesZip` required** — bool. Missing → 400 with `errors.createTilesZip`. +12. **Cross-field constraint**: `createTilesZip == true` implies `requestMaps == true` (can't zip what wasn't downloaded). Violation → 400 with `errors.$` or `errors.createTilesZip`. +13. **Unknown root or nested fields rejected** — covered by AZ-795's `UnmappedMemberHandling.Disallow` (probe-confirmed active globally via AZ-808). Any unknown field at any nesting level → 400 with `errors.` ("could not be mapped to any .NET member"). +14. **Type mismatch** — e.g. `"lat": "fifty"` at any nesting level → 400 with `errors.`. Covered by AZ-795's `GlobalExceptionHandler`. + +## Implementation pattern (mirror AZ-796, extended for nesting) + +1. New files (all under `SatelliteProvider.Api/Validators/`): + - `CreateRouteRequestValidator.cs` — root validator with rules 2–7, 10–12. + - `RoutePointValidator.cs` — per-point validator (rule 8); invoked via `RuleForEach(x => x.Points).SetValidator(new RoutePointValidator())`. + - `GeofencePolygonValidator.cs` — per-polygon validator (rule 9); invoked via `RuleForEach(x => x.Geofences.Polygons).SetValidator(new GeofencePolygonValidator())` (guarded by `When(x => x.Geofences != null)`). +2. Mark required props on `CreateRouteRequest`, `RoutePoint`, `Geofences`, `GeofencePolygon`, `GeoPoint` with `[JsonRequired]` per the cycle-7 `TileCoord` pattern. Pay special attention to `Id` (probe confirmed it's not enforced today). +3. Add `.WithValidation()` to the `MapPost("/api/satellite/route", ...)` chain. +4. Unit tests: `SatelliteProvider.Tests/Validators/CreateRouteRequestValidatorTests.cs` + `RoutePointValidatorTests.cs` + `GeofencePolygonValidatorTests.cs` (≥ 13 test methods total — one per `RuleFor`/`RuleForEach` chain; new id-rule method must reproduce the probe's missing-id case). +5. Integration tests: `SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs` (new file) — ≥ 14 methods (1 happy + 1 per failure-mode AC). +6. Manual probe: `scripts/probe_route_validation.sh`. MUST include missing-id, NW-southeast-inverted polygon, points-too-few, createTilesZip-without-requestMaps. + +## New contract doc + +Create `_docs/02_document/contracts/api/route-creation.md` v1.0.0. Like the region endpoint, this has **no formal contract** today. Cover: + +- Endpoint, auth, request body (with nested DTO recursion), response body (`RouteResponse` shape — acknowledge the input/output point-naming asymmetry; reference AC-10 advisory), error shape (reference `error-shape.md` v1.0.0). +- Invariants (client-provided non-zero Id; one routeId per request; min 2 points; max 500 points; polygon NW>SE; cross-field createTilesZip implies requestMaps). +- Test cases table (same format as `tile-inventory.md` v2.0.0). MUST include missing-id, geofence NW/SE inversion, createTilesZip cross-field, points-too-few cases. +- Cross-link to Flow F4 (Route Creation) + Flow F5 (Route Map Processing background) + `region-request.md` (referenced by F5 enqueue path). + +## Coordination with sibling tickets + +- **Parent (AZ-795)**: depends on shared infra already landed in cycle 7. +- **AZ-796 (inventory)**: reference for single-DTO validator pattern. +- **AZ-808 (region)**: reference for endpoint without prior contract doc (same precondition: must create new `region-request.md`); coordinate field-name conventions across the two contracts. The naming inconsistency `RequestRegionRequest.sizeMeters` vs `CreateRouteRequest.regionSizeMeters` (same concept, different names) is flagged in AC-9. +- **AZ-812 (region field rename)**: tangentially related — AZ-812 is bringing Region into the lat/lon convention that Route already uses. No direct dependency on this task. +- **AZ-777 (gps-denied-onboard)**: consumer-side dependency — Phase 2 cannot proceed safely until this validator lands AND `route-creation.md` exists. + +## Acceptance criteria + +**AC-1**: Each of the 14 validations above rejects with HTTP 400 + ValidationProblemDetails (single-rule precision). + +**AC-2**: Happy path unchanged — a valid body still returns HTTP 200 + `RouteResponse`; background F5 processing still runs when `requestMaps=true`; probe's 2-point 132m route still completes (`mapsReady:true`) in under 20 seconds. + +**AC-3**: All three validators (`CreateRouteRequestValidator`, `RoutePointValidator`, `GeofencePolygonValidator`) live in their own files under `SatelliteProvider.Api/Validators/` and are unit-tested (≥ 1 test per `RuleFor`/`RuleForEach` chain, ≥ 13 methods total). + +**AC-4**: `SatelliteProvider.IntegrationTests/CreateRouteValidationTests.cs` covers happy + 13+ failure modes with full ValidationProblemDetails assertion. MUST include `Post_WithMissingId_ReturnsBadRequest` (reproducing the 2026-05-22 probe's silent-coercion case). + +**AC-5**: `_docs/02_document/contracts/api/route-creation.md` v1.0.0 created and published. + +**AC-6**: `_docs/02_document/system-flows.md` F4 + F5 updated to reference the new contract doc + error shape. + +**AC-7**: OpenAPI spec marks all required fields at every nesting level, declares ranges, and documents the 400 response. + +**AC-8**: Manual probe script exercises each failure mode end-to-end via `curl` + JWT. + +**AC-9** (advisory — surface in PR, parent-suite to decide): the inconsistency `RequestRegionRequest.sizeMeters` vs `CreateRouteRequest.regionSizeMeters` is named differently for the same concept. Either keep the discrepancy and document why, or harmonize to a single name in a follow-up MAJOR contract bump for both. + +**AC-10** (advisory — surface in PR, parent-suite to decide): the **input/output point-naming asymmetry** on this endpoint (input `points: [{"lat":..,"lon":..}]`, response `points: [{"latitude":..,"longitude":..}]` for the same `RoutePoint` round-trip) is a DTO inconsistency. Probe-confirmed 2026-05-22. Either keep + document, or file a follow-up to harmonize. + +## Out of scope + +- Route processing semantics (Flow F5 background, ZIP creation, point-in-polygon geofence filtering) — validation lives at the API layer only. +- `GET /api/satellite/route/{id}` status endpoint (separate task if needed; Guid binding is framework-handled). +- Performance — nested validation overhead is negligible vs interpolation + background region enqueue. +- Route interpolation algorithm — unchanged. +- Input/output point-naming asymmetry fix — surfaced as AC-10 advisory only. + +## Constraints + +- **Breaking behavior change** — callers today omitting `id` (silently getting zero-Guid) or sending malformed nested bodies will start getting 400. Known consumer set: gps-denied-onboard (uses correct body shape with id and lat/lon points, per black-box probe 2026-05-22). Other consumers TBD by parent-suite team. +- No regression in any existing `RouteCreationTests.cs` happy-path coverage. +- Cross-field constraint (rule 12) requires custom `When/Otherwise` or a top-level `Must()` rule — FluentValidation 12.0.0 supports both; pick the more readable one. + +## References + +- Jira AZ-809: https://denyspopov.atlassian.net/browse/AZ-809 +- Parent Epic: AZ-795 +- Reference implementations: AZ-796 (single-DTO pattern), AZ-808 (no-prior-contract pattern, same batch) +- Tangentially related: AZ-812 (region field rename to OSM) +- Cycle-7 retro: `_docs/06_metrics/retro_2026-05-22_cycle7.md` (flagged this endpoint as a per-endpoint child of AZ-795) +- Originating consumer discovery: gps-denied-onboard AZ-777 Phase 2 (route-based seeding is the consumer's preferred path; 2026-05-22 black-box probe surfaced silent-coercion + naming asymmetry) +- Related contract docs: `error-shape.md` v1.0.0, `tile-inventory.md` v2.0.0 (both produced by AZ-795+AZ-796 cycle 7) diff --git a/_docs/02_tasks/todo/AZ-810_upload_metadata_validation.md b/_docs/02_tasks/todo/AZ-810_upload_metadata_validation.md new file mode 100644 index 0000000..911cfe5 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-810_upload_metadata_validation.md @@ -0,0 +1,147 @@ +# Strict validation for UAV upload metadata (POST /api/satellite/upload) + +**Task**: AZ-810_upload_metadata_validation +**Name**: Strict validation for UAV upload metadata +**Description**: Add FluentValidation-backed strict input validation to the metadata DTO layer of `POST /api/satellite/upload` (UAV batch upload, AZ-488). Reject malformed metadata JSON envelopes with RFC 7807 ValidationProblemDetails (HTTP 400). Fourth concrete child of AZ-795; reuses the shared infra wired in cycle 7. The file-level quality checks (size, luminance, age, future-skew) remain in scope of the existing `IUavTileQualityGate`. +**Complexity**: 5 points (multipart envelope requires custom filter, 14 rules, two validator classes, MINOR contract bump, defense-in-depth with existing UavTileQualityGate) +**Dependencies**: AZ-795 (HARD — shared infra); AZ-796 (single-DTO reference); AZ-809 (nested per-item reference); AZ-488 (original endpoint — must remain green); AZ-503 (flightId semantics) +**Component**: SatelliteProvider.Api/Validators + SatelliteProvider.Common (UavTileBatchMetadataPayload, UavTileMetadata DTOs) +**Tracker**: AZ-810 (https://denyspopov.atlassian.net/browse/AZ-810) +**Epic**: AZ-795 — Strict input validation across all public endpoints +**Originating ticket**: AZ-795 cycle-7 retro (explicitly names this endpoint as a remaining per-endpoint child) + +## Scope + +Add FluentValidation-backed strict input validation to the **metadata DTO** layer of `POST /api/satellite/upload`. Reject malformed `metadata` JSON envelopes with **RFC 7807 ValidationProblemDetails** (HTTP 400) per the Epic's `error-shape.md` v1.0.0 contract. + +**Important scope boundary**: this task is about the **metadata envelope** — `UavTileBatchMetadataPayload` and its per-item `UavTileMetadata` payloads. The **file-level** quality checks (size, luminance variance, age, future-skew) are already enforced by the existing `IUavTileQualityGate` per AZ-488 and remain in scope of that gate. The DTO validator runs **before** the quality gate (per-item bytes inspection) so malformed metadata can short-circuit without ever touching the file bytes. + +Originating discovery: AZ-795 cycle-7 retro — the metadata DTO is explicitly named as a remaining gap ("already partly validated by `UavTileQualityGate`, but the metadata layer is a separate validator"). + +Jira AZ-810 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Endpoint surface + +`POST /api/satellite/upload` (multipart/form-data, auth: `RequiresGpsPermission` policy on top of JWT bearer) + +Multipart envelope: +- `metadata` form field — JSON string deserialized to `UavTileBatchMetadataPayload`. +- `files` form field — `IFormFileCollection`, one entry per metadata item, position-correlated. + +`UavTileBatchMetadataPayload` (current shape, per `modules/common_dtos.md`): + +```json +{ + "items": [ + { + "lat": 50.10, + "lon": 36.10, + "tileZoom": 18, + "tileSizeMeters": 19.10925707, + "capturedAt": "2026-05-22T08:00:00Z", + "flightId": "a1b2c3d4-..." + } + ] +} +``` + +Response (current per AZ-488): HTTP 200 `{items: [UavTileUploadResultItem[]]}` even on per-item failures. Envelope-level failures (oversize batch, malformed metadata, mismatched batch) are HTTP 400 ProblemDetails. **This task tightens the "malformed metadata" path.** + +## Required validations + +### Envelope-level + +1. **Multipart envelope present** — missing multipart form → framework-level 400 (unchanged). +2. **`metadata` field present** — missing form field → 400 with `errors.metadata` ("required"). +3. **`metadata` parses as JSON** — malformed JSON → 400 with `errors.metadata` ("could not be parsed as JSON"). Covered by AZ-795's `GlobalExceptionHandler` once metadata binding routes through `JsonSerializerOptions`. +4. **`metadata.items` required, non-empty** — missing or `[]` → 400 with `errors.metadata.items`. +5. **`metadata.items.length` ≤ `UavQualityConfig.MaxBatchSize`** — over cap → 400 with `errors.metadata.items`. (Existing framework limit handles oversize via `KestrelServerOptions.Limits.MaxRequestBodySize` at the byte layer; this rule guards the item count specifically.) +6. **`metadata.items.length` == `files.length`** — envelope alignment per AZ-488. Already detected by the upload handler; surface via ValidationProblemDetails for consistency with sibling endpoints → 400 with `errors.metadata` + `errors.files`. + +### Per-item (under `metadata.items[i]`) + +7. **`lat` required** — double, in `[-90.0, 90.0]`. Missing/out-of-range → 400 with `errors.metadata.items[i].lat`. +8. **`lon` required** — double, in `[-180.0, 180.0]`. Missing/out-of-range → 400 with `errors.metadata.items[i].lon`. +9. **`tileZoom` required** — int, in `[0, 22]` (align with `TileCoordValidator`). Missing/out-of-range → 400 with `errors.metadata.items[i].tileZoom`. +10. **`tileSizeMeters` required** — double, `> 0.0`. Missing/non-positive → 400 with `errors.metadata.items[i].tileSizeMeters`. (Tighter range can be added if parent-suite team has a documented expected range; for now just guard `> 0`.) +11. **`capturedAt` required** — ISO-8601 UTC `DateTime`. Must satisfy AZ-488 Rule 4 freshness window: `capturedAt ≤ now + UavQualityConfig.CapturedAtFutureSkewSeconds` AND `capturedAt ≥ now - UavQualityConfig.MaxAgeDays`. Missing/out-of-window → 400 with `errors.metadata.items[i].capturedAt`. (Equivalent to AZ-488 Rule 4 but at the API layer; the existing UavTileQualityGate still enforces the same rule for defense-in-depth.) +12. **`flightId` optional** — if present, must be valid `Guid` (RFC 4122). Malformed UUID → 400 with `errors.metadata.items[i].flightId`. (Null/missing is valid — anonymous-flight semantics per AZ-503.) + +### Cross-cutting + +13. **Unknown fields rejected at root or any nesting level of `metadata`** — covered by AZ-795's `UnmappedMemberHandling.Disallow`. Any unknown field at root or under `items[i]` → 400 with `errors.metadata.` ("could not be mapped to any .NET member"). +14. **Type mismatch** — e.g. `"lat": "fifty"` or `"tileZoom": 18.5` (non-integer double for int) → 400 with `errors.metadata.`. Covered by AZ-795's `GlobalExceptionHandler`. + +## Implementation pattern (mirror AZ-796, extended for multipart + per-item) + +1. New files (all under `SatelliteProvider.Api/Validators/`): + - `UavTileBatchMetadataPayloadValidator.cs` — root validator with rules 4–6. + - `UavTileMetadataValidator.cs` — per-item validator (rules 7–12); invoked via `RuleForEach(x => x.Items).SetValidator(new UavTileMetadataValidator(uavQualityConfig))`. +2. Mark required props on `UavTileBatchMetadataPayload` + `UavTileMetadata` with `[JsonRequired]` per the cycle-7 `TileCoord` pattern. +3. Wire the validator into the multipart handler in `Program.cs` (the `UploadUavTileBatch` endpoint) — likely a custom endpoint filter that: + a. Reads the `metadata` form field. + b. Deserializes via the strict `JsonSerializerOptions` (already configured by AZ-795). + c. Resolves `IValidator` from DI and runs it. + d. Returns `Results.ValidationProblem` on failure. + + This is a more involved wiring than AZ-796 (which uses the bog-standard `.WithValidation()` filter for pure JSON bodies). Document the new filter in `_docs/02_document/modules/api_program.md`. +4. Unit tests: `SatelliteProvider.Tests/Validators/UavTileBatchMetadataPayloadValidatorTests.cs` + `UavTileMetadataValidatorTests.cs` (≥ 11 test methods total — one per `RuleFor`/`RuleForEach` chain). +5. Integration tests: `SatelliteProvider.IntegrationTests/UavUploadValidationTests.cs` (new file) — ≥ 13 methods (1 happy + 1 per failure-mode AC + envelope alignment regression). +6. Manual probe: `scripts/probe_upload_validation.sh` — multipart `curl` against each failure mode. + +## Update existing contract doc + +Bump `_docs/02_document/contracts/api/uav-tile-upload.md` from v1.1.0 → v1.2.0 (MINOR). The contract doc exists; this task adds the validation rules + error shape reference. Do NOT change the wire format (no rename like AZ-794); MINOR is correct. + +Add a new section: "Validation rules (AZ-810)" that enumerates the 14 rules and references `error-shape.md` v1.0.0. + +## Coordination with sibling tickets + +- **Parent (AZ-795)**: depends on shared infra already landed in cycle 7. +- **AZ-796 (inventory)**: reference for single-DTO pattern. +- **AZ-809 (route)**: reference for nested per-item validator pattern (RuleForEach). +- **AZ-488** (original UAV upload): existing happy-path integration tests + `UavTileQualityGate` MUST remain green. +- **AZ-503** (flightId semantics): rule 12 must respect the anonymous-flight contract — `flightId=null` is a valid case. + +## Acceptance criteria + +**AC-1**: Each of the 14 validations above rejects with HTTP 400 + ValidationProblemDetails (single-rule precision). + +**AC-2**: Happy path unchanged — valid envelope still returns HTTP 200 + per-item result list; per-item file rejections (existing `UavTileQualityGate` semantics) still return HTTP 200 with per-item status (unchanged contract). + +**AC-3**: Both validator classes live in their own files under `SatelliteProvider.Api/Validators/` and are unit-tested (≥ 11 methods total). + +**AC-4**: `SatelliteProvider.IntegrationTests/UavUploadValidationTests.cs` covers happy + 12+ failure modes with full ValidationProblemDetails assertion. + +**AC-5**: `_docs/02_document/contracts/api/uav-tile-upload.md` bumped to v1.2.0 (MINOR) with the new validation section. + +**AC-6**: `_docs/02_document/modules/api_program.md` updated to document the new multipart-validation endpoint filter. + +**AC-7**: OpenAPI spec marks `UavTileBatchMetadataPayload` + `UavTileMetadata` fields `required`, declares ranges, and documents the 400 response. + +**AC-8**: Manual probe script exercises each failure mode end-to-end via multipart `curl` + JWT. + +**AC-9**: No regression in any existing AZ-488 integration tests (`UavTileBatchUploadTests.cs`, `UavTileQualityGateTests.cs`). + +## Out of scope + +- File-level quality checks (size, luminance, age, future-skew) — already enforced by `IUavTileQualityGate` per AZ-488; do NOT duplicate at the validator layer (the validator covers metadata-only). +- Per-item file-byte validation — unchanged. +- Auth (`RequiresGpsPermission`) — unchanged. +- Performance — metadata validation overhead is negligible vs the per-item file decode + DB writes. + +## Constraints + +- **Breaking behavior change** — callers sending malformed metadata that silently coerces will start getting 400 instead of HTTP 200 with per-item rejections. Known consumer set: gps-denied-onboard (D-PROJ-2 flight-uploader path — not currently active per AZ-777 task spec). +- No regression in any existing `UavTileBatchUploadTests.cs` happy-path coverage. +- Cross-field rule 6 (alignment) requires access to BOTH `metadata.Items.Count` AND `files.Count` — it can't be a pure `IValidator` rule. Wire it as a separate envelope-level check inside the endpoint filter, with the same ValidationProblemDetails shape. +- The multipart validation filter (item 3 of Implementation pattern above) is a NEW shared piece of infra. Consider whether it should live as a generic `MultipartValidationEndpointFilter` for future reuse, or stay specific to this endpoint. Parent-suite team decides. + +## References + +- Jira AZ-810: https://denyspopov.atlassian.net/browse/AZ-810 +- Parent Epic: AZ-795 +- Reference implementations: AZ-796 (single-DTO pattern), AZ-809 (nested per-item pattern, same batch) +- Cycle-7 retro: `_docs/06_metrics/retro_2026-05-22_cycle7.md` (explicitly names this endpoint as a per-endpoint child of AZ-795) +- Original endpoint: AZ-488 (UAV batch upload), AZ-503 (flightId semantics) +- Related contract docs: `error-shape.md` v1.0.0, `tile-inventory.md` v2.0.0 (both produced by AZ-795+AZ-796 cycle 7), `uav-tile-upload.md` v1.1.0 (to be bumped) diff --git a/_docs/02_tasks/todo/AZ-811_latlon_get_endpoint_validation.md b/_docs/02_tasks/todo/AZ-811_latlon_get_endpoint_validation.md new file mode 100644 index 0000000..c883f70 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-811_latlon_get_endpoint_validation.md @@ -0,0 +1,105 @@ +# Strict validation for lat/lon tile GET endpoint (GET /api/satellite/tiles/latlon) + +**Task**: AZ-811_latlon_get_endpoint_validation +**Name**: Strict validation for lat/lon tile GET endpoint +**Description**: Add FluentValidation-backed strict input validation to `GET /api/satellite/tiles/latlon` (single-tile download by lat/lon/zoom). Reject malformed query strings with RFC 7807 ValidationProblemDetails (HTTP 400). Fifth concrete child of AZ-795; query-string surface differs from sibling JSON-body endpoints — needs explicit unknown-query-param filter. +**Complexity**: 2 points (simple endpoint, 3 typed params + unknown-param check, reuses cycle-7 shared infra, small new contract doc) +**Dependencies**: AZ-795 (HARD — shared infra); AZ-796 (single-DTO reference); AZ-808 (no-prior-contract reference) +**Component**: SatelliteProvider.Api/Validators + small new endpoint filter (RejectUnknownQueryParamsEndpointFilter) +**Tracker**: AZ-811 (https://denyspopov.atlassian.net/browse/AZ-811) +**Epic**: AZ-795 — Strict input validation across all public endpoints +**Originating ticket**: AZ-795 cycle-7 retro (explicitly names this endpoint as a remaining per-endpoint child) + +## Scope + +Add FluentValidation-backed strict input validation to `GET /api/satellite/tiles/latlon`. Reject malformed query strings with **RFC 7807 ValidationProblemDetails** (HTTP 400) per the Epic's `error-shape.md` v1.0.0 contract. + +Differs from siblings (AZ-796 / AZ-808 / AZ-809 / AZ-810) in that the input surface is **query string**, not a JSON body, so the unknown-field rejection knob (`UnmappedMemberHandling.Disallow`) does not apply directly — query-param-strictness needs an explicit shape check. + +Originating discovery: AZ-795 cycle-7 retro — this endpoint is explicitly named as a remaining gap alongside the POST endpoints. + +Jira AZ-811 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Endpoint surface + +`GET /api/satellite/tiles/latlon?lat=&lon=&zoom=` (auth: JWT bearer required, no permission claim). + +Response (current per `api_program.md::GetTileByLatLon Handler`): HTTP 200 with `DownloadTileResponse` (tile metadata; the actual bytes are served separately via `GET /tiles/{z}/{x}/{y}`). + +Current behavior on bad input: query params bind via the framework's default model binder — missing/malformed params trigger a generic 400 or silent defaults, neither of which conforms to `error-shape.md` v1.0.0. + +## Required validations + +1. **`lat` query param required** — double, in `[-90.0, 90.0]`. Missing/out-of-range/malformed → 400 with `errors.lat`. +2. **`lon` query param required** — double, in `[-180.0, 180.0]`. Missing/out-of-range/malformed → 400 with `errors.lon`. +3. **`zoom` query param required** — int, in `[0, 22]` (align with `TileCoordValidator`). Missing/out-of-range/malformed → 400 with `errors.zoom`. +4. **Unknown query parameters rejected** — any query string param outside `{lat, lon, zoom}` → 400 with `errors.`. (Requires explicit query-param-shape check inside the endpoint filter — the framework's default binder silently ignores extras.) +5. **Type mismatch** — e.g. `lat=fifty` (not parseable as double) → 400 with `errors.lat` ("could not be parsed"). Covered by AZ-795's `GlobalExceptionHandler` IF the binding throws — verify this code path triggers it (it does for `[FromBody]` deserializers; query-string parse failures may take a different path — surface in PR and adapt). + +## Implementation pattern (adapted for query string) + +1. Bind query params to a dedicated record: `record GetTileByLatLonQuery(double Lat, double Lon, int Zoom)`. Default `[AsParameters]` binding works; `[JsonRequired]` doesn't apply (no JSON deserializer in the path), so missing-required is detected by the validator only. +2. New file: `SatelliteProvider.Api/Validators/GetTileByLatLonQueryValidator.cs` — `AbstractValidator` with rules 1–3. +3. Add `.WithValidation()` to the `MapGet("/api/satellite/tiles/latlon", ...)` chain. May require a small variant of `ValidationEndpointFilter` that runs against the bound query-record rather than the body-bound record — the cycle-7 generic filter already does the bound-argument lookup, so it should Just Work; verify. +4. **Rule 4 (unknown query params)** is the novel piece: implement as a separate endpoint filter that inspects `HttpContext.Request.Query.Keys` against the allowed set `{"lat", "lon", "zoom"}`. On any extras → `Results.ValidationProblem` with one `errors` entry per unexpected key. Either: + - Standalone filter `RejectUnknownQueryParamsEndpointFilter` (parameterized by allowed keys; reusable across future query-param endpoints). + - Inline `Func` for now and extract when the second consumer arrives. Parent-suite team decides. +5. Unit tests: `SatelliteProvider.Tests/Validators/GetTileByLatLonQueryValidatorTests.cs` (≥ 3 methods — one per RuleFor). Plus a test for the unknown-query-param filter (≥ 1 method). +6. Integration tests: `SatelliteProvider.IntegrationTests/GetTileByLatLonValidationTests.cs` (new file) — ≥ 6 methods (1 happy + 1 per failure-mode AC + 1 unknown-query-param). +7. Manual probe: `scripts/probe_latlon_validation.sh` — `curl` against each failure mode. + +## New contract doc + +Create `_docs/02_document/contracts/api/tile-latlon.md` v1.0.0. This endpoint has **no formal contract** today; the producer-doc surface is `modules/api_program.md::GetTileByLatLon Handler` only. Cover: + +- Endpoint, auth, query params, response body (`DownloadTileResponse`), error shape (reference `error-shape.md` v1.0.0). +- Invariants (single tile per request; (lat, lon, zoom) deterministically maps to a (z, x, y) coord; output references the slippy-map URL `/tiles/{z}/{x}/{y}` for body fetch). +- Test cases table mirroring validator rules. +- Cross-link to `tile-inventory.md` v2.0.0 (related single-vs-bulk read patterns) + `GET /tiles/{z}/{x}/{y}` URL contract. + +## Coordination with sibling tickets + +- **Parent (AZ-795)**: depends on shared infra already landed in cycle 7. +- **AZ-796 (inventory)**: reference for `[FromBody]` validator pattern. +- **AZ-808 (region)**: reference for endpoint without prior contract doc. +- **AZ-777 (gps-denied-onboard)**: not currently a consumer (the onboard side uses `GET /tiles/{z}/{x}/{y}` directly with pre-computed coords from inventory); but this endpoint is needed for future workflows (e.g. UI-driven single-tile fetch by user-clicked coordinates). + +## Acceptance criteria + +**AC-1**: Each of the 5 validations above rejects with HTTP 400 + ValidationProblemDetails (single-rule precision). + +**AC-2**: Happy path unchanged — a valid `?lat=&lon=&zoom=` still returns HTTP 200 + `DownloadTileResponse`; tile is still downloaded/persisted as before. + +**AC-3**: `GetTileByLatLonQueryValidator` lives in its own file under `SatelliteProvider.Api/Validators/` and is unit-tested (≥ 3 methods). + +**AC-4**: `SatelliteProvider.IntegrationTests/GetTileByLatLonValidationTests.cs` covers happy + 4+ failure modes with full ValidationProblemDetails assertion. + +**AC-5**: `_docs/02_document/contracts/api/tile-latlon.md` v1.0.0 created and published. + +**AC-6**: `_docs/02_document/modules/api_program.md::GetTileByLatLon Handler` updated to reference the validator + new contract doc. + +**AC-7**: OpenAPI spec marks the query params as required + ranges + 400 response. + +**AC-8**: Manual probe script exercises each failure mode end-to-end via `curl` + JWT. + +**AC-9**: The novel unknown-query-param rejection filter (item 4 of Implementation pattern) is documented in `_docs/02_document/modules/api_program.md` so the next query-param endpoint can reuse it cleanly. + +## Out of scope + +- The actual tile download / persistence semantics — unchanged. +- `GET /tiles/{z}/{x}/{y}` path-parameter validation (separate concern; the path int binder rejects malformed values at the framework layer, but range-checking `z` and `x`/`y` bounds is a gap that may warrant a separate task if parent-suite team decides). +- Performance — query-string validation overhead is negligible vs the conditional Google-Maps round-trip. + +## Constraints + +- **Breaking behavior change** — callers sending unknown extra query params (e.g. typo `?latitude=`) that today silently fall back to `lat=0` will start getting 400. Known consumer set: TBD by parent-suite team (gps-denied-onboard does NOT currently call this endpoint). +- No regression in any existing `TileByLatLonTests.cs` happy-path coverage. +- The unknown-query-param rejection (rule 4) is a NEW behavior on top of standard ASP.NET binding; document it loudly in the contract doc so consumers know. + +## References + +- Jira AZ-811: https://denyspopov.atlassian.net/browse/AZ-811 +- Parent Epic: AZ-795 +- Reference implementations: AZ-796 (single-DTO pattern), AZ-808 (no-prior-contract pattern, same batch) +- Cycle-7 retro: `_docs/06_metrics/retro_2026-05-22_cycle7.md` (flagged this endpoint as a per-endpoint child of AZ-795) +- Related contract docs: `error-shape.md` v1.0.0, `tile-inventory.md` v2.0.0 (both produced by AZ-795+AZ-796 cycle 7) diff --git a/_docs/02_tasks/todo/AZ-812_region_field_rename_to_osm.md b/_docs/02_tasks/todo/AZ-812_region_field_rename_to_osm.md new file mode 100644 index 0000000..5a203e0 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-812_region_field_rename_to_osm.md @@ -0,0 +1,117 @@ +# Region API: rename Latitude/Longitude → Lat/Lon (OSM convention) + +**Task**: AZ-812_region_field_rename_to_osm +**Name**: Rename `RequestRegionRequest.{Latitude, Longitude}` → `{Lat, Lon}` for OSM consistency +**Description**: Rename the JSON wire-format fields on `RequestRegionRequest` from verbose `latitude`/`longitude` to OSM-standard short `lat`/`lon`. Mirror of AZ-794 (which did the same for the inventory endpoint's `tileZoom/tileX/tileY` → `z/x/y`). Breaking wire-format change. +**Complexity**: 3 points (same scope as AZ-794: DTO rename + downstream code + docs + manual probe; no new behavior) +**Dependencies**: — (coordinate ordering with AZ-808 — see *Coordination*) +**Component**: SatelliteProvider.Common (RequestRegionRequest DTO) + SatelliteProvider.Services (RegionService consumers) + SatelliteProvider.IntegrationTests + producer docs +**Tracker**: AZ-812 (https://denyspopov.atlassian.net/browse/AZ-812) +**Originating ticket**: gps-denied-onboard AZ-777 Phase 2 (cross-repo, 2026-05-22) — black-box probe revealed Region is the lone hold-out using verbose `latitude`/`longitude` while every other coord field across the API uses OSM-standard `lat`/`lon` / `z/x/y` + +## Scope + +Rename the JSON wire-format fields on `RequestRegionRequest` from verbose `latitude`/`longitude` to OSM-standard short `lat`/`lon`. Mirror of **AZ-794** (which did the same for the inventory endpoint's `tileZoom/tileX/tileY` → `z/x/y`). This is a **breaking wire-format change**. + +Originating discovery: gps-denied-onboard AZ-777 Phase 2 black-box probe (2026-05-22). The consumer probed `POST /api/satellite/request` with `{"lat":49.94,"lon":36.31,...}` (OSM convention, matching the slippy-map URL `/tiles/{z}/{x}/{y}` and the Route endpoint's `RoutePoint`/`GeoPoint` DTOs which already use `lat`/`lon`). The producer rejected with HTTP 400 — the Region endpoint is the lone hold-out using verbose `latitude`/`longitude`. + +Jira AZ-812 is the authoritative spec; this file mirrors the in-workspace-only sections. + +## Why this matters + +Current state — satellite-provider's coord-naming surface is **internally inconsistent**: + +| Endpoint / DTO | Field names | Source | +|---|---|---| +| `GET /tiles/{z}/{x}/{y}` | `z`, `x`, `y` | URL path — OSM slippy-map standard | +| `POST /api/satellite/tiles/inventory` body | `z`, `x`, `y` | AZ-794 (cycle 7) | +| `POST /api/satellite/route` → `RoutePoint` | `lat`, `lon` | `[JsonPropertyName("lat")]` already in DTO | +| `POST /api/satellite/route` → `GeoPoint` | `lat`, `lon` | `[JsonPropertyName("lat")]` already in DTO | +| `POST /api/satellite/request` body | `latitude`, `longitude` | **← the outlier this ticket fixes** | + +After this rename, every coord field in every satellite-provider request body uses the OSM short form. Consumers can rely on one naming convention end-to-end. + +A secondary issue surfaced by the same probe — the Route endpoint's **response** echoes points as `latitude`/`longitude` even though the request shape uses `lat`/`lon` (input/output asymmetry on the same DTO round-trip). This task **does not** fix that (it's the Route DTO's response shape, not the Region request). Surfaced as AZ-809 AC-10 advisory for a separate follow-up if parent-suite team confirms it's a bug. + +## Endpoint surface + +`POST /api/satellite/request` + +Before (current): + +```json +{ + "id": "", + "latitude": 49.94, + "longitude": 36.31, + "sizeMeters": 200, + "zoomLevel": 18, + "stitchTiles": false +} +``` + +After: + +```json +{ + "id": "", + "lat": 49.94, + "lon": 36.31, + "sizeMeters": 200, + "zoomLevel": 18, + "stitchTiles": false +} +``` + +## Implementation + +1. Modify `SatelliteProvider.Common/DTO/RequestRegionRequest.cs`: + - Rename C# properties: `Latitude` → `Lat`, `Longitude` → `Lon`. + - Add `[JsonPropertyName("lat")]` / `[JsonPropertyName("lon")]` so the wire format is unambiguous even if anyone later reads the camelCase defaults. +2. Find all references via `git grep -w 'Latitude\|Longitude' SatelliteProvider.*/` — update C# usages in: + - `SatelliteProvider.Services/RegionService.cs` (or wherever the handler unpacks the DTO). + - `SatelliteProvider.IntegrationTests/RegionTests.cs` + `SatelliteProvider.IntegrationTests/Models.cs`. + - Any other test fixtures / mocks. +3. Update the OpenAPI spec snapshot test (if one exists). +4. Update producer documentation: + - `_docs/02_document/modules/common_dtos.md::RegionRequest` — update field-name listing. + - `_docs/02_document/modules/api_program.md::RequestRegion Handler` — update example body. + - `_docs/02_document/system-flows.md::F2 Region Request Flow` — update example body. +5. The new `_docs/02_document/contracts/api/region-request.md` (to be created by AZ-808) MUST use the post-rename field names. Coordinate with AZ-808 implementer: if AZ-808 lands first, the contract starts at v1.0.0 with `latitude/longitude`, then this task bumps to v2.0.0 with `lat/lon`. If this task lands first, AZ-808's contract starts at v1.0.0 with `lat/lon` directly. + +## Acceptance criteria + +**AC-1**: `RequestRegionRequest` DTO uses `Lat` / `Lon` (C#) + `[JsonPropertyName("lat")]` / `[JsonPropertyName("lon")]`. + +**AC-2**: Wire format is `{"lat":..,"lon":..}` end-to-end (request body, OpenAPI schema, docs, all integration tests). + +**AC-3**: `RegionTests.cs` happy-path tests pass against the new wire format. + +**AC-4**: Manual `curl` probe with `{"id":"","lat":49.94,"lon":36.31,"sizeMeters":200,"zoomLevel":18,"stitchTiles":false}` returns HTTP 200 + valid regionId; old `{"latitude":..,"longitude":..}` returns HTTP 400 with `UnmappedMemberHandling.Disallow` rejecting the unknown fields. + +**AC-5**: Docs updated: `common_dtos.md`, `api_program.md`, `system-flows.md` (F2). + +**AC-6**: If `region-request.md` contract doc exists at the time this task lands (AZ-808 already shipped), bump v1.0.0 → v2.0.0 with a change-log entry naming AZ-812. If `region-request.md` does NOT yet exist (AZ-808 still in flight), coordinate so AZ-808 publishes v1.0.0 directly with the new names — then this task only needs to land the code + non-contract docs. + +## Coordination with sibling tickets + +- **AZ-794** (inventory rename): same pattern, same justification. Recommended to follow the same hard-switch rollout strategy AZ-794 used. +- **AZ-808** (region validation): hard coordination point. Pick the ordering during planning — either ship this first so AZ-808 writes validators against the final names, or ship together as a coordinated release. +- **AZ-777 Phase 2** (gps-denied-onboard consumer): the consumer adapter for Region API will be written against the final names — prefer this ticket lands first or co-ships with AZ-808 so the consumer doesn't have to migrate twice. +- **Follow-up (not in scope)**: the Route endpoint's input/output point-shape asymmetry (input `lat`/`lon`, output `latitude`/`longitude`). Tracked as AZ-809 AC-10 advisory; file separately if parent-suite team confirms. + +## Constraints + +- **Breaking wire-format change** — same risk profile as AZ-794. Known consumer set: gps-denied-onboard (AZ-777 Phase 2 — will adapt before first integration). Other consumers TBD. +- Coordinate with AZ-808 to avoid validator code being written against the wrong names. +- No regression in `RegionTests.cs` happy-path coverage. + +## References + +- Jira AZ-812: https://denyspopov.atlassian.net/browse/AZ-812 +- Mirror of: AZ-794 (inventory body-field rename) +- Hard coordination with: AZ-808 (region validator) +- Parent epic context: AZ-795 (validation epic provides the `UnmappedMemberHandling.Disallow` infra that makes this rename safely diagnosable on the consumer side) +- Originating probe: gps-denied-onboard AZ-777 Phase 2 black-box probe of Region API (2026-05-22) +- Current DTO: `SatelliteProvider.Common/DTO/RequestRegionRequest.cs` +- Sibling DTOs already using lat/lon: `SatelliteProvider.Common/DTO/RoutePoint.cs`, `SatelliteProvider.Common/DTO/GeoPoint.cs`