mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-22 10:11:15 +00:00
[AZ-350] Archive previously-completed task files todo -> done
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
@@ -1,69 +0,0 @@
|
||||
# Refactor: idempotent POST contract for caller-supplied GUIDs
|
||||
|
||||
**Task**: AZ-362_refactor_idempotent_post_contract
|
||||
**Name**: Return existing resource on duplicate POST instead of 500
|
||||
**Description**: On `INSERT` conflict for a known caller-supplied `Id` in `/api/satellite/request` and `/api/satellite/route`, return the existing resource (200 OK) instead of bubbling a 500.
|
||||
**Complexity**: 3 points
|
||||
**Dependencies**: AZ-353 (C03) — so the new path doesn't traverse the leaky 500 handler.
|
||||
**Component**: Api + Services.RegionProcessing + Services.RouteManagement
|
||||
**Tracker**: AZ-362
|
||||
**Epic**: AZ-350
|
||||
|
||||
## Problem
|
||||
|
||||
`SatelliteProvider.Api/Program.cs:187-211` (`RequestRegion`) and `Program.cs:233-250` (`CreateRoute`) accept `request.Id` from the caller and `INSERT` blindly. A retried POST hits a unique-key conflict at the DB and surfaces as 500 with a leaky message (pre-C03). The client cannot determine whether their first POST succeeded.
|
||||
|
||||
## Outcome
|
||||
|
||||
- Two consecutive POSTs with the same `Id` to either endpoint return 200 OK with the existing resource state.
|
||||
- No duplicate background processing is triggered for retried POSTs.
|
||||
- OpenAPI documents the idempotency contract.
|
||||
- 37 unit + 5 smoke tests stay green.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- In `RegionService` (and `RouteService`), detect the unique-key violation on insert; if present, fetch and return the existing row instead of throwing.
|
||||
- Update the API handlers to translate the "existing resource" outcome to 200 OK.
|
||||
- Update OpenAPI / Swagger annotations to document the idempotency.
|
||||
- Add a unit/integration test that POSTs the same `Id` twice and asserts on 200 OK both times.
|
||||
|
||||
### Excluded
|
||||
- Changing the `Id` source (caller-supplied stays).
|
||||
- Adding a separate idempotency-key header.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Region POST is idempotent**
|
||||
Given a `POST /api/satellite/request` that creates a region with `Id = X`
|
||||
When the same payload is POSTed again
|
||||
Then both calls return 200 OK with the same region resource and only one background processing job is queued.
|
||||
|
||||
**AC-2: Route POST is idempotent**
|
||||
Given a `POST /api/satellite/route` that creates a route with `Id = X`
|
||||
When the same payload is POSTed again
|
||||
Then both calls return 200 OK with the same route resource.
|
||||
|
||||
**AC-3: OpenAPI documents the contract**
|
||||
Given the generated `swagger.json`
|
||||
When inspected
|
||||
Then the two endpoints describe the idempotency behavior (200 on duplicate `Id`).
|
||||
|
||||
**AC-4: Tests stay green**
|
||||
Given the post-refactor build
|
||||
When `scripts/run-tests.sh --smoke` runs
|
||||
Then all 37 unit + 5 smoke scenarios pass.
|
||||
|
||||
## Constraints
|
||||
|
||||
- HTTP shape preserved on success (200 OK with the same resource body).
|
||||
- 500-on-duplicate becomes 200-on-duplicate (a strict improvement).
|
||||
- No DB schema change.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: differing payloads with the same `Id`**
|
||||
- *Risk*: a caller may POST the same `Id` with a *different* body. We must define behavior.
|
||||
- *Mitigation*: for this task, return the existing resource (treating the second POST as a retry). A future ticket can add 409-on-mismatch detection.
|
||||
|
||||
Full change entry: `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C09).
|
||||
@@ -1,61 +0,0 @@
|
||||
# Refactor: consolidate Haversine + tile-coord parsing into Common/Utils
|
||||
|
||||
**Task**: AZ-366_refactor_consolidate_haversine_parser
|
||||
**Name**: Single Haversine + co-located tile-filename parser
|
||||
**Description**: Delete the duplicate Haversine implementation in `RouteProcessingService` and move `ExtractTileCoordinatesFromFilename` next to `StorageConfig.GetTileFilePath`.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: Common + Services.RouteManagement
|
||||
**Tracker**: AZ-366
|
||||
**Epic**: AZ-350
|
||||
|
||||
## Problem
|
||||
|
||||
`SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs:596-608` re-implements `GeoUtils.CalculateDistance(GeoPoint, GeoPoint)` as `CalculateDistance(lat1, lon1, lat2, lon2)`. Lines 610-630 host `ExtractTileCoordinatesFromFilename`, a parser tied to the `tile_{ts}_{x}_{y}.jpg` pattern that's *generated* by `StorageConfig.GetTileFilePath` in another assembly. Coupling by string convention only.
|
||||
|
||||
## Outcome
|
||||
|
||||
- One Haversine implementation in the codebase (in `GeoUtils`).
|
||||
- The tile-filename writer and parser live in the same module.
|
||||
- 37 unit + 5 smoke tests stay green.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Delete `RouteProcessingService.CalculateDistance(double, double, double, double)`.
|
||||
- Replace its call sites with `GeoUtils.CalculateDistance(GeoPoint a, GeoPoint b)`.
|
||||
- Move `ExtractTileCoordinatesFromFilename` to live next to `StorageConfig.GetTileFilePath` (or onto `StorageConfig` itself as a static method).
|
||||
- Update consumers' `using` directives.
|
||||
|
||||
### Excluded
|
||||
- Changing the filename pattern.
|
||||
- Changing the `GeoUtils.CalculateDistance` algorithm.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: One Haversine**
|
||||
Given the post-refactor source
|
||||
When grepped for `Math.Sin\(.*lat\)` (or any Haversine-like pattern)
|
||||
Then matches are confined to `GeoUtils.cs`.
|
||||
|
||||
**AC-2: Writer and parser co-located**
|
||||
Given the post-refactor source
|
||||
When `StorageConfig.GetTileFilePath` and `ExtractTileCoordinatesFromFilename` are inspected
|
||||
Then they live in the same file or class.
|
||||
|
||||
**AC-3: Tests stay green**
|
||||
Given the post-refactor build
|
||||
When `scripts/run-tests.sh --smoke` runs
|
||||
Then all 37 unit + 5 smoke scenarios pass.
|
||||
|
||||
## Constraints
|
||||
|
||||
- No public API change beyond moving a static method.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: the parser's empty-catch behavior was already changed by C02**
|
||||
- *Risk*: ordering matters — if C13 ships before C02 the empty catch still exists.
|
||||
- *Mitigation*: implement C02 first (already in Phase 1); C13 inherits the cleaned-up parser.
|
||||
|
||||
Full change entry: `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C13).
|
||||
@@ -1,62 +0,0 @@
|
||||
# Refactor: extract shared TileCsvWriter
|
||||
|
||||
**Task**: AZ-368_refactor_extract_tile_csv_writer
|
||||
**Name**: Shared CSV writer for tile lists
|
||||
**Description**: Extract `TileCsvWriter` from `RegionService` and `RouteProcessingService`.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: None
|
||||
**Component**: Common + Services.RegionProcessing + Services.RouteManagement
|
||||
**Tracker**: AZ-368
|
||||
**Epic**: AZ-350
|
||||
|
||||
## Problem
|
||||
|
||||
`SatelliteProvider.Services.RegionProcessing/RegionService.cs:323-334` and `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs:388-404` both write the same CSV header (`latitude,longitude,file_path`) with the same ordering rule (`OrderByDescending(t.Latitude).ThenBy(t.Longitude)`) and the same `F6` numeric format. Two near-identical writers.
|
||||
|
||||
## Outcome
|
||||
|
||||
- One `TileCsvWriter` class in `Common`.
|
||||
- Region and route CSV-writing paths both use it.
|
||||
- Output bytes byte-for-byte identical to pre-refactor.
|
||||
- 37 unit + 5 smoke tests stay green.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Add `TileCsvWriter` with `Task WriteAsync(string path, IEnumerable<TileRecord> tiles, CancellationToken ct)`.
|
||||
- Add a `TileRecord` record (or use an existing minimal DTO).
|
||||
- Replace the duplicate writers in `RegionService` and `RouteProcessingService` (or its post-C11 `RouteCsvWriter` collaborator).
|
||||
- Unit-test the writer.
|
||||
|
||||
### Excluded
|
||||
- Changing the CSV header or column order.
|
||||
- Changing the numeric format (`F6` stays).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Single writer used by both consumers**
|
||||
Given the post-refactor source
|
||||
When grepped for the CSV header `latitude,longitude,file_path`
|
||||
Then matches are confined to `TileCsvWriter`.
|
||||
|
||||
**AC-2: Output bytes identical**
|
||||
Given the existing region/route scenarios
|
||||
When the post-refactor code runs
|
||||
Then the produced CSV files are byte-for-byte identical.
|
||||
|
||||
**AC-3: Tests stay green**
|
||||
Given the post-refactor build
|
||||
When `scripts/run-tests.sh --smoke` runs
|
||||
Then all 37 unit + 5 smoke scenarios pass.
|
||||
|
||||
## Constraints
|
||||
|
||||
- CSV header, column order, numeric format unchanged.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: line-ending drift on Windows**
|
||||
- *Risk*: a refactor may swap `\n` for `\r\n` and break byte-identical comparison.
|
||||
- *Mitigation*: use `StreamWriter` with `NewLine = "\n"` (matching current behavior).
|
||||
|
||||
Full change entry: `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C15).
|
||||
Reference in New Issue
Block a user