mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-22 12:51:13 +00:00
[AZ-364] [AZ-360] Archive task files: todo -> done
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -1,60 +0,0 @@
|
||||
# Refactor: replace IServiceProvider with IRegionService in RouteProcessingService
|
||||
|
||||
**Task**: AZ-360_refactor_replace_iserviceprovider_routeproc
|
||||
**Name**: Direct IRegionService injection in RouteProcessingService
|
||||
**Description**: Inject `IRegionService` directly into `RouteProcessingService`; remove the `IServiceProvider` field and the per-iteration scope creation.
|
||||
**Complexity**: 2 points
|
||||
**Dependencies**: AZ-364 (C11) — if C11 ships first, the C08 changes happen as part of C11 and this task may close as duplicate.
|
||||
**Component**: Services.RouteManagement
|
||||
**Tracker**: AZ-360
|
||||
**Epic**: AZ-350
|
||||
|
||||
## Problem
|
||||
|
||||
`SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs:18-22, 105-109, 165-169` injects `IServiceProvider`, then creates a new scope and resolves `IRegionService` inside the loop. `IRegionService` is registered as a singleton (verified in `RegionProcessingServiceCollectionExtensions`), so the scope creation is unnecessary and the service-locator pattern hides the real dependency.
|
||||
|
||||
## Outcome
|
||||
|
||||
- `RouteProcessingService` declares `IRegionService` in its constructor.
|
||||
- No `IServiceProvider` field, no per-iteration `using var scope = _serviceProvider.CreateScope();`.
|
||||
- 37 unit + 5 smoke tests stay green.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Update constructor parameter list and field.
|
||||
- Update DI registration in `RouteManagementServiceCollectionExtensions` if the order matters.
|
||||
- Remove the two `using var scope` blocks; call `_regionService.<method>` directly.
|
||||
|
||||
### Excluded
|
||||
- Changing `IRegionService`'s registration lifetime.
|
||||
- Other parts of the C11 god-class decomposition.
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Constructor declares dependency**
|
||||
Given the post-refactor `RouteProcessingService`
|
||||
When inspected
|
||||
Then the constructor parameter list contains `IRegionService` (not `IServiceProvider`).
|
||||
|
||||
**AC-2: No scope creation in the loop**
|
||||
Given the post-refactor source
|
||||
When grepping for `_serviceProvider.CreateScope()` in `RouteProcessingService.cs`
|
||||
Then zero matches.
|
||||
|
||||
**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
|
||||
|
||||
- `IRegionService` must remain a singleton for this change to be safe. If a future change makes it scoped, switch to `IServiceScopeFactory`.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: C11 lands first and folds this in**
|
||||
- *Risk*: duplicate work if C11 (AZ-364) ships before C08.
|
||||
- *Mitigation*: C11 task spec explicitly calls out folding C08; this ticket closes as duplicate when that happens.
|
||||
|
||||
Full change entry: `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C08).
|
||||
@@ -1,82 +0,0 @@
|
||||
# Refactor: decompose RouteProcessingService god-class into 6 collaborators
|
||||
|
||||
**Task**: AZ-364_refactor_decompose_routeprocessing_service
|
||||
**Name**: Split RouteProcessingService into orchestrator + 6 collaborators
|
||||
**Description**: Extract `RouteRegionMatcher`, `RouteCsvWriter`, `RouteSummaryWriter`, `RouteImageRenderer`, `TilesZipBuilder`, `RegionFileCleaner` from `RouteProcessingService`. The hosted service becomes a thin orchestrator. Folds in C08 (replace `IServiceProvider` with `IRegionService`).
|
||||
**Complexity**: 5 points
|
||||
**Dependencies**: AZ-366 (C13 — shared Haversine), AZ-367 (C14 — shared stitcher); folds in AZ-360 (C08)
|
||||
**Component**: Services.RouteManagement
|
||||
**Tracker**: AZ-364
|
||||
**Epic**: AZ-350
|
||||
|
||||
## Problem
|
||||
|
||||
`SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs` (~750 LOC) is a single `BackgroundService` that does queue polling, region matching, CSV parsing, summary writing, image stitching, geofence-rectangle drawing, route-cross drawing, ZIP creation, and per-region cleanup. The file even hosts a public `TileInfo` POCO at the bottom. Six+ responsibilities; the file is hard to navigate, hard to test, and any change touches multiple concerns.
|
||||
|
||||
## Outcome
|
||||
|
||||
- `RouteProcessingService` becomes a thin orchestrator that polls the queue and dispatches to collaborators.
|
||||
- Six new collaborator classes, each with a single responsibility, each unit-testable without a queue.
|
||||
- `TileInfo` lives in its own file under `Services.RouteManagement` (or `Common/DTO`).
|
||||
- `IRegionService` is injected directly (folds in C08).
|
||||
- Same `BackgroundService` lifecycle, same DB writes, same output files (CSV, summary, stitched image, ZIP).
|
||||
- 37 unit + 5 smoke tests stay green; route image output identical for existing scenarios.
|
||||
|
||||
## Scope
|
||||
|
||||
### Included
|
||||
- Extract:
|
||||
- `RouteRegionMatcher` (pure: route points + completed regions → ordered region list).
|
||||
- `RouteCsvWriter` (writes route_<id>_ready.csv from `IEnumerable<TileInfo>`).
|
||||
- `RouteSummaryWriter` (writes route_<id>_summary.txt; includes the StringBuilder block).
|
||||
- `RouteImageRenderer` (image stitching + cross/border drawing).
|
||||
- `TilesZipBuilder` (ZIP archive creation; resolves entry names).
|
||||
- `RegionFileCleaner` (deletes per-region CSV/summary/stitched files).
|
||||
- Move `TileInfo` to its own file.
|
||||
- Inject `IRegionService` directly (delete `IServiceProvider` field and the two scope blocks).
|
||||
- Add unit tests for each collaborator in isolation.
|
||||
|
||||
### Excluded
|
||||
- Changing the queue mechanism.
|
||||
- Changing the `BackgroundService` lifecycle.
|
||||
- Changing output file formats (CSV header, summary structure, ZIP layout).
|
||||
|
||||
## Acceptance Criteria
|
||||
|
||||
**AC-1: Single-responsibility collaborators**
|
||||
Given the post-refactor source
|
||||
When inspected
|
||||
Then each new collaborator class has one public entry point and is independently unit-testable.
|
||||
|
||||
**AC-2: Same outputs for existing scenarios**
|
||||
Given the existing route smoke tests (BasicRouteTests, ExtendedRouteTests)
|
||||
When they run against the post-refactor code
|
||||
Then the produced CSV, summary, stitched image, and ZIP files are identical (byte-for-byte for CSV/summary; pixel-for-pixel for the image).
|
||||
|
||||
**AC-3: No IServiceProvider in RouteProcessingService**
|
||||
Given the post-refactor source
|
||||
When grepping `RouteProcessingService.cs` for `IServiceProvider`
|
||||
Then zero matches.
|
||||
|
||||
**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
|
||||
|
||||
- Same `BackgroundService` registration (no DI lifetime changes for the hosted service).
|
||||
- Output file paths and contents preserved.
|
||||
- Architecture Vision (`architecture.md` H2) honored — collaborators stay inside `Services.RouteManagement`.
|
||||
|
||||
## Risks & Mitigation
|
||||
|
||||
**Risk 1: image output diff after stitcher refactor**
|
||||
- *Risk*: a subtle pixel diff in the stitched image may break the integration test image comparisons.
|
||||
- *Mitigation*: drive C14 (shared stitcher) first; this task plugs into the result.
|
||||
|
||||
**Risk 2: hidden state shared across the 6 concerns**
|
||||
- *Risk*: the god class may share state in ways that don't surface until extracted.
|
||||
- *Mitigation*: extract one collaborator at a time, run tests between each extraction.
|
||||
|
||||
Full change entry: `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C11).
|
||||
Reference in New Issue
Block a user