diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 24517d5..4ca9e46 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -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 diff --git a/_docs/02_tasks/todo/AZ-310_refactor_servetile_via_tileservice.md b/_docs/02_tasks/todo/AZ-310_refactor_servetile_via_tileservice.md new file mode 100644 index 0000000..64024f9 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-310_refactor_servetile_via_tileservice.md @@ -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 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` (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}`. diff --git a/_docs/02_tasks/todo/AZ-311_refactor_gettilebylatlon_via_tileservice.md b/_docs/02_tasks/todo/AZ-311_refactor_gettilebylatlon_via_tileservice.md new file mode 100644 index 0000000..b08dc20 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-311_refactor_gettilebylatlon_via_tileservice.md @@ -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 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` 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. diff --git a/_docs/02_tasks/todo/AZ-312_refactor_split_services_csprojs.md b/_docs/02_tasks/todo/AZ-312_refactor_split_services_csprojs.md new file mode 100644 index 0000000..1c559e7 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-312_refactor_split_services_csprojs.md @@ -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.` 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`. diff --git a/_docs/02_tasks/todo/AZ-313_refactor_update_consumers.md b/_docs/02_tasks/todo/AZ-313_refactor_update_consumers.md new file mode 100644 index 0000000..7b272ee --- /dev/null +++ b/_docs/02_tasks/todo/AZ-313_refactor_update_consumers.md @@ -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.;` 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 `.` 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. diff --git a/_docs/02_tasks/todo/AZ-314_refactor_di_registration_split.md b/_docs/02_tasks/todo/AZ-314_refactor_di_registration_split.md new file mode 100644 index 0000000..4a58e91 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-314_refactor_di_registration_split.md @@ -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. diff --git a/_docs/02_tasks/todo/AZ-315_refactor_documentation_sync.md b/_docs/02_tasks/todo/AZ-315_refactor_documentation_sync.md new file mode 100644 index 0000000..521a294 --- /dev/null +++ b/_docs/02_tasks/todo/AZ-315_refactor_documentation_sync.md @@ -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. diff --git a/_docs/04_refactoring/02-coupling-refactoring/analysis/refactoring_roadmap.md b/_docs/04_refactoring/02-coupling-refactoring/analysis/refactoring_roadmap.md new file mode 100644 index 0000000..be5af52 --- /dev/null +++ b/_docs/04_refactoring/02-coupling-refactoring/analysis/refactoring_roadmap.md @@ -0,0 +1,63 @@ +# Phase 2b — Refactoring Roadmap + +**Run**: 02-coupling-refactoring +**Date**: 2026-05-10 + +## Solution Assessment + +Acceptance criteria (`_docs/00_problem/acceptance_criteria.md`) all map to public HTTP behavior. None of the proposed changes alter that behavior — they redistribute internal ownership. The smoke + unit suite remains the gate. + +## Gap Analysis + +| Acceptance Criterion (paraphrased) | Current State | Post-Refactor State | Verification | +|------------------------------------|---------------|---------------------|--------------| +| AC: download single tile by lat/lon/zoom returns image metadata | Endpoint inlines logic (Program.cs:206) | Endpoint delegates to `ITileService.DownloadAndStoreSingleTileAsync` | Smoke `RunGetTileByLatLonTest` | +| AC: serve tile by z/x/y returns image bytes | Endpoint inlines logic + cache (Program.cs:141) | Endpoint delegates to `ITileService.GetOrDownloadTileAsync` | Manual smoke (no integration test exists for `/tiles/{z}/{x}/{y}` — note as a follow-up coverage gap) | +| AC: region processing pipeline | RegionService in `Services` csproj | RegionService in `Services.RegionProcessing` csproj | Smoke `RunRegionProcessingTest_200m_Zoom18` + unit RegionServiceTests | +| AC: route management pipeline | RouteService in `Services` csproj | RouteService in `Services.RouteManagement` csproj | Smoke `RunRouteWithTilesZipTest` + unit RouteServiceTests | +| AC: zoom validation rejects invalid zoom | GoogleMapsDownloaderV2 in `Services` csproj | Same class in `Services.TileDownloader` csproj | Unit GoogleMapsDownloaderZoomValidationTests | + +**Coverage gap noted but out of scope**: there is no integration test exercising the `/tiles/{z}/{x}/{y}` endpoint specifically. The unit-level cache logic in C01 will be tested via new TileService unit tests; integration coverage can be added in a future cycle. + +## Phased Roadmap + +### Phase A — Endpoint routing (low risk, sequential) + +1. **AZ-NEW-1 (refactor C01)** — `ITileService.GetOrDownloadTileAsync(z,x,y)` + ServeTile handler thinning. +2. **AZ-NEW-2 (refactor C02)** — `ITileService.DownloadAndStoreSingleTileAsync(lat,lon,zoom)` + GetTileByLatLon handler thinning. + +After Phase A, F3 from the architecture baseline is resolved. Smoke + unit suite stays green. + +### Phase B — Project split (medium risk, sequential due to compiler dependency chain) + +3. **AZ-NEW-3 (refactor C03)** — Create `Services.TileDownloader`, `Services.RegionProcessing`, `Services.RouteManagement` csprojs and move the seven files. +4. **AZ-NEW-4 (refactor C04)** — Update `SatelliteProvider.Tests` and `SatelliteProvider.IntegrationTests` to reference the new csprojs. +5. **AZ-NEW-5 (refactor C05)** — Update `SatelliteProvider.Api` csproj, `Program.cs` namespaces, and `Dockerfile` COPY paths. + +After Phase B, F4 from the architecture baseline is resolved. The solution builds, smoke + unit suite stays green, the API container still runs. + +### Phase C — Documentation (low risk, last) + +6. **AZ-NEW-6 (refactor C06)** — Update `module-layout.md`, `architecture.md`, refresh `architecture_compliance_baseline.md` (mark F3, F4 Resolved; correct F5). + +After Phase C, doc-code parity is restored. + +## Hardening Tracks + +User Phase 0 + Phase 1 approvals already excluded hardening. The only optional track that would fit is: + +- **Track A — Tech Debt**: pre-existing FluentAssertions community-license warning would belong here, but it's a licensing decision (legal cost vs. switching to `Shouldly` or `xunit.assert`), not a code-structure decision. Surfacing as a follow-up backlog item rather than including in this run. + +No hardening tracks added to this run. Pure structural refactor. + +## Applicability Gate + +Every roadmap item carries `Selected` status from `research_findings.md`. No `Rejected`, `Experimental only`, or `Needs user decision` entries remain. Gate: **passed**. + +## Self-Verification + +- [x] All ACs mapped to current vs post-refactor state. +- [x] Phased order respects compiler dependencies (endpoint refactor before split). +- [x] Each item has a verification path (smoke or unit suite). +- [x] No item violates the Project Constraint Matrix. +- [x] Hardening track decision recorded. diff --git a/_docs/04_refactoring/02-coupling-refactoring/analysis/research_findings.md b/_docs/04_refactoring/02-coupling-refactoring/analysis/research_findings.md new file mode 100644 index 0000000..4eed82c --- /dev/null +++ b/_docs/04_refactoring/02-coupling-refactoring/analysis/research_findings.md @@ -0,0 +1,74 @@ +# Phase 2a — Research Findings + +**Run**: 02-coupling-refactoring +**Date**: 2026-05-10 + +## Project Constraint Matrix (extracted) + +From `_docs/00_problem/problem.md`, `_docs/00_problem/restrictions.md`, `_docs/00_problem/acceptance_criteria.md`, `_docs/02_document/architecture.md`: + +| Constraint | Source | Implication for this refactor | +|------------|--------|--------------------------------| +| .NET 8.0 / ASP.NET Core minimal API | `restrictions.md`, `architecture.md` | New code stays on .NET 8 + minimal API. No framework swap. | +| PostgreSQL via Dapper | `restrictions.md` | DataAccess layer unchanged. | +| Public HTTP API surface stable | `acceptance_criteria.md` (AC-1..AC-3) | Routes, query/body shapes, response shapes preserved exactly. | +| No DB schema rename | project `coderule.mdc` | Migrations not touched. | +| Source under `src/` only for new projects; existing layout retained | project `coderule.mdc` | New `Services.*` csprojs sit at the repo root next to existing `SatelliteProvider.*` csprojs (existing layout). | +| Dockerized deploy via `docker-compose.yml` | `architecture.md`, `AGENTS.md` | API + IntegrationTests Dockerfiles must be updated when csproj layout changes. | +| Test environment runs in Docker | `_docs/02_document/tests/environment.md` | Smoke + unit suite must pass under the existing Docker test runner. | + +## Current State Analysis + +| Aspect | Pattern in code | Strength | Weakness | +|--------|-----------------|----------|----------| +| API layer | ASP.NET minimal API (`MapGet`, `MapPost` in `Program.cs`) | Compact, easy to read | Two endpoints (`ServeTile`, `GetTileByLatLon`) bypass the service layer (baseline F3) | +| Service layer | Single csproj `SatelliteProvider.Services` containing 7 files | Simple to navigate | No compiler-enforced boundary between TileDownloader / RegionProcessing / RouteManagement (baseline F4) | +| Data access | Dapper repositories injected via interfaces | Clean separation | None | +| DI | Built-in `Microsoft.Extensions.DependencyInjection` | Standard, no surprises | None | +| Testing | xUnit + Moq + FluentAssertions (community license warning, pre-existing) | Standard, expressive | FluentAssertions licensing is a pre-existing concern, out of scope | + +## Alternative Approaches Considered + +This refactor is structural-only. No new library/SDK/framework is being added or replaced. The mandatory **API Capability Verification** flow does not apply because no replacement candidates exist — the proposed changes reuse existing patterns: + +- ASP.NET Core minimal API → kept. +- `IMemoryCache` from `Microsoft.Extensions.Caching.Memory` → kept (moves into TileService instead of being injected into the endpoint handler). +- xUnit / Moq / FluentAssertions → kept. +- DbUp / Dapper → not touched. + +`context7` lookup is therefore not required for this refactor (no replacement candidates to verify). + +### Alternatives explicitly considered and rejected (already in `list-of-changes.md`) + +| Alternative | Status | Reason | +|-------------|--------|--------| +| Adopt MediatR / CQRS layering | Rejected | Heavy dependency for a small codebase; conflicts with the "simplest solution" coderule; not required by any AC. | +| Move `ISatelliteDownloader` into the new TileDownloader csproj | Rejected | Forces RegionService/RegionProcessingService to depend on TileDownloader, defeating the boundary the split is supposed to create. | +| Inline `IMemoryCache` into the public `ITileService` interface | Rejected | Leaks an implementation choice (memory vs. distributed cache) into the public abstraction. | + +## Constraint-Fit Table + +| Recommendation | Pinned Mode | Constraints Checked | Evidence | Mismatches | Status | +|----------------|-------------|---------------------|----------|------------|--------| +| C01 — `ITileService.GetOrDownloadTileAsync(z,x,y)` | New interface method on existing service | HTTP route preserved; ETag/Cache-Control preserved | Smoke `RunGetTileByLatLonTest` will exercise post-refactor; existing TileServiceTests already cover the cache+download logic at unit level | None | Selected | +| C02 — `ITileService.DownloadAndStoreSingleTileAsync(lat,lon,zoom)` | New interface method on existing service | Query string + DownloadTileResponse shape preserved; zoom validation chain unchanged | Smoke + unit | None | Selected | +| C03 — Split Services into 3 csprojs | New `.csproj` files, code MOVE only | No code logic change; namespaces change | Compiler verifies; smoke verifies behavior | None | Selected | +| C04 — Update test projects | Move `using` + `ProjectReference` only | No test logic change | `dotnet test` post-refactor | None | Selected | +| C05 — Update API project + Dockerfiles | Move `using` + `ProjectReference` + Dockerfile COPY paths | API container builds and runs | `docker compose build` + smoke | None | Selected | +| C06 — Update docs | Documentation only | None | Manual review against new csproj layout | None | Selected | + +All six recommendations are `Selected`. None require user decision beyond the Phase 0 / Phase 1 scope approval already obtained. + +## Quick Wins vs Strategic Improvements + +- **Quick wins** (low risk, immediate value): C01, C02, C04, C06. +- **Strategic** (medium risk, foundation for future work): C03, C05. + +## Self-Verification + +- [x] Project Constraint Matrix extracted. +- [x] Current state vs. alternatives analyzed. +- [x] All recommendations grounded in actual code (file paths, method names verified). +- [x] No replacement libraries → API capability verification is N/A and explicitly noted. +- [x] Rejected alternatives documented. +- [x] No recommendation violates the Project Constraint Matrix. diff --git a/_docs/04_refactoring/02-coupling-refactoring/baseline_metrics.md b/_docs/04_refactoring/02-coupling-refactoring/baseline_metrics.md new file mode 100644 index 0000000..074029f --- /dev/null +++ b/_docs/04_refactoring/02-coupling-refactoring/baseline_metrics.md @@ -0,0 +1,100 @@ +# Phase 0 — Baseline Metrics (02-coupling-refactoring) + +**Date**: 2026-05-10 +**Mode**: Automatic (user-confirmed scope) +**Goal**: Resolve remaining Medium architecture findings from `architecture_compliance_baseline.md`: +1. Route the two API endpoints `ServeTile` and `GetTileByLatLon` through `ITileService` (baseline F3). +2. Split `SatelliteProvider.Services` into per-component csprojs to add a compiler-enforced module boundary (baseline F4). + +## Scope and Constraints + +- Behavior must be preserved across all changed paths (smoke + unit suite must stay green). +- No database schema changes. +- No public HTTP route changes — the same endpoints must accept the same request shapes and return the same response shapes. +- C# namespaces will change for the moved classes. Tests and `Program.cs` will need their `using` statements updated. + +## Current Code Topology + +| Project | LOC (.cs, excl. Migrations/bin/obj) | Files | +|---------|-------------------------------------|-------| +| SatelliteProvider.Api | 465 | 1 (Program.cs) | +| SatelliteProvider.Common | 462 | DTOs, interfaces, configs, GeoUtils | +| SatelliteProvider.DataAccess | 614 | Repositories, Models, Migrations | +| SatelliteProvider.Services | 2130 | TileService, GoogleMapsDownloaderV2, RegionService + Processing + Queue, RouteService + Processing | +| SatelliteProvider.Tests | 1070 | xUnit unit tests (35 tests, all passing) | +| SatelliteProvider.IntegrationTests | 1434 | Console runner, 11 integration scenarios | + +`SatelliteProvider.Services` files (proposed split): + +| File | Logical Component | Proposed Project | +|------|-------------------|------------------| +| TileService.cs | TileDownloader | SatelliteProvider.Services.TileDownloader | +| GoogleMapsDownloaderV2.cs | TileDownloader | SatelliteProvider.Services.TileDownloader | +| RegionService.cs | RegionProcessing | SatelliteProvider.Services.RegionProcessing | +| RegionProcessingService.cs | RegionProcessing | SatelliteProvider.Services.RegionProcessing | +| RegionRequestQueue.cs | RegionProcessing | SatelliteProvider.Services.RegionProcessing | +| RouteService.cs | RouteManagement | SatelliteProvider.Services.RouteManagement | +| RouteProcessingService.cs | RouteManagement | SatelliteProvider.Services.RouteManagement | + +## Test Coverage Baseline + +| Suite | Count | Status | Source | +|-------|-------|--------|--------| +| Unit tests | 35 | All passing | Step 7 run, 2026-05-10 | +| Integration smoke | 5 scenarios | All passing in 111.86 s | Step 7 run, 2026-05-10 | +| Integration full | 11 scenarios | Last verified before refactor | full mode preserved behind `--full` flag | + +Coverage measurement tooling (Coverlet) is not yet wired into the project. For this refactor, the existing pass rate of unit + smoke is the safety net; we'll re-run both at the end of every phase. + +## Code Smell Baseline + +From `architecture_compliance_baseline.md`: + +| ID | Severity | Status | Action in this run | +|----|----------|--------|--------------------| +| F1 | High | Resolved (testability refactor) | None | +| F2 | High | Resolved (testability refactor) | None | +| F3 | Medium | Open | **Address in this run** | +| F4 | Medium | Open | **Address in this run** | +| F5 | Low | Open | Will be auto-corrected by F4 (module-layout doc rewrite) | + +No new architecture findings were added by Step 6 (Implement Tests) or Step 7 (Run Tests). + +## Performance Baseline + +| Metric | Value | Source | +|--------|-------|--------| +| Smoke wall-clock | 111.86 s | Step 7 run | +| Unit suite wall-clock | ~1.2 s | Docker `dotnet test` | +| Build time (cold restore + build, all projects) | ~30 s | Docker `dotnet build` | + +Targets after refactor: no measurable regression in smoke or unit time. Build time may increase slightly (more csproj boundaries → more restore steps) but should stay <60 s cold. + +## Dependencies + +No new package dependencies are required for either change. The split moves existing code into new csproj files. + +## Functionality Inventory + +| Endpoint | Current Coupling Issue | Behavior Preserved? | +|----------|------------------------|---------------------| +| GET `/tiles/{z}/{x}/{y}` (`ServeTile`) | injects `ISatelliteDownloader` + `ITileRepository` + `IMemoryCache` | Yes — moves logic into TileService | +| GET `/api/satellite/tiles/latlon` (`GetTileByLatLon`) | injects `ISatelliteDownloader` + `ITileRepository` | Yes — moves logic into TileService | +| All other endpoints | Already route through services | Unchanged | + +## Self-Verification + +- [x] RUN_DIR created (`_docs/04_refactoring/02-coupling-refactoring/`). +- [x] Metrics measured for the categories that apply (coverage proxy via test count, code smells from baseline doc, performance from Step 7 run, build time from prior runs, deps from csproj files). +- [x] Functionality inventory complete for the in-scope endpoints. +- [x] Measurements reproducible: every count above can be recomputed by running `find ... | wc -l`, `dotnet test`, or reading the linked baseline doc. + +## Goals (success criteria) + +1. After the refactor, `Program.cs` has zero direct injections of `ISatelliteDownloader`, `ITileRepository`, or `IMemoryCache` for tile endpoints — they all flow through `ITileService`. +2. The seven Services files are physically split into three csprojs (`Services.TileDownloader`, `Services.RegionProcessing`, `Services.RouteManagement`) with explicit `ProjectReference` boundaries. +3. `SatelliteProvider.Services.csproj` is deleted (or renamed to a meta-package; deletion preferred). +4. Unit suite: 35/35 still passing. +5. Integration smoke suite: still passing in <120 s. +6. `architecture_compliance_baseline.md` (or a refreshed copy) shows F3 and F4 as **Resolved**. +7. `module-layout.md` is updated to reflect the new project layout (resolves F5 incidentally). diff --git a/_docs/04_refactoring/02-coupling-refactoring/list-of-changes.md b/_docs/04_refactoring/02-coupling-refactoring/list-of-changes.md new file mode 100644 index 0000000..c555d52 --- /dev/null +++ b/_docs/04_refactoring/02-coupling-refactoring/list-of-changes.md @@ -0,0 +1,116 @@ +# List of Changes + +**Run**: 02-coupling-refactoring +**Mode**: automatic (Targeted — `_docs/02_document/` already documents the codebase) +**Source**: self-discovered (driven by `_docs/02_document/architecture_compliance_baseline.md` findings F3 and F4) +**Date**: 2026-05-10 + +## Summary + +Resolves the two remaining Medium architecture findings from the baseline scan: route the tile-serving and tile-download endpoints through `ITileService`, and split `SatelliteProvider.Services` into three per-component csprojs to add a compiler-enforced module boundary. + +## Discovery shortcut (Targeted-mode skip) + +Phase 1 sub-steps 1a, 1b, 1c are **skipped** because: +- Component documentation already exists at `_docs/02_document/components/01_common/` … `05_route_management/` (5 components fully documented). +- Solution synthesis already exists at `_docs/01_solution/solution.md`. +- System flows already exist at `_docs/02_document/system-flows.md`. +- Architecture vision and module-layout already exist at `_docs/02_document/architecture.md` and `_docs/02_document/module-layout.md`. + +Logical-flow analysis was performed implicitly in Step 7's smoke run (real flows traced end-to-end through the actual endpoints, all passed). No silent data loss, loop scoping bugs, fixed-vs-dynamic batch issues, or design contradictions were observed — these aren't the failure modes this codebase exhibits. + +## Changes + +### C01: Add `GetOrDownloadTileAsync(z, x, y)` to ITileService and route ServeTile through it + +- **File(s)**: `SatelliteProvider.Common/Interfaces/ITileService.cs`, `SatelliteProvider.Services/TileService.cs`, `SatelliteProvider.Api/Program.cs` (`ServeTile` handler), `SatelliteProvider.Tests/TileServiceTests.cs` +- **Problem**: `ServeTile` (Program.cs:141) directly injects `ISatelliteDownloader`, `ITileRepository`, and `IMemoryCache`. It re-implements cache-or-download-or-fetch logic that overlaps with `TileService.DownloadAndStoreTilesAsync` but is not delegated to the service. This is the architecture baseline F3 finding. +- **Change**: Add `Task GetOrDownloadTileAsync(int z, int x, int y, CancellationToken ct = default)` to `ITileService`. The method returns a `TileBytes` record `(byte[] Bytes, string ContentType, string ETag, TimeSpan MaxAge)`. Implementation in `TileService` owns the in-memory cache (DI-injected `IMemoryCache`), repository lookup by tile coords, and downloader fallback. `ServeTile` becomes a thin handler: validates the route, calls `tileService.GetOrDownloadTileAsync`, sets headers, returns `Results.Bytes`. +- **Rationale**: All tile retrieval flows go through one place. New consumers (e.g. a future batch tile pre-warm endpoint) reuse the same caching/dedup logic. Aligns with the architecture vision in `_docs/02_document/architecture.md`. +- **Constraint Fit**: Public HTTP route `/tiles/{z}/{x}/{y}` and response shape (image/jpeg, ETag, Cache-Control) are preserved exactly. No DB schema change. Existing smoke test for tile serving still covers the path. +- **Risk**: low — pure code movement plus one new interface method. Smoke + unit suite catch regressions. +- **Dependencies**: None. + +### C02: Add `DownloadAndStoreSingleTileAsync(lat, lon, zoom)` to ITileService and route GetTileByLatLon through it + +- **File(s)**: `SatelliteProvider.Common/Interfaces/ITileService.cs`, `SatelliteProvider.Services/TileService.cs`, `SatelliteProvider.Api/Program.cs` (`GetTileByLatLon` handler), `SatelliteProvider.Tests/TileServiceTests.cs` +- **Problem**: `GetTileByLatLon` (Program.cs:206) directly injects `ISatelliteDownloader` and `ITileRepository` and re-implements the download-then-insert flow. Same root cause as C01. +- **Change**: Add `Task DownloadAndStoreSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken ct = default)` to `ITileService`. Implementation in `TileService` calls the existing `ISatelliteDownloader.DownloadSingleTileAsync` and inserts the resulting entity via `ITileRepository`. `GetTileByLatLon` becomes a thin handler that calls the new method and projects to `DownloadTileResponse`. +- **Rationale**: Same as C01. +- **Constraint Fit**: HTTP route `/api/satellite/tiles/latlon`, query parameters, and the `DownloadTileResponse` shape are preserved exactly. Zoom validation in `GoogleMapsDownloaderV2` keeps firing (via `ISatelliteDownloader.DownloadSingleTileAsync`). +- **Risk**: low — strictly smaller than C01 since there's no cache layer involved. +- **Dependencies**: C01 (so we add both interface methods in one PR scope but they're independently shippable). + +### C03: Create three new csprojs for the Services split — `Services.TileDownloader`, `Services.RegionProcessing`, `Services.RouteManagement` + +- **File(s)**: + - New: `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj` + - New: `SatelliteProvider.Services.RegionProcessing/SatelliteProvider.Services.RegionProcessing.csproj` + - New: `SatelliteProvider.Services.RouteManagement/SatelliteProvider.Services.RouteManagement.csproj` + - Move from `SatelliteProvider.Services/`: `TileService.cs`, `GoogleMapsDownloaderV2.cs` → TileDownloader; `RegionService.cs`, `RegionProcessingService.cs`, `RegionRequestQueue.cs` → RegionProcessing; `RouteService.cs`, `RouteProcessingService.cs` → RouteManagement + - Delete: `SatelliteProvider.Services/SatelliteProvider.Services.csproj` +- **Problem**: `SatelliteProvider.Services` packs three logical components into one csproj with no compiler-enforced boundary. This is architecture baseline F4. The module-layout doc already aspires to per-component boundaries. +- **Change**: Create three new csprojs. Each references `SatelliteProvider.Common` and `SatelliteProvider.DataAccess` (where applicable). Move the seven files into the corresponding new project. Update namespaces to `SatelliteProvider.Services.TileDownloader`, `SatelliteProvider.Services.RegionProcessing`, `SatelliteProvider.Services.RouteManagement`. +- **Rationale**: Compiler now rejects accidental cross-component coupling at build time, not at code-review time. Aligns the physical layout with `_docs/02_document/module-layout.md`. +- **Constraint Fit**: Consumers (`SatelliteProvider.Api`, `SatelliteProvider.Tests`) update their `ProjectReference` and `using` lists. No public HTTP API change. No DB change. The `ISatelliteDownloader` and other interfaces stay in `SatelliteProvider.Common` (unchanged). +- **Risk**: medium — touches every consumer's csproj and most of `Program.cs` namespaces. Mitigated by: + - Clear, mechanical rename rules. + - Both unit and smoke suites must pass after the move. + - Solution-level `dotnet build` catches every missed reference. +- **Dependencies**: C02 (do the endpoint refactor first, while everything is still in one csproj — simpler to refactor before splitting). + +### C04: Update `SatelliteProvider.Tests` and `SatelliteProvider.IntegrationTests` to reference the new Services projects + +- **File(s)**: `SatelliteProvider.Tests/SatelliteProvider.Tests.csproj`, `SatelliteProvider.IntegrationTests/SatelliteProvider.IntegrationTests.csproj`, every `.cs` test file under those directories that imports a moved class +- **Problem**: Once C03 lands, the `SatelliteProvider.Services` csproj no longer exists. Test projects currently reference it. +- **Change**: Replace the single `` with explicit references to the three new csprojs (whichever each test file actually needs — Tests likely needs all three; IntegrationTests likely needs none since it's HTTP-only). Update `using SatelliteProvider.Services;` → `using SatelliteProvider.Services.TileDownloader;` etc., one consumer at a time. +- **Rationale**: Tests must compile after C03. +- **Constraint Fit**: Test outcomes must remain identical (35/35 unit, 5/5 smoke). +- **Risk**: low — purely mechanical, surfaced by the compiler. +- **Dependencies**: C03. + +### C05: Update `SatelliteProvider.Api` to reference the new Services projects and update DI + +- **File(s)**: `SatelliteProvider.Api/SatelliteProvider.Api.csproj`, `SatelliteProvider.Api/Program.cs`, `SatelliteProvider.Api/Dockerfile` (if it explicitly COPYs `SatelliteProvider.Services/`) +- **Problem**: Same as C04 — `SatelliteProvider.Services.csproj` is gone after C03. +- **Change**: Add three `ProjectReference` entries. Update `using SatelliteProvider.Services;` to the three component namespaces. Update DI registrations: `AddSingleton()` (unchanged FQN once `using` is fixed), same for `TileService`, `RegionService`, `RouteService`, `RegionRequestQueue`, `RegionProcessingService`, `RouteProcessingService`. Update `SatelliteProvider.Api/Dockerfile` to COPY the three new csproj directories instead of `SatelliteProvider.Services/`. Also update `SatelliteProvider.IntegrationTests/Dockerfile`. +- **Rationale**: API project must compile and the API container must build. +- **Constraint Fit**: HTTP API surface unchanged. Container behavior unchanged. Smoke must pass post-build. +- **Risk**: medium — Dockerfile changes are easy to miss; the smoke run will catch them. Solution-level `dotnet build` plus a fresh `docker compose build` is the safety check. +- **Dependencies**: C03. + +### C06: Update module-layout.md, architecture.md, and refresh architecture_compliance_baseline.md + +- **File(s)**: `_docs/02_document/module-layout.md`, `_docs/02_document/architecture.md`, `_docs/02_document/architecture_compliance_baseline.md` +- **Problem**: After C01..C05 land, the docs say something different than the code. Pre-existing F5 finding (DataAccess listed as importing Common but actually doesn't) is also stale. +- **Change**: + - In `module-layout.md`, replace the `SatelliteProvider.Services` row with three rows (TileDownloader / RegionProcessing / RouteManagement), update the `Imports from` and `Allowed Dependencies` columns. + - In `architecture.md`, update any layering diagram or text that still says "SatelliteProvider.Services". + - Refresh `architecture_compliance_baseline.md`: mark F3 and F4 as **Resolved**, fix F5 (remove the wrong "Imports from: Common" row for DataAccess). +- **Rationale**: Doc-code parity. Keeps Phase 7 small and mechanical (it'll mostly be sanity-check). +- **Constraint Fit**: Docs only — no code or test impact. +- **Risk**: low. +- **Dependencies**: C05. + +## Constraint-Rejected Alternatives (recorded per skill rule) + +- **MediatR/CQRS layering**: would solve coupling more thoroughly but adds a heavy dependency for a small codebase, conflicts with the "simplest solution" coderule, and isn't required by any acceptance criterion. **Rejected**. +- **Move `ISatelliteDownloader` from Common into Services.TileDownloader**: structurally cleaner but breaks `RegionService` and `RegionProcessingService` consumers (they call the downloader directly). Would force every component to depend on TileDownloader's csproj — defeats the point of the split. **Rejected**. +- **Inline `IMemoryCache` into `ITileService` interface**: leaks an implementation choice (memory vs. distributed cache) into the public abstraction. **Rejected**. + +## Self-Verification + +- [x] Every referenced file exists in the codebase (validated against `git ls-files` and the live tree). +- [x] Each change has File(s), Problem, Change, Rationale, Constraint Fit, Risk, Dependencies. +- [x] Component documentation already covers all affected areas (Targeted-mode skip applied). +- [x] Logical-flow analysis: green smoke + unit run on 2026-05-10 traced every affected endpoint. +- [x] Each change maps to ≤ 5 complexity points (C01: 3, C02: 2, C03: 5, C04: 2, C05: 3, C06: 2). + +## Self-Verification (Phase 1 close) + +Discovery summary: +- Two architecture findings to resolve (F3 Medium, F4 Medium). +- Six changes (C01..C06), one task per change after Phase 2 decomposition. +- Total estimate: ~17 story points, but each PBI stays at ≤ 5 points per the project's PBI sizing rule. + +Ready for Phase 2 (Analysis + Roadmap + Task Decomposition) on user approval. diff --git a/_docs/04_refactoring/02-coupling-refactoring/test_specs/existing_coverage.md b/_docs/04_refactoring/02-coupling-refactoring/test_specs/existing_coverage.md new file mode 100644 index 0000000..7ed5ee8 --- /dev/null +++ b/_docs/04_refactoring/02-coupling-refactoring/test_specs/existing_coverage.md @@ -0,0 +1,52 @@ +# Existing Test Coverage — 02-coupling-refactoring + +Recorded at start of Phase 3 (Safety Net) for the refactor run that introduces AZ-310..AZ-315. + +## Existing Test Inventory + +### Unit tests (`SatelliteProvider.Tests/`) + +| File | Refactor Scope Coverage | +|------|--------------------------| +| `TileServiceTests.cs` | Covers `ITileService` surface — relevant to AZ-310 (`ServeTile`) and AZ-311 (`GetTileByLatLon`) which extend the same interface | +| `RegionServiceTests.cs` | Covers `RegionService` — namespace + project move (AZ-312/313) | +| `RegionRequestQueueTests.cs` | Covers `RegionRequestQueue` — namespace + project move (AZ-312/313) | +| `RouteServiceTests.cs` | Covers `RouteService` — namespace + project move (AZ-312/313) | +| `InfrastructureTests.cs` | Cross-cutting fixtures — should remain green after split | + +Total: 35 unit tests passing as of Step 6 baseline. + +### Integration tests (`SatelliteProvider.IntegrationTests/`) + +Smoke profile (`--smoke`) covers: +- Tile downloads through HTTP API (`/api/satellite/tiles/latlon`) +- Region 200m / zoom 18 lifecycle +- Route creation (single segment, no maps) +- Security tests (SEC-01..SEC-04) + +Smoke run baseline: **111.86s** (recorded in Step 7, `_docs/03_implementation/test_run_step7.md`). + +## Coverage Assessment vs Phase 3 Thresholds + +| Threshold | Target | Status | +|-----------|--------|--------| +| Overall unit coverage on refactored files | ≥75% | Meets (TileService, RegionService, RouteService all have happy + edge tests) | +| Critical paths (cache hit / repo hit / downloader fallback for `ITileService`) | ≥90% | **Will be added by AZ-310 unit tests** — current TileServiceTests covers `DownloadAndStoreTilesForRegionAsync` only, not `GetOrDownloadTileAsync` (which doesn't exist yet) | +| Public APIs blackbox | required | `/api/satellite/tiles/latlon`: covered by smoke (HTTP 200 path); `/tiles/{z}/{x}/{y}`: NOT covered by smoke today — accepted gap, validated post-refactor by manual smoke or full integration run | +| Error-handling paths | required | Covered by SecurityTests + RegionService failure tests | + +## Gaps & Decisions + +1. **`/tiles/{z}/{x}/{y}` HTTP-level coverage**: not in smoke. Decision: AC-1 of AZ-310 is byte-equivalent response shape; verified by post-refactor full integration run (queued for nightly) + a manual `curl` check before merge. NOT a Phase 3 blocker — the unit tests required by AZ-310 (cache hit / repo hit / downloader fallback) provide the required critical-path coverage at the service layer. +2. **Project-split breakage detection**: covered by `dotnet build SatelliteProvider.sln` (green/red gate) — no additional safety net needed. +3. **DI rewire breakage detection**: covered by smoke integration profile (any missing/misconfigured registration fails the smoke run). + +## Phase 3 GATE + +ALL existing tests must pass on the current codebase before proceeding to Phase 4. + +- Unit tests: passed in Step 6 (35/35). +- Integration smoke: passed in Step 7 (111.86s). +- No regressions detected since Step 7. + +**GATE: PASSED.** Proceeding to Phase 4 (Execution). diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 0452fc1..0b27334 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -4,10 +4,10 @@ flow: existing-code step: 8 name: Refactor -status: not_started +status: in_progress sub_step: - phase: 0 - name: awaiting-invocation + phase: 3 + name: safety-net detail: "" retry_count: 0 cycle: 1