mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 09:31:13 +00:00
2393bff1f2
Both POST /api/satellite/request and POST /api/satellite/route accept a caller-supplied id (Guid). Before this change, a retried POST with the same id would either crash with a unique-key violation (regions) or quietly create a divergent row (routes), neither of which matched the documented intent of caller-supplied GUIDs. RegionService.RequestRegionAsync and RouteService.CreateRouteAsync now check for an existing row by id at the top of the method. If one is found, the existing resource is returned with HTTP 200 and the side effects (insert + enqueue + point regeneration + geofence-region queueing) are all skipped. The Information-level log line on the idempotent path makes retries observable. OpenAPI Description metadata documents the contract on both endpoints so client integrators see it in Swagger. Coverage: - 2 new unit tests (one per service) assert that on duplicate id no insert / enqueue / point-generation / region-queueing call is made. - 2 new integration tests (IdempotentPostTests.cs) exercise the contract end-to-end via HTTP, asserting both calls return 200 and CreatedAt matches within 1ms (PostgreSQL truncates TIMESTAMP to microseconds while .NET DateTime keeps 100ns ticks; a real re-insertion would shift CreatedAt by milliseconds at minimum). Note: the check-first pattern leaves a TOCTOU window for concurrent retries. The repository unique key still surfaces the race as a PostgresException which AZ-353 maps to a clean error. Acceptable for realistic sequential-retry patterns; recorded in batch report as a non-blocking observation. Co-authored-by: Cursor <cursoragent@cursor.com>
8.2 KiB
8.2 KiB
Batch 11 Report — Refactor 03 Phase 2 (continued)
Date: 2026-05-10 Epic: AZ-350 (03-code-quality-refactoring) Status: ✅ Complete, pushed
Scope (1 task / 3 SP)
| ID | C-ID | Title | Points | Component |
|---|---|---|---|---|
| AZ-362 | C09 | Idempotent POST contract for caller-supplied GUIDs | 3 | Api + Services.RegionProcessing + Services.RouteManagement |
Single-task batch — focused on contract semantics across two POST endpoints. Depends on AZ-353 (typed exception handling) which was completed in batch 8.
Problem statement
Both POST /api/satellite/request and POST /api/satellite/route accept a caller-supplied id (Guid). Before this batch:
- A retried POST with the same
idwould either crash with a unique-key violation (regions:regions_pkeyconflict on insert) or quietly create a new row (routes: independent insert path with no key check), depending on the endpoint. - Neither behavior matched the documented intent of caller-supplied GUIDs: enable safe client-side retries.
Changes
Production
- MODIFIED
SatelliteProvider.Services.RegionProcessing/RegionService.cs(RequestRegionAsync)- Added an early
_regionRepository.GetByIdAsync(id)check at the top of the method. - If a row exists for the supplied id, returns
MapToStatus(existing)immediately — no second insert, no second enqueue, no background work re-triggered. - Logs
Idempotent region POST: id {RegionId} already exists with status {Status}; returning existing resource without re-enqueueingat Information so retries are observable in logs but don't pollute as warnings.
- Added an early
- MODIFIED
SatelliteProvider.Services.RouteManagement/RouteService.cs(CreateRouteAsync)- Added an early
await GetRouteAsync(request.Id)check at the top of the method. - If a row exists, returns the existing
RouteResponseimmediately — no point regeneration (Haversine work skipped), no geofence regions re-queued viaRequestRegionAsync. - Logs
Idempotent route POST: id {RouteId} already exists; returning existing resourceat Information.
- Added an early
- MODIFIED
SatelliteProvider.Api/Program.cs(OpenAPI metadata)- Added
Descriptionto both POST endpoints describing the idempotency contract —Idempotent (AZ-362): POSTing the same id twice returns the existing region/route resource with HTTP 200 and does not enqueue duplicate background processing.Surfaces in Swagger UI for client integrators.
- Added
Tests
Unit
- MODIFIED
SatelliteProvider.Tests/RegionServiceTests.cs- Added
RequestRegionAsync_DuplicateId_ReturnsExistingResource_NoReQueue_AZ362_AC1— pre-seeds the mock repo with a region having idX, callsRequestRegionAsync(X, ...), asserts the response mirrors the pre-existing row AND that_regionRepository.AddAsync(...)was never invoked AND that the queue'sEnqueueAsyncwas never invoked.
- Added
- MODIFIED
SatelliteProvider.Tests/RouteServiceTests.cs- Added
CreateRouteAsync_DuplicateId_ReturnsExistingRoute_NoReinsertion_AZ362_AC2— pre-seeds the mock repo with a route having idX, callsCreateRouteAsync({Id = X, ...}), asserts the response mirrors the existing row AND that_routeRepository.InsertRouteAsync(...)was never invoked AND that no points were generated AND that_regionService.RequestRegionAsync(...)was never invoked.
- Added
Integration (end-to-end through HTTP)
- NEW
SatelliteProvider.IntegrationTests/IdempotentPostTests.csRegionPost_SameIdTwice_BothReturn200_NoDuplicateProcessing_AZ362_AC1: posts the same payload twice with a fresh Guid, asserts both return HTTP 200 and thatCreatedAtmatches within 1 ms tolerance (sub-millisecond drift comes from PostgreSQL TIMESTAMP truncating to microseconds while .NET DateTime keeps 100ns ticks — a real re-insertion would shift CreatedAt by tens of milliseconds at minimum).RoutePost_SameIdTwice_BothReturn200_NoReinsertion_AZ362_AC2: same shape for routes, additionally assertsTotalPointsmatches between calls (proves no point regeneration ran).
- MODIFIED
SatelliteProvider.IntegrationTests/Program.cs— wiredIdempotentPostTests.RunAll(httpClient)into both smoke and full suites.
Verification
- Unit tests: 71 / 71 passing (was 69 → +2 new AZ-362 tests).
- Integration smoke + full suite: green. Container exits 0. Idempotency tests confirmed against the live API:
- Region: first POST returned
status=queued, createdAt=2026-05-10T21:42:30.2824410Z; retry returnedstatus=processing, createdAt=2026-05-10T21:42:30.2824410(same row — status had advanced because the background worker picked it up between calls, exactly the kind of state evolution the test design needs to tolerate). Server log lineIdempotent region POST: id 2bd9524a-... already exists with status processing; returning existing resource without re-enqueueingconfirms the early-return path fired. - Route: first POST returned
totalPoints=2, createdAt=2026-05-10T21:42:30.2929352Z; retry returnedtotalPoints=2, createdAt=2026-05-10T21:42:30.2929350(same row; no point regeneration). Log lineIdempotent route POST: id f693556d-... already exists; returning existing resourceconfirms the early-return path fired.
- Region: first POST returned
Acceptance criteria coverage
| AC | Evidence |
|---|---|
| AC-1 Region POST with duplicate id returns 200 with the existing resource and does not re-enqueue | Unit RequestRegionAsync_DuplicateId_ReturnsExistingResource_NoReQueue_AZ362_AC1 (asserts mock interactions); integration RegionPost_SameIdTwice_BothReturn200_NoDuplicateProcessing_AZ362_AC1 (asserts HTTP shape + CreatedAt + log line). |
| AC-2 Route POST with duplicate id returns 200 with the existing resource and does not regenerate points or re-queue regions | Unit CreateRouteAsync_DuplicateId_ReturnsExistingRoute_NoReinsertion_AZ362_AC2; integration RoutePost_SameIdTwice_BothReturn200_NoReinsertion_AZ362_AC2. |
| AC-3 OpenAPI documents the idempotency contract for both endpoints | Swagger Description strings on both MapPost(...) registrations. |
| AC-4 Existing 4xx validation paths preserved (non-idempotent failures still surface as 400) | Existing CreateRoute_InvalidPayload_Returns400_AZ353_AC3 integration test still green — the idempotency check sits above validation but only fires on duplicate-id; new POSTs still hit request.Points.Count < 2 etc. as before. |
Behavior notes
- The check-first pattern is a TOCTOU window in theory: two concurrent retries with the same id could both pass the
GetByIdAsynccheck and then race on insert. The repository layer still has the unique key on the primary id, so the loser of the race surfaces aPostgresException— and AZ-353's typed exception handling (added in batch 8) maps this to a 400/500 with ProblemDetails. Acceptable for the realistic retry pattern (sequential retries from a single client). A fully race-free implementation would require an INSERT...ON CONFLICT DO NOTHING + re-read, which is out of scope (would touch the repository contract). Recorded as a non-blocking observation; not a leftover. - For routes, the check uses
GetRouteAsync(request.Id)(the public service method) rather than calling the repository directly. This means the cached read paths and any future caching layer applied toGetRouteAsyncare exercised by the idempotency check too. Same pattern would be reasonable for regions but the existingRequestRegionAsyncalready takes the repository directly so the more targeted call was kept. - The idempotency check happens before validation. A retry of a request that originally succeeded but had bad params (impossible — bad params would have rejected the first request) is a non-issue. A retry of a request with changed params under the same id is treated as idempotent against the first row — clients SHOULD NOT mutate the request body across retries with the same id; this matches the contract documented in OpenAPI.
Up next
- Batch 12: TBD by the next planning step. Remaining Phase 2 work: AZ-360 / AZ-361 / AZ-364..366 plus the remaining Phase 3 (Tooling) and Phase 4 (Cleanup) tasks. ~46 SP / 18 tickets left in the epic.
- K=3 cumulative review next fires after batches 10+11+12 — after batch 12 we trigger another read-only audit covering AZ-357 + AZ-362 + (whatever batch 12 brings).