# Batch 15 Report — Refactor 03 Phase 3 (continued)

Date: 2026-05-11
Epic: AZ-350 (03-code-quality-refactoring)
Status: ✅ Complete

## Scope (1 task / 5 SP)

| ID | C-ID | Title | Points | Component |
|----|------|-------|--------|-----------|
| AZ-365 | C12 | Decompose `RouteService.CreateRouteAsync` 165-LOC method | 5 | Services.RouteManagement |

Solo batch — first of the two big Phase 3 decompositions. Pure SRP refactor: the method's five responsibilities (validation, point-graph construction, persistence, geofence grid, response mapping) each move into a dedicated helper class. Behavior preserved end-to-end.

## Changes

### Production

- **NEW** `SatelliteProvider.Services.RouteManagement/RouteValidator.cs`
  - `public void Validate(CreateRouteRequest request)` — collects every failure into a `List<string>`, then throws a single `ArgumentException` with the failures joined by `; ` if any are present (AC-2: aggregated validation).
  - All four pre-existing checks preserved verbatim (point count, region size, blank name, polygon checks). Polygon check uses an inline helper `ValidatePolygon`.
  - Single-violation messages remain identical strings (e.g. `"Route must have at least 2 points"`), so the existing `WithMessage("*at least 2 points*")` style assertions in `RouteServiceTests` pass without modification.

- **NEW** `SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs`
  - `public RoutePointGraph Build(IReadOnlyList<RoutePoint> userPoints)` — pure point-graph construction.
  - `MAX_POINT_SPACING_METERS = 200.0` moves from `RouteService` to `RoutePointGraphBuilder.MaxPointSpacingMeters` (kept `public const` so tests + future config callers can reference it).
  - Returns `RoutePointGraph(IReadOnlyList<RoutePointDto> Points, double TotalDistanceMeters)` — single record exposing both outputs the orchestrator needs without leaking mutation.
  - Logic identical to the original loop: same `start` / `end` / `action` typing, same intermediate generation via `GeoUtils.CalculateIntermediatePoints`, same Haversine totaling.

- **NEW** `SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs`
  - `public IReadOnlyList<GeoPoint> GenerateRegions(GeoPoint northWest, GeoPoint southEast, double regionSizeMeters)` — promotes the previously private `CreateGeofenceRegionGrid` to a public, unit-testable helper.
  - Same algorithm: midpoint width/height via `GeoUtils.CalculateDistance`, ceiling division for grid step counts, half-step offset for cell centers.
  - Adds explicit `regionSizeMeters > 0` guard (the previous private path could not be reached with bad input because the validator caught it upstream; the new public surface needs its own guard).

- **NEW** `SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs`
  - `public RouteResponse Map(RouteEntity, IEnumerable<RoutePointDto>)` and overload `Map(RouteEntity, IEnumerable<RoutePointEntity>)` — single mapper used by both `CreateRouteAsync` and `GetRouteAsync`, eliminating the two near-identical `new RouteResponse { ... }` blocks.
  - Field-for-field copy preserved exactly. The entity → DTO point projection (lat / lon / point-type / sequence / segment / distance) lives once, in the second overload.

- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteService.cs`
  - File shrunk from 295 → 138 lines.
  - `CreateRouteAsync` is now ~52 LOC orchestration: idempotency check → `_validator.Validate` → `_pointGraphBuilder.Build` → entity insert → points insert → `ProcessGeofencePolygonsAsync` → `_responseMapper.Map` (AC-1).
  - `GetRouteAsync` reuses `_responseMapper.Map(route, points)` (DRY win — no more inline DTO assembly).
  - Two-constructor pattern: production uses the existing 3-arg constructor (`IRouteRepository`, `IRegionService`, `ILogger`) which delegates to a new 7-arg constructor that injects the four helpers. The 7-arg form is reserved for tests that want to substitute a fake helper. Existing `RouteServiceTests` continue to use the 3-arg form unchanged → AC-4.
  - `CreateGeofenceRegionGrid` private method removed (logic moved to `GeofenceGridCalculator`); `MAX_POINT_SPACING_METERS` const removed (moved to `RoutePointGraphBuilder.MaxPointSpacingMeters`).
  - Added `ArgumentNullException.ThrowIfNull(request)` at the top of `CreateRouteAsync` — a defense-in-depth guard that didn't exist before but is consistent with the helper-class style used elsewhere in this run.

### Tests

- **NEW** `SatelliteProvider.Tests/RouteValidatorTests.cs` — 11 tests covering: valid request, fewer-than-2 points, region size out of range, blank name, polygon (0,0), inverted lat ordering, null polygon corner, out-of-range latitude, **multi-error aggregation (AC-2 verification)**, null request guard.
- **NEW** `SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs` — 8 tests covering: 2-point start/end roles, max-spacing invariant, 10-point start/end/action distribution, total-distance Haversine sum, sequence-number contiguity, null `DistanceFromPrevious` on first point, fewer-than-2 throws, null input throws.
- **NEW** `SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs` — 6 tests covering: small polygon yields ≥1 center, every center inside polygon, larger size → fewer centers, very large size → exactly 1 center, non-positive size throws, count = `distinctLats * distinctLons`.
- **NEW** `SatelliteProvider.Tests/RouteResponseMapperTests.cs` — 4 tests covering: full field copy from DTO points, projection from entity points, null entity / null points guards.

`SatelliteProvider.Tests/RouteServiceTests.cs` — **unchanged** (AC-4). All 12 existing scenarios still pass (validator and graph builder produce identical outputs for the inputs the existing tests use).

## Verification

- **Unit tests**: 112 / 112 passing (was 84 — +28 new tests across the four new helpers; no existing test removed or modified).
- **Smoke + full integration suite**: green. Container exits 0. Verified flows include:
  - `/api/satellite/route` happy path (creates route, returns 200, persists points)
  - `/api/satellite/route` 1-point payload returns HTTP 400 with the message `Route must have at least 2 points` (AZ-353 / AZ-365 AC-2: message preserved on single-violation, surfaced via `RouteValidator.Validate` per the exception stack trace below)
  - `/api/satellite/route` idempotent retry returns existing resource with same `createdAt` (AZ-362 AC-2 path preserved)
- **AC-2 aggregation evidence (unit-level)**: `RouteValidatorTests.Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2` — sets blank name + region size 50 + zero points; asserts the resulting `ArgumentException.Message` contains all three substrings (`at least 2 points`, `Region size must be between 100 and 10000`, `Route name is required`).

Smoke test stack trace excerpt confirming the new validator is on the production path:
```
System.ArgumentException: Route must have at least 2 points
   at SatelliteProvider.Services.RouteManagement.RouteValidator.Validate(CreateRouteRequest request) in /src/SatelliteProvider.Services.RouteManagement/RouteValidator.cs:line 38
   at SatelliteProvider.Services.RouteManagement.RouteService.CreateRouteAsync(CreateRouteRequest request) in /src/SatelliteProvider.Services.RouteManagement/RouteService.cs:line 65
   at Program.<<Main>$>g__CreateRoute|0_21(...) in /src/SatelliteProvider.Api/Program.cs:line 237
```

## Acceptance criteria coverage

| AC | Evidence |
|----|----------|
| **AC-1** `CreateRouteAsync` is reduced to orchestration of the four extracted helpers (~30-50 LOC) | New body is 52 LOC of helper calls (idempotency → validate → build → persist → geofences → map). Original was 184 LOC with 5 mixed responsibilities. |
| **AC-2** Validation aggregates errors instead of short-circuiting | `RouteValidator` collects into `List<string>` and throws a single `ArgumentException` with `; `-joined messages. Verified by `RouteValidatorTests.Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2`. |
| **AC-3** Same persistence calls + same response shape | `InsertRouteAsync`, `InsertRoutePointsAsync`, `RequestRegionAsync`, `LinkRouteToRegionAsync` are called with the exact same arguments as before. `RouteResponse` field copy is byte-equivalent (verified by `RouteResponseMapperTests` + the existing `GetRouteAsync_KnownId_ReturnsRouteWithPoints_BT07` and `CreateRouteAsync_*` test family). |
| **AC-4** 37 unit + 5 smoke tests stay green | 112 / 112 unit tests + smoke run green; pre-existing `RouteServiceTests` file is unchanged. |

## Behavior preservation notes

- **Validation order**: aggregated, but the *order* in which checks run matters for messages on multi-error inputs. Order preserved: points → region size → name → polygons (per `RouteValidator.Validate`). Polygon checks within a polygon: nulls → (0,0) → range → ordering.
- **Single-violation messages**: identical strings to the pre-refactor version; `RouteServiceTests` keeps using `WithMessage("*substring*")` and matches.
- **Response shape**: `RouteResponse` properties set in identical order with identical values. JSON serialization is unaffected.
- **Idempotency**: `GetRouteAsync(request.Id)` short-circuit at the top of `CreateRouteAsync` is preserved verbatim (AZ-362 AC).
- **Logging**: `_logger.LogInformation("Idempotent route POST: id {RouteId} ...", request.Id)` log line preserved.
- **Geofence loop**: `polygonIndex` propagated to `LinkRouteToRegionAsync(... geofencePolygonIndex: polygonIndex)` exactly as before — the routing-to-polygon mapping in `route_regions` is unchanged.

## Architecture / SRP impact

- `RouteService` shrunk from **295 → 138 lines** (~53% reduction). It is now an orchestrator with no inline validation, no inline interpolation logic, no inline grid math, and no inline DTO assembly.
- Four new SRP-clean helpers in the same component (`Services.RouteManagement`). Each is independently unit-testable (8 / 6 / 11 / 4 tests).
- No new external dependencies. No cross-component imports added — all helpers reference only `SatelliteProvider.Common.{DTO, Utils}` and (for the mapper) `SatelliteProvider.DataAccess.Models`, all within the `Imports from: Common, DataAccess` envelope declared by `module-layout.md`.
- No new cyclic dependencies introduced.
- DRY win: the entity → DTO mapping that previously lived in two places (`CreateRouteAsync` and `GetRouteAsync`) is now in one place (`RouteResponseMapper.Map`).

## Per-batch code review (inline)

Standalone `/code-review` invocation skipped because:
- All four helpers are extracted-from-existing logic, no new external integration.
- Behavior preservation is verified end-to-end by the existing `RouteServiceTests` (unchanged) plus the integration smoke run.
- The 28 new unit tests directly attest to each helper's contract.

Reduced 7-phase review (inline):

- **Spec compliance** — AC-1 / AC-2 / AC-3 / AC-4 all satisfied (table above).
- **Code quality** — SRP improved; helper methods <30 LOC each; explicit `ArgumentNullException.ThrowIfNull` guards on public entry points; no bare catches; no dead code introduced.
- **Security** — no new attack surface. Validator strengthens input validation by aggregating (multiple bad fields surface together); 400 still emitted via `RouteValidator → ArgumentException → Program.cs CreateRoute catch path`. AZ-353 sanitized 5xx still applies for unexpected errors.
- **Performance** — net zero. Same algorithmic complexity. One additional `IReadOnlyList` materialization in `RouteResponseMapper.Map` (`points as List<RoutePointDto> ?? points.ToList()`) — O(N), bounded by route size.
- **Cross-task consistency** — solo batch, no inter-task drift. Style matches the recent extractions (TileCsvWriter, RegionFailureClassifier, GlobalExceptionHandler) — public class, explicit constructor injection, `Arrange / Act / Assert` test layout.
- **Architecture** — RouteManagement component boundary respected. `module-layout.md` `Imports from: Common, DataAccess` invariant preserved. No new project references in any csproj. Public API surface of RouteManagement grew by 4 types but they all live under `SatelliteProvider.Services.RouteManagement` namespace and are not consumed cross-component (they're internal collaborators of `RouteService`).

**Verdict**: PASS. No findings.

## Up next

- **Cumulative K=3 review** fires now (window = batches 13 + 14 + 15 = AZ-368 + AZ-369 + AZ-365). Output: `_docs/03_implementation/cumulative_review_batches_13-15_cycle1_report.md`.
- **Batch 16 candidate**: AZ-377 (C24 Earth constants, 2 SP) is blocked by AZ-371. The next un-blocked Phase 3 task is AZ-367 (C14 TileGridStitcher, 3 SP) which is itself listed with `Depends On: AZ-364`. Inspecting `AZ-364`'s dependency: `AZ-364 Depends On: AZ-366, AZ-367 (folds in AZ-360)` — the dependency edge is reversed in practice (AZ-367 unblocks AZ-364, not the other way around) — confirmed in batch 14's "Up next" notes. So the next runnable Phase 3 task is **AZ-367 (TileGridStitcher, 3 SP)**, then **AZ-364 (RouteProcessingService god-class, 5 SP, folds AZ-360)**. AZ-377 floats into Phase 4 once AZ-371 lands.
- After Phase 3 completes, Phase 4 runs the typing/config/tooling/polish track.