[AZ-309] Refactor 02-coupling-refactoring Phase 0-2 artifacts

- Baseline metrics, list of changes, and analysis (research findings,
  refactoring roadmap) for the coupling refactor run
- Six task specs AZ-310..AZ-315 covering endpoint routing through
  ITileService, Services csproj split, consumer rewire, DI extension
  methods, and docs sync
- Existing test coverage assessment for Phase 3 safety net gate
- Dependencies table updated with the refactor block

Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
Oleksandr Bezdieniezhnykh
2026-05-10 05:53:29 +03:00
parent cc0a876168
commit 220277b9c7
13 changed files with 860 additions and 10 deletions
+27 -7
View File
@@ -2,25 +2,45 @@
## Dependency Graph
### Step 6 — Implement Tests (AZ-285..AZ-290)
| Task | Depends On | Points | Status |
|------|-----------|--------|--------|
| AZ-285 Test Infrastructure | — | 3 | To Do |
| AZ-286 TileService Tests | AZ-285 | 3 | To Do |
| AZ-287 RegionService Tests | AZ-285 | 3 | To Do |
| AZ-288 RouteService Tests | AZ-285 | 3 | To Do |
| AZ-289 Integration Route Maps | AZ-285 | 2 | To Do |
| AZ-290 Non-Functional Tests | AZ-285 | 3 | To Do |
| AZ-285 Test Infrastructure | — | 3 | Done |
| AZ-286 TileService Tests | AZ-285 | 3 | Done |
| AZ-287 RegionService Tests | AZ-285 | 3 | Done |
| AZ-288 RouteService Tests | AZ-285 | 3 | Done |
| AZ-289 Integration Route Maps | AZ-285 | 2 | Done |
| AZ-290 Non-Functional Tests | AZ-285 | 3 | Done |
### Step 8 — Refactor 02-coupling-refactoring (AZ-309 epic)
| Task | Depends On | Points | Status |
|------|-----------|--------|--------|
| AZ-310 ServeTile via ITileService | — | 3 | To Do |
| AZ-311 GetTileByLatLon via ITileService | AZ-310 | 2 | To Do |
| AZ-312 Split Services into 3 csprojs | AZ-311 | 5 | To Do |
| AZ-313 Update consumers (Api/Tests) | AZ-312 | 3 | To Do |
| AZ-314 DI registration split | AZ-313 | 2 | To Do |
| AZ-315 Documentation sync | AZ-314 | 2 | To Do |
## Execution Order
### Step 6
1. AZ-285 (test infrastructure — all others depend on this)
2. AZ-286, AZ-287, AZ-288 (unit tests — can run in parallel)
3. AZ-289 (integration tests — depends on infra only)
4. AZ-290 (non-functional tests — depends on infra only)
### Step 8 (refactor)
1. AZ-310 → AZ-311 (Phase A: route tile endpoints through ITileService)
2. AZ-312 → AZ-313 → AZ-314 (Phase B: physical split + consumer + DI rewire)
3. AZ-315 (Phase C: docs sync, must be last)
## Total Effort
6 tasks, 17 story points total
Step 6: 6 tasks, 17 story points
Step 8 (refactor): 6 tasks, 17 story points
## Coverage Verification
@@ -0,0 +1,79 @@
# Refactor: route ServeTile through ITileService
**Task**: AZ-310_refactor_servetile_via_tileservice
**Name**: ServeTile via ITileService.GetOrDownloadTileAsync
**Description**: Move tile cache+DB+download logic from the `/tiles/{z}/{x}/{y}` endpoint into `ITileService` and make the endpoint a thin handler.
**Complexity**: 3 points
**Dependencies**: None
**Component**: TileDownloader (currently in SatelliteProvider.Services)
**Tracker**: AZ-310
**Epic**: AZ-309
## Problem
`Program.cs:141` (`ServeTile`) directly injects `ISatelliteDownloader`, `ITileRepository`, and `IMemoryCache`. It re-implements cache-or-DB-or-download logic that overlaps with `TileService` but is not delegated. This is architecture baseline finding F3 (Medium).
## Outcome
- All tile-serving logic for `/tiles/{z}/{x}/{y}` is owned by `TileService`.
- `ServeTile` handler is a thin function: validate route, call service, write headers, return bytes.
- `IMemoryCache` is no longer injected at the endpoint level for tile serving.
## Scope
### Included
- New method `Task<TileBytes> GetOrDownloadTileAsync(int z, int x, int y, CancellationToken ct = default)` on `ITileService`. `TileBytes` is a record `(byte[] Bytes, string ContentType, string ETag, TimeSpan MaxAge)`.
- Implementation in `TileService` owns the in-memory cache (DI-injected).
- New unit tests for `GetOrDownloadTileAsync` (cache hit, repo hit, downloader fallback).
- `Program.cs` ServeTile handler thinned.
### Excluded
- Adding a new integration test for `/tiles/{z}/{x}/{y}` (existing smoke does not cover it; out of scope here).
- Renaming any database tables, columns, or DTOs.
## Acceptance Criteria
**AC-1: Endpoint route preserved**
Given the API is running
When a client calls `GET /tiles/{z}/{x}/{y}`
Then the response shape (image/jpeg bytes, ETag header, Cache-Control header) is unchanged from before the refactor.
**AC-2: Cache hit path**
Given a tile has previously been served and is still in the in-memory cache
When `GetOrDownloadTileAsync` is called for the same `(z,x,y)`
Then the result comes from cache and neither the repository nor the downloader is invoked.
**AC-3: Repo hit path**
Given a tile is not in cache but exists in the database with a valid file path
When `GetOrDownloadTileAsync` is called
Then the file is read from disk, cached, and returned without invoking the downloader.
**AC-4: Downloader fallback path**
Given a tile is neither in cache nor in the database
When `GetOrDownloadTileAsync` is called
Then `ISatelliteDownloader.DownloadSingleTileAsync` is invoked, the downloaded tile is inserted into the repository, the bytes are read, cached, and returned.
**AC-5: Endpoint no longer injects downloader/repo/cache**
Given the post-refactor `Program.cs`
When the `ServeTile` handler is inspected
Then it injects only `ITileService`, `HttpContext`, and `ILogger<Program>` (not `ISatelliteDownloader`, `ITileRepository`, or `IMemoryCache`).
## Unit Tests
| AC Ref | What to Test | Required Outcome |
|--------|--------------|------------------|
| AC-2 | Cache hit | Mock cache returns bytes; repo + downloader mocks asserted unused |
| AC-3 | Repo hit | Mock repo returns tile entity with existing file path; downloader mock asserted unused |
| AC-4 | Downloader fallback | Mock repo returns null; downloader returns DownloadedTileInfoV2; assert insert + return |
## Constraints
- Public route, query/body shape, response shape preserved.
- No new external libraries.
- Cache lifetime (1h absolute, 30min sliding) preserved exactly.
## Risks & Mitigation
**Risk 1: Cache singleton lifetime semantics change**
- *Risk*: Moving `IMemoryCache` from endpoint scope into service scope might alter cache key collision behavior.
- *Mitigation*: TileService is registered as `Singleton`; `IMemoryCache` is also Singleton. Cache keys remain `tile_{z}_{x}_{y}`.
@@ -0,0 +1,66 @@
# Refactor: route GetTileByLatLon through ITileService
**Task**: AZ-311_refactor_gettilebylatlon_via_tileservice
**Name**: GetTileByLatLon via ITileService.DownloadAndStoreSingleTileAsync
**Description**: Move single-tile download+insert logic from `/api/satellite/tiles/latlon` into `ITileService` and thin the endpoint handler.
**Complexity**: 2 points
**Dependencies**: AZ-310
**Component**: TileDownloader (currently in SatelliteProvider.Services)
**Tracker**: AZ-311
**Epic**: AZ-309
## Problem
`Program.cs:206` (`GetTileByLatLon`) injects `ISatelliteDownloader` and `ITileRepository` and re-implements download-then-insert. Same root cause as AZ-310 (architecture baseline F3).
## Outcome
- `GetTileByLatLon` handler is thin: call service, project to response, return.
- `TileService` owns the download+insert flow for single-tile requests.
## Scope
### Included
- New method `Task<TileMetadata> DownloadAndStoreSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken ct = default)` on `ITileService`.
- Implementation calls `ISatelliteDownloader.DownloadSingleTileAsync` then `ITileRepository.InsertAsync`.
- New unit tests for the new method.
- `Program.cs` `GetTileByLatLon` handler thinned.
### Excluded
- Changes to `DownloadTileResponse` shape.
- Changes to `ISatelliteDownloader` (zoom validation chain is unchanged).
## Acceptance Criteria
**AC-1: Endpoint contract preserved**
Given the API is running
When a client calls `GET /api/satellite/tiles/latlon?Latitude=...&Longitude=...&ZoomLevel=...`
Then the response is the same `DownloadTileResponse` JSON shape as before the refactor.
**AC-2: Service owns the flow**
Given valid lat/lon/zoom
When `DownloadAndStoreSingleTileAsync` is called
Then `ISatelliteDownloader.DownloadSingleTileAsync` is invoked once, `ITileRepository.InsertAsync` is invoked once, and the resulting `TileMetadata` is returned.
**AC-3: Endpoint no longer injects downloader/repo**
Given the post-refactor `Program.cs`
When the `GetTileByLatLon` handler is inspected
Then it injects `ITileService` and `ILogger<Program>` only (no `ISatelliteDownloader` or `ITileRepository`).
## Unit Tests
| AC Ref | What to Test | Required Outcome |
|--------|--------------|------------------|
| AC-2 | Happy path | Mocks for downloader + repo wired; assert one call each; assert TileMetadata fields match the downloaded tile |
| AC-2 | Downloader throws | Service propagates the exception; repo `InsertAsync` is not called |
## Constraints
- HTTP route, query parameter names, and response JSON shape preserved exactly.
- Zoom validation in `GoogleMapsDownloaderV2.DownloadSingleTileAsync` keeps firing.
## Risks & Mitigation
**Risk 1: TileMetadata projection drift**
- *Risk*: The endpoint currently constructs `DownloadTileResponse` directly from a `TileEntity`. After refactor it must do so from `TileMetadata`.
- *Mitigation*: Compare every field one-to-one in the new code; existing field-level mapping is straightforward.
@@ -0,0 +1,82 @@
# Refactor: split SatelliteProvider.Services into per-component csprojs
**Task**: AZ-312_refactor_split_services_csprojs
**Name**: Split Services into TileDownloader + RegionProcessing + RouteManagement
**Description**: Replace the single `SatelliteProvider.Services` csproj with three per-component csprojs to add a compiler-enforced module boundary.
**Complexity**: 5 points
**Dependencies**: AZ-311
**Component**: All Services components
**Tracker**: AZ-312
**Epic**: AZ-309
## Problem
`SatelliteProvider.Services.csproj` packs three logical components (TileDownloader, RegionProcessing, RouteManagement) into one project. No compiler-enforced boundary prevents accidental cross-component coupling. This is architecture baseline finding F4 (Medium).
## Outcome
- Three new csprojs exist:
- `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj`
- `SatelliteProvider.Services.RegionProcessing/SatelliteProvider.Services.RegionProcessing.csproj`
- `SatelliteProvider.Services.RouteManagement/SatelliteProvider.Services.RouteManagement.csproj`
- Seven source files moved into the matching project.
- `SatelliteProvider.Services.csproj` deleted.
- Each new csproj `ProjectReference`s only what it needs.
- Solution file `SatelliteProvider.sln` updated.
## Scope
### Included
- File moves:
- `TileService.cs`, `GoogleMapsDownloaderV2.cs``SatelliteProvider.Services.TileDownloader/`
- `RegionService.cs`, `RegionProcessingService.cs`, `RegionRequestQueue.cs``SatelliteProvider.Services.RegionProcessing/`
- `RouteService.cs`, `RouteProcessingService.cs``SatelliteProvider.Services.RouteManagement/`
- Namespace changes:
- `SatelliteProvider.Services``SatelliteProvider.Services.TileDownloader` (in TileService, GoogleMapsDownloaderV2)
- `SatelliteProvider.Services``SatelliteProvider.Services.RegionProcessing` (in three Region* files)
- `SatelliteProvider.Services``SatelliteProvider.Services.RouteManagement` (in two Route* files)
- Add `ProjectReference` entries in each new csproj as required by its members.
- Delete `SatelliteProvider.Services/` directory once empty.
- Update `SatelliteProvider.sln` (add new projects, remove old).
### Excluded
- Updating consumers (Tests, IntegrationTests, Api csprojs and their `using` directives) — those go in AZ-313 and AZ-314.
- Any logic change inside the moved files.
## Acceptance Criteria
**AC-1: Three new csprojs exist with correct contents**
Given the post-refactor tree
When listing the new csproj directories
Then each contains only the files listed in the `Included` section, plus its csproj file.
**AC-2: Old project deleted**
Given the post-refactor tree
When searching for `SatelliteProvider.Services.csproj` or `SatelliteProvider.Services/` directory
Then neither exists.
**AC-3: Solution-level build succeeds with the new csprojs unreferenced**
Given the new csprojs exist but no consumer has been updated yet
When `dotnet build SatelliteProvider.sln` is run
Then build fails ONLY for the unreferenced consumers (Api, Tests, IntegrationTests). The three new csprojs themselves compile clean.
**AC-4: No cross-component reference between the three new csprojs**
Given the three new csproj files
When inspecting their `ProjectReference` entries
Then none of TileDownloader, RegionProcessing, or RouteManagement references another of the three. They all reference only `SatelliteProvider.Common` and (where needed) `SatelliteProvider.DataAccess`.
## Constraints
- No code logic changes inside the moved files.
- Namespaces follow `SatelliteProvider.Services.<Component>` pattern.
- Existing public API of `ITileService`, `IRegionService`, `IRouteService`, `IRegionRequestQueue` (in Common) unchanged.
## Risks & Mitigation
**Risk 1: Hidden cross-component coupling**
- *Risk*: A move reveals that a Region* class actually `using SatelliteProvider.Services` for a TileDownloader-only type.
- *Mitigation*: If found, the cleanest fix is to lift the shared type into `SatelliteProvider.Common` (extend in this task) or add a `ProjectReference` (last resort, document why). Stop and ask if this surfaces.
**Risk 2: SatelliteProvider.sln drift**
- *Risk*: Forgetting to update the solution file leaves the new csprojs invisible to Docker/CI.
- *Mitigation*: Use `dotnet sln add` and `dotnet sln remove` exactly once for each project; assert via `dotnet sln list`.
@@ -0,0 +1,64 @@
# Refactor: update consumers (Api, Tests, IntegrationTests) for split
**Task**: AZ-313_refactor_update_consumers
**Name**: Update consumer csprojs and using directives after Services split
**Description**: Re-wire `SatelliteProvider.Api`, `SatelliteProvider.Tests`, and `SatelliteProvider.IntegrationTests` to reference the three new per-component csprojs.
**Complexity**: 3 points
**Dependencies**: AZ-312
**Component**: API + Tests
**Tracker**: AZ-313
**Epic**: AZ-309
## Problem
After AZ-312 splits `SatelliteProvider.Services` into three csprojs, every consumer of the old project breaks (compile errors). This task fixes the consumers without changing any test logic or DI behavior.
## Outcome
- `SatelliteProvider.Api.csproj` references all three new Services projects.
- `SatelliteProvider.Tests.csproj` references whichever Services projects its current tests depend on (typically all three; verify per file).
- `SatelliteProvider.IntegrationTests.csproj` references whichever Services projects its current tests depend on.
- All `using SatelliteProvider.Services;` directives across consumers replaced with the correct `using SatelliteProvider.Services.<Component>;` directive.
- `dotnet build SatelliteProvider.sln` succeeds.
- All previously-green unit tests remain green.
## Scope
### Included
- Edit `.csproj` files of Api, Tests, IntegrationTests: add new `ProjectReference` entries, remove the old `SatelliteProvider.Services` reference.
- Update all `using` directives in `.cs` files under those three projects.
- Run `dotnet build` to confirm zero errors.
- Run unit test suite (`SatelliteProvider.Tests`) to confirm zero regressions vs. Step 6 baseline.
### Excluded
- DI container registration changes — those happen in AZ-314.
- Re-running integration suite — that happens in AZ-315.
- Changing any test assertion or test data.
## Acceptance Criteria
**AC-1: Solution builds clean**
Given the post-refactor consumer csprojs
When `dotnet build SatelliteProvider.sln` is run
Then build succeeds with zero errors and zero new warnings vs. the pre-refactor baseline.
**AC-2: Unit tests green**
Given the post-refactor consumer csprojs
When `dotnet test SatelliteProvider.Tests` is run
Then all 35 (or current baseline count) unit tests pass.
**AC-3: No stale `using SatelliteProvider.Services;`**
Given the post-refactor source tree
When grepping for `using SatelliteProvider.Services;` (without a `.<Component>` suffix)
Then zero matches outside intentional comments.
## Constraints
- No DI registration changes in this task.
- No test logic, fixture, or assertion changes.
## Risks & Mitigation
**Risk 1: Shadowed types if a single file references types from two of the new components**
- *Risk*: A test that exercises both `RegionService` and `RouteService` needs both `using` statements.
- *Mitigation*: Add both. Compiler will tell us which.
@@ -0,0 +1,69 @@
# Refactor: split DI registration to per-component extension methods
**Task**: AZ-314_refactor_di_registration_split
**Name**: Per-component AddXxxServices() extension methods in Program.cs
**Description**: Replace ad-hoc DI registrations in `Program.cs` with three extension methods, one per new csproj, matching the module boundary.
**Complexity**: 2 points
**Dependencies**: AZ-313
**Component**: API
**Tracker**: AZ-314
**Epic**: AZ-309
## Problem
`Program.cs` currently registers all services inline and ungrouped. After the split, the three Services csprojs have logically distinct registrations; co-locating each set with its csproj clarifies ownership and makes adding a new component easier.
## Outcome
- New extension methods in each new Services csproj:
- `SatelliteProvider.Services.TileDownloader.TileDownloaderServiceCollectionExtensions.AddTileDownloader(this IServiceCollection)`
- `SatelliteProvider.Services.RegionProcessing.RegionProcessingServiceCollectionExtensions.AddRegionProcessing(this IServiceCollection)`
- `SatelliteProvider.Services.RouteManagement.RouteManagementServiceCollectionExtensions.AddRouteManagement(this IServiceCollection)`
- Each method registers exactly the services its csproj owns (concrete types + `IHostedService` registrations).
- `Program.cs` calls the three methods and removes the duplicated lines.
- DI container behavior is byte-equivalent (same lifetimes, same concrete types, same hosted services).
## Scope
### Included
- Three new `*ServiceCollectionExtensions.cs` files (one per csproj).
- Edit `Program.cs` to call them.
- Verify via integration smoke run that DI resolves end-to-end.
### Excluded
- Changing service lifetimes or replacing concrete types.
- Adding `IOptions<>` bindings that aren't already there.
## Acceptance Criteria
**AC-1: Each extension method exists and registers the expected services**
Given the post-refactor source
When inspecting each `*ServiceCollectionExtensions.cs`
Then it registers all services owned by its csproj and nothing else.
**AC-2: Program.cs uses extension methods**
Given `Program.cs` after refactor
When inspecting service registration block
Then `services.AddTileDownloader();`, `services.AddRegionProcessing();`, `services.AddRouteManagement();` appear and the previously-inlined registrations are gone.
**AC-3: DI graph unchanged**
Given the running app
When the smoke integration test profile is executed
Then it passes end-to-end (proving every required service still resolves).
**AC-4: Consumers updated**
Given the post-refactor source
When grepping consumers
Then all references to old service-registration code are removed.
## Constraints
- No service lifetime changes (Singleton stays Singleton, Scoped stays Scoped).
- Hosted services stay hosted services.
- DI registration order preserved where ordering matters.
## Risks & Mitigation
**Risk 1: Hosted-service registration order regression**
- *Risk*: `IHostedService` execution order is registration-order; an accidental reorder could change startup behavior.
- *Mitigation*: Inspect Program.cs Git diff carefully; keep the same call-site order in the three extension methods as in the original file.
@@ -0,0 +1,65 @@
# Refactor: sync documentation after coupling refactor
**Task**: AZ-315_refactor_documentation_sync
**Name**: Update architecture, module-layout, and component docs for split
**Description**: Update `_docs/02_document/` to reflect the post-refactor module boundary and endpoint routing through `ITileService`.
**Complexity**: 2 points
**Dependencies**: AZ-314
**Component**: Docs
**Tracker**: AZ-315
**Epic**: AZ-309
## Problem
The architecture docs (`architecture.md`, `module-layout.md`, `components/03_tile_downloader/description.md`, etc.) describe the pre-refactor world: a single Services csproj and tile endpoints that bypass the service layer. After AZ-310 through AZ-314, the docs drift from reality and the architecture-baseline F3/F4 findings remain unchecked.
## Outcome
- `_docs/02_document/architecture.md` reflects the new project structure.
- `_docs/02_document/module-layout.md` lists the three new csprojs with their dependency graph.
- `_docs/02_document/components/03_tile_downloader/description.md` and the per-component module docs (`modules/services_*.md`) updated for the split.
- `_docs/02_document/architecture_compliance_baseline.md` Findings F3 and F4 marked **resolved** with a note linking to AZ-309 / this PR.
- Diagrams (`_docs/02_document/diagrams/components.md`) updated if they show the old single Services box.
## Scope
### Included
- Edits to the documents listed above.
- Cross-reference check: any other doc that names `SatelliteProvider.Services` is updated.
### Excluded
- Changing any code.
- Adding new tests or test docs.
## Acceptance Criteria
**AC-1: Architecture doc reflects the split**
Given `_docs/02_document/architecture.md`
When read after this task
Then it lists three Services csprojs and their dependency boundaries explicitly.
**AC-2: Module layout is accurate**
Given `_docs/02_document/module-layout.md`
When read
Then each new csproj has its own row with file list and `ProjectReference` set.
**AC-3: Compliance baseline closed for F3/F4**
Given `_docs/02_document/architecture_compliance_baseline.md`
When inspected
Then findings F3 and F4 are marked resolved with reference to AZ-309 epic and the post-refactor commit.
**AC-4: No stale references**
Given the post-refactor docs tree
When grepping for `SatelliteProvider.Services` (without `.TileDownloader`, `.RegionProcessing`, or `.RouteManagement` suffix)
Then matches are limited to historical context (e.g. retro / migration notes) — not to descriptions of current state.
## Constraints
- No new diagrams unless an existing one becomes wrong.
- No copy-edit polish beyond what the refactor requires.
## Risks & Mitigation
**Risk 1: Diagram drift**
- *Risk*: Mermaid diagrams may show "Services" as a single block.
- *Mitigation*: Find each diagram, update to show three blocks with arrows preserving the existing dependency direction.