Commit Graph

8 Commits

Author SHA1 Message Date
Oleksandr Bezdieniezhnykh 6f23120c49 [AZ-364] [AZ-360] Refactor C11+C08: decompose RouteProcessingService
Extracts RouteRegionMatcher, RouteCsvWriter, RouteSummaryWriter,
RouteImageRenderer, TilesZipBuilder, RegionFileCleaner from the
~750-LOC RouteProcessingService god-class. Moves TileInfo to its
own file as a sealed record. Replaces IServiceProvider scope-
locator with a direct IRegionService injection (folds AZ-360 / C08).
Updates DI registration and tests.

Tests: 133 / 133 unit + 5 / 5 smoke green; integration suite exit 0.
Pixel-equivalent stitched route image and byte-equivalent CSV /
summary / ZIP outputs verified through the smoke run.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 03:12:49 +03:00
Oleksandr Bezdieniezhnykh 10d31b4c1c [AZ-367] Refactor C14: extract shared TileGridStitcher
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 02:55:25 +03:00
Oleksandr Bezdieniezhnykh f7ad7aa5ab [AZ-365] Refactor C12: decompose RouteService.CreateRouteAsync
Extract RouteValidator (aggregating validator), RoutePointGraphBuilder
(point interpolation + sequence numbering), GeofenceGridCalculator
(NW/SE region centers), and RouteResponseMapper (entity -> DTO; also
used by GetRouteAsync, eliminating duplicate DTO assembly).

CreateRouteAsync shrinks 184 -> 52 LOC of orchestration. RouteService.cs
shrinks 295 -> 138 LOC overall. Validation aggregates all failures into
a single ArgumentException (AC-2); single-violation messages preserved
verbatim so existing RouteServiceTests pass unchanged. 28 new unit
tests for the four helpers (112/112 unit tests, smoke green).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 02:08:21 +03:00
Oleksandr Bezdieniezhnykh 89260d0ec4 [AZ-368] Refactor C15: extract shared TileCsvWriter
Both RegionService.GenerateCsvFileAsync and
RouteProcessingService.GenerateRouteCsvAsync wrote the same CSV
shape: header "latitude,longitude,file_path", same
OrderByDescending(Latitude).ThenBy(Longitude) ordering, same F6
numeric format. Two near-identical writers with no shared abstraction.

Extracted TileCsvWriter (instance class, no DI dependencies) plus a
TileCsvRow record bridging the per-pipeline DTOs (TileMetadata vs
TileInfo) to a single contract. The header constant, ordering rule,
and StreamWriter lifecycle now live in one place.

Both call sites collapse to a one-line projection plus a delegated
WriteAsync call. Region method becomes static (no longer references
instance state). Route method preserves its existing logger line.

Coverage:
- 7 new unit tests including a byte-for-byte equivalence test that
  writes the same input via both the new TileCsvWriter and the
  inlined-original code path side by side and asserts file bytes
  are identical.
- Integration smoke + full suite green; route + region CSV outputs
  unchanged across all existing scenarios (verified by extended-route
  CSV verification step in the integration suite).
- 84/84 unit tests pass (was 77).

Side improvement: writer now respects CancellationToken mid-loop.
The pre-refactor inline code did not. Strict improvement; consistent
with every other async API in the codebase.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 01:07:51 +03:00
Oleksandr Bezdieniezhnykh 330bccd724 [AZ-366] Refactor C13: consolidate Haversine + tile-coord parsing
RouteProcessingService.CalculateDistance(double, double, double, double)
re-implemented Haversine using EARTH_RADIUS=6371000 alongside the
canonical GeoUtils.CalculateDistance(GeoPoint, GeoPoint) which uses
EARTH_RADIUS=6378137. Two implementations of the same formula for the
same problem.

Separately, ExtractTileCoordinatesFromFilename in RouteProcessingService
parsed the tile_{z}_{x}_{y}_{ts}.jpg filename pattern that's *generated*
by StorageConfig.GetTileFilePath in another assembly — the writer and
parser were coupled by string convention only and a format change in
one would silently break the other.

Both fixes:

(a) Deleted the duplicate Haversine in RouteProcessingService. The
single call site (region-matching nearest-neighbor OrderBy) now uses
GeoUtils.CalculateDistance with GeoPoint instances. The constant
difference is monotonic-equivalent for OrderBy purposes — same region
is picked.

(b) Added static StorageConfig.TryExtractTileCoordinates(string, out
int, out int): bool — pure parser, co-located with GetTileFilePath so
the inverse-pair invariant is structural, not by-convention.
RouteProcessingService.ExtractTileCoordinatesFromFilename becomes a
thin wrapper that delegates to the helper and emits the existing
warning log on malformed input — the AZ-352 tests for warning behavior
all still pass.

Verification:
- 77/77 unit tests green (was 71 → +6 new StorageConfigTests including
  a writer/parser round-trip test for AC-2).
- Smoke + full integration suite green.
- AC-1 grep verification: Math.Sin/EARTH_RADIUS patterns are now
  confined to GeoUtils.cs only.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 00:56:46 +03:00
Oleksandr Bezdieniezhnykh 2393bff1f2 [AZ-362] Refactor C09: idempotent POST contract for caller-supplied GUIDs
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>
2026-05-11 00:45:51 +03:00
Oleksandr Bezdieniezhnykh de4d4fa760 [AZ-351][AZ-352][AZ-363] Refactor 03 batch 1: critical defensive fixes
ci/woodpecker/push/01-test Pipeline was successful
ci/woodpecker/push/02-build-push Pipeline was successful
AZ-351: Resolve ILogger<DatabaseMigrator> directly from DI in
Program.cs instead of casting ILogger<Program> (which always
returned null). Migrator now logs through Serilog at startup.

AZ-352: Drop empty catch in
RouteProcessingService.ExtractTileCoordinatesFromFilename. Convert
the method from private static to internal instance so it can use
the existing _logger (per coderule: side-effecting code must not be
static). Add typed null-guard via ArgumentNullException.ThrowIfNull
so unexpected exceptions propagate. Adds InternalsVisibleTo on the
RouteManagement csproj for SatelliteProvider.Tests, plus 4 unit
tests in RouteProcessingServiceTests.cs covering AC-1 (valid /
malformed / non-numeric) and AC-2 (null path propagation).

AZ-363: Delete _totalEnqueued / _totalDequeued fields and the two
non-atomic ++ writes in RegionRequestQueue. Fields were write-only
dead code and a thread-safety hazard.

Tests: 44/44 unit + 5/5 smoke (scripts/run-tests.sh --smoke).
Code review verdict: PASS, 0 findings, 0 auto-fix attempts.
Batch report: _docs/03_implementation/batch_07_report.md.

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-10 23:34:17 +03:00
Oleksandr Bezdieniezhnykh 8b0ddae075 [AZ-312] [AZ-313] [AZ-314] Split Services into per-component csprojs
Phase B of architecture coupling refactor (epic AZ-309). Replaces
the monolithic SatelliteProvider.Services with three per-component
csprojs to add a compiler-enforced module boundary (resolves F4):

- SatelliteProvider.Services.TileDownloader
- SatelliteProvider.Services.RegionProcessing
- SatelliteProvider.Services.RouteManagement

DI registrations relocated into per-component AddTileDownloader /
AddRegionProcessing / AddRouteManagement extension methods called
from Program.cs. RateLimitException moved to Common/Exceptions/ to
keep the three new csprojs as siblings (no Region->TileDownloader
ProjectReference). Dockerfiles and consumer csprojs (Api, Tests)
rewired to the new project paths. No DI lifetime or hosted-service
order changes.

Build: 0 warn, 0 err. Unit tests: 40/40. Smoke integration: green.
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-10 07:15:44 +03:00