diff --git a/_docs/02_document/architecture.md b/_docs/02_document/architecture.md index f3a1e10..1f6621f 100644 --- a/_docs/02_document/architecture.md +++ b/_docs/02_document/architecture.md @@ -4,12 +4,14 @@ Satellite Provider is a self-hosted .NET 8.0 backend service that pre-downloads, caches, and composites Google Maps satellite imagery for offline use. It runs as a single containerized monolith with PostgreSQL, processing requests asynchronously via in-process queues. The dominant pattern is a layered architecture (API → Services → DataAccess → PostgreSQL) with background hosted services for long-running work. -**Components & responsibilities**: -- **Common** — Shared contracts: DTOs, service interfaces, configuration models, geospatial math -- **DataAccess** — PostgreSQL persistence via Dapper + DbUp migrations -- **TileDownloader** — Provider-agnostic tile acquisition via `ISatelliteDownloader` interface (first implementation: Google Maps) with deduplication and concurrency control -- **RegionProcessing** — Batch tile downloads for geographic areas, stitching, output generation -- **RouteManagement** — Route interpolation, geofenced region generation, consolidated map output +**Components & responsibilities** (each owns its own `.csproj` since AZ-309): +- **Common** (`SatelliteProvider.Common`) — Shared contracts: DTOs, service interfaces, common exceptions, configuration models, geospatial math +- **DataAccess** (`SatelliteProvider.DataAccess`) — PostgreSQL persistence via Dapper + DbUp migrations +- **TileDownloader** (`SatelliteProvider.Services.TileDownloader`) — Provider-agnostic tile acquisition via `ISatelliteDownloader` interface (first implementation: Google Maps) with deduplication, concurrency control, and an in-memory tile-byte cache owned by `TileService` +- **RegionProcessing** (`SatelliteProvider.Services.RegionProcessing`) — Batch tile downloads for geographic areas, stitching, output generation +- **RouteManagement** (`SatelliteProvider.Services.RouteManagement`) — Route interpolation, geofenced region generation, consolidated map output + +The three Layer-3 service components are compile-time siblings: each only references `SatelliteProvider.Common` and `SatelliteProvider.DataAccess`. Cross-component runtime calls flow exclusively through interfaces in `SatelliteProvider.Common.Interfaces`. **Major data flows**: - *Tile acquisition*: HTTP request → cache check → Google Maps download → disk + DB persistence @@ -104,11 +106,11 @@ Satellite Provider is a self-hosted .NET 8.0 backend service that pre-downloads, | From | To | Protocol | Pattern | Notes | |------|----|----------|---------|-------| -| WebApi | RegionProcessing | In-process queue (Channel) | Fire-and-forget | Request queued, status polled | -| WebApi | TileDownloader | Direct method call | Request-Response | Synchronous single-tile download | -| RegionProcessing | TileDownloader | Direct method call | Request-Response | Per-tile within region processing | -| RouteManagement | RegionProcessing | In-process queue (Channel) | Fire-and-forget | Route regions submitted to queue | -| All Services | DataAccess | Direct method call | Repository pattern | Dapper queries | +| WebApi | RegionProcessing | In-process queue (Channel) | Fire-and-forget | Request queued, status polled. Uses `IRegionService` / `IRegionRequestQueue` from Common. | +| WebApi | TileDownloader | `ITileService` (Common interface) | Request-Response | Single-tile reads (`GetOrDownloadTileAsync`) and writes (`DownloadAndStoreSingleTileAsync`) flow through `ITileService` since AZ-310 / AZ-311. No direct dependency on the concrete `GoogleMapsDownloaderV2`. | +| RegionProcessing | TileDownloader | `ITileService` (Common interface) | Request-Response | Per-tile within region processing. Resolved through DI; no compile-time `ProjectReference` between RegionProcessing and TileDownloader csprojs. | +| RouteManagement | RegionProcessing | `IRegionService` / `IRegionRequestQueue` (Common interfaces) | Fire-and-forget | Route regions submitted to queue. No compile-time `ProjectReference` between RouteManagement and RegionProcessing csprojs. | +| All Services | DataAccess | Direct method call (via repository interfaces) | Repository pattern | Dapper queries | ### External Integrations diff --git a/_docs/02_document/architecture_compliance_baseline.md b/_docs/02_document/architecture_compliance_baseline.md index 72e46fd..25da14a 100644 --- a/_docs/02_document/architecture_compliance_baseline.md +++ b/_docs/02_document/architecture_compliance_baseline.md @@ -3,17 +3,17 @@ **Date**: 2026-05-10 **Mode**: Baseline (Phase 1 + Phase 7) **Scope**: Full existing codebase -**Verdict**: PASS_WITH_WARNINGS +**Verdict**: PASS_WITH_WARNINGS (baseline) → all findings resolved by epic AZ-309 ## Findings -| # | Severity | Category | File:Line | Title | -|---|----------|----------|-----------|-------| -| 1 | High | Architecture | SatelliteProvider.Services/TileService.cs:11 | Concrete dependency on GoogleMapsDownloaderV2 bypasses ISatelliteDownloader | -| 2 | High | Architecture | SatelliteProvider.Common/Interfaces/ISatelliteDownloader.cs | ISatelliteDownloader interface is dead code | -| 3 | Medium | Architecture | SatelliteProvider.Api/Program.cs:141 | API endpoint directly injects concrete downloader + repository | -| 4 | Medium | Architecture | SatelliteProvider.Services/ | No physical boundary between logical components in shared project | -| 5 | Low | Architecture | module-layout.md | DataAccess documented as importing Common but actually has zero cross-project dependencies | +| # | Severity | Category | File:Line | Title | Status | +|---|----------|----------|-----------|-------|--------| +| 1 | High | Architecture | SatelliteProvider.Services/TileService.cs:11 | Concrete dependency on GoogleMapsDownloaderV2 bypasses ISatelliteDownloader | Resolved (pre-AZ-309 cleanup) | +| 2 | High | Architecture | SatelliteProvider.Common/Interfaces/ISatelliteDownloader.cs | ISatelliteDownloader interface is dead code | Resolved (pre-AZ-309 cleanup) | +| 3 | Medium | Architecture | SatelliteProvider.Api/Program.cs:141 | API endpoint directly injects concrete downloader + repository | **Resolved by AZ-310 + AZ-311** (commit 8b0ddae's parent) | +| 4 | Medium | Architecture | SatelliteProvider.Services/ | No physical boundary between logical components in shared project | **Resolved by AZ-312 + AZ-313 + AZ-314** (commit `8b0ddae`) | +| 5 | Low | Architecture | module-layout.md | DataAccess documented as importing Common but actually has zero cross-project dependencies | **Resolved by AZ-315** (this commit) — module-layout.md now reflects the actual no-import layout | ## Finding Details @@ -29,31 +29,44 @@ - Impact: The provider-agnostic abstraction doesn't function. Interface and implementation have diverged. - Suggestion: Update `ISatelliteDownloader` to match the actual API surface needed by consumers, then implement it in `GoogleMapsDownloaderV2`. -**F3: API endpoint bypasses service layer** (Medium / Architecture) +**F3: API endpoint bypasses service layer** (Medium / Architecture) — **RESOLVED** - Location: `SatelliteProvider.Api/Program.cs:141` (`ServeTile`) and `:206` (`GetTileByLatLon`) - Description: Two API endpoints directly inject `GoogleMapsDownloaderV2` and `ITileRepository` instead of using `ITileService`. This bypasses the service layer and creates a shortcut from Layer 4 to Layer 2. - Impact: Business logic (caching, dedup) in TileService is bypassed for these endpoints; tile download logic is duplicated. - Suggestion: Route all tile operations through `ITileService`. +- **Resolution (AZ-310 + AZ-311)**: `ITileService` extended with `GetOrDownloadTileAsync` and `DownloadAndStoreSingleTileAsync`; both endpoints now inject only `ITileService`. Caching (IMemoryCache), repository lookup, and downloader fallback consolidated inside TileService. Verified by 5 new unit tests + smoke integration. -**F4: No physical boundary in Services project** (Medium / Architecture) +**F4: No physical boundary in Services project** (Medium / Architecture) — **RESOLVED** - Location: `SatelliteProvider.Services/` (all files) - Description: Three logical components (TileDownloader, RegionProcessing, RouteManagement) share one `.csproj`. No compiler-enforced boundary prevents direct cross-component coupling. - Impact: Over time, services may accumulate hidden coupling that's hard to detect without code review. - Suggestion: Accept as-is for current scale; consider splitting into separate projects if the codebase grows significantly. +- **Resolution (AZ-312 + AZ-313 + AZ-314, commit `8b0ddae`)**: Project split into `SatelliteProvider.Services.TileDownloader`, `SatelliteProvider.Services.RegionProcessing`, `SatelliteProvider.Services.RouteManagement`. None of the three references another sibling — cross-sibling calls now flow exclusively through interfaces in `SatelliteProvider.Common.Interfaces`. `RateLimitException` relocated to `SatelliteProvider.Common.Exceptions` to keep the sibling boundary clean. Per-component DI extension methods (`AddTileDownloader`, `AddRegionProcessing`, `AddRouteManagement`) registered from `Program.cs`. Verified by 0 build errors, 40/40 unit tests, and full smoke integration suite passing post-split. -**F5: module-layout.md incorrect — DataAccess has no Common dependency** (Low / Architecture) +**F5: module-layout.md incorrect — DataAccess has no Common dependency** (Low / Architecture) — **RESOLVED** - Location: `_docs/02_document/module-layout.md` - Description: DataAccess was documented as "Imports from: Common" but `SatelliteProvider.DataAccess.csproj` has no ProjectReference to Common and no `using SatelliteProvider.Common` in any file. - Impact: Documentation inaccuracy; no code impact. - Suggestion: Correct module-layout.md. +- **Resolution (AZ-315)**: `module-layout.md` now lists DataAccess Imports from: (none); the §Verification section calls out the no-Common-dependency invariant explicitly. ## Summary -| Severity | Count | -|----------|-------| -| Critical | 0 | -| High | 2 | -| Medium | 2 | -| Low | 1 | +| Severity | Count (baseline) | Count (post AZ-309) | +|----------|------------------|---------------------| +| Critical | 0 | 0 | +| High | 2 | 0 | +| Medium | 2 | 0 | +| Low | 1 | 0 | The two High findings both relate to the same root cause: the `ISatelliteDownloader` abstraction was created but never wired into the system. The concrete `GoogleMapsDownloaderV2` is used directly everywhere. This is the primary architecture gap — addressing it would enable the provider-agnostic design the system intends to have. + +## Post-AZ-309 Status + +Epic AZ-309 (architecture coupling refactor) closes all five baseline findings: +- F1, F2 — pre-AZ-309 cleanup (interface implementation + DI rewire). +- F3 — AZ-310 + AZ-311 (route tile endpoints through `ITileService`). +- F4 — AZ-312 + AZ-313 + AZ-314 (split monolithic Services csproj into three per-component csprojs with compiler-enforced sibling boundary). +- F5 — AZ-315 (this commit; documentation now matches actual ProjectReference graph). + +No new Architecture findings were introduced by the refactor. The baseline is now clean. diff --git a/_docs/02_document/components/03_tile_downloader/description.md b/_docs/02_document/components/03_tile_downloader/description.md index 7ea11bc..a4f9f0b 100644 --- a/_docs/02_document/components/03_tile_downloader/description.md +++ b/_docs/02_document/components/03_tile_downloader/description.md @@ -6,9 +6,11 @@ **Architectural Pattern**: Service + Gateway (wraps external API with retry/throttling) -**Upstream dependencies**: Common (DTOs, GeoUtils, configs), DataAccess (TileEntity, ITileRepository) +**csproj**: `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj` (split out of the monolithic `SatelliteProvider.Services` project in epic AZ-309) -**Downstream consumers**: RegionProcessing (via ITileService), WebApi (GoogleMapsDownloaderV2 directly for single-tile endpoints) +**Upstream dependencies**: Common (DTOs, GeoUtils, configs, RateLimitException), DataAccess (TileEntity, ITileRepository) + +**Downstream consumers**: RegionProcessing and WebApi — both via `ITileService` from Common (no compile-time `ProjectReference` from any consumer to this project's concrete types). ## 2. Internal Interfaces @@ -24,13 +26,15 @@ | `DownloadAndStoreTilesAsync` | lat, lon, sizeM, zoom, CancellationToken | `List` | Yes | propagated from downloader | | `GetTileAsync` | Guid | `TileMetadata?` | Yes | NpgsqlException | | `GetTilesByRegionAsync` | lat, lon, sizeM, zoom | `IEnumerable` | Yes | NpgsqlException | +| `GetOrDownloadTileAsync` (AZ-310) | z, x, y, CancellationToken | `TileBytes` | Yes | propagated from downloader | +| `DownloadAndStoreSingleTileAsync` (AZ-311) | lat, lon, zoom, CancellationToken | `TileMetadata` | Yes | propagated from downloader | ## 4. Data Access Patterns ### Caching Strategy | Data | Cache Type | TTL | Invalidation | |------|-----------|-----|-------------| -| Tile bytes | In-memory (IMemoryCache, WebApi layer) | 1h absolute, 30min sliding | None (manual restart) | +| Tile bytes | In-memory (IMemoryCache, owned by TileService since AZ-310) | 1h absolute, 30min sliding | None (manual restart) | | Tile metadata | Database | Until year rollover | Version-based (current year) | | Active downloads | ConcurrentDictionary | Duration of download | Removed on completion | @@ -53,7 +57,7 @@ ## 7. Caveats & Edge Cases -- `GoogleMapsDownloaderV2` is registered as a concrete singleton (not behind an interface), creating tight coupling in `TileService` and `Program.cs` +- `GoogleMapsDownloaderV2` is registered behind `ISatelliteDownloader` (resolved by AZ-310 cleanup); the previous concrete-type coupling is gone. - User-Agent header spoofs Chrome — could be rejected if Google changes detection - Allowed zoom levels hardcoded to [15,16,17,18,19] — throws for others - Session token rotation threshold (100 tiles) is an educated guess; Google's actual limit is not documented diff --git a/_docs/02_document/diagrams/components.md b/_docs/02_document/diagrams/components.md index 8b92898..0967053 100644 --- a/_docs/02_document/diagrams/components.md +++ b/_docs/02_document/diagrams/components.md @@ -37,16 +37,17 @@ graph TD Route --> Common Region --> Common Tile --> Common - DA --> Common ``` ## Component Summary -| # | Component | Project(s) | Responsibility | -|---|-----------|-----------|---------------| -| 1 | Common | SatelliteProvider.Common | Shared DTOs, interfaces, configs, GeoUtils | +| # | Component | Project | Responsibility | +|---|-----------|---------|---------------| +| 1 | Common | SatelliteProvider.Common | Shared DTOs, interfaces, configs, GeoUtils, common exceptions | | 2 | DataAccess | SatelliteProvider.DataAccess | Database entities, Dapper repositories, DbUp migrations | -| 3 | TileDownloader | SatelliteProvider.Services (GoogleMapsDownloaderV2, TileService) | Google Maps tile acquisition, storage, caching | -| 4 | RegionProcessing | SatelliteProvider.Services (RegionService, RegionProcessingService, RegionRequestQueue) | Region request lifecycle, tile stitching, CSV/summary output | -| 5 | RouteManagement | SatelliteProvider.Services (RouteService, RouteProcessingService) | Route creation, point interpolation, geofencing, consolidated map output | -| — | WebApi | SatelliteProvider.Api (Program.cs) | HTTP endpoints, DI configuration, startup | +| 3 | TileDownloader | SatelliteProvider.Services.TileDownloader | Google Maps tile acquisition (GoogleMapsDownloaderV2), tile orchestration with caching (TileService) | +| 4 | RegionProcessing | SatelliteProvider.Services.RegionProcessing | Region request lifecycle (RegionService), in-process queue (RegionRequestQueue), background worker (RegionProcessingService) | +| 5 | RouteManagement | SatelliteProvider.Services.RouteManagement | Route creation + point interpolation + geofencing (RouteService), background worker (RouteProcessingService) | +| — | WebApi | SatelliteProvider.Api | HTTP endpoints, DI configuration, startup | + +**Note**: the arrows `Region --> Tile` and `Route --> Region` in the diagram represent **logical** dependencies via interfaces (`ITileService`, `IRegionService`, `IRegionRequestQueue`) defined in `SatelliteProvider.Common`. There are NO compile-time `ProjectReference` entries between the three Layer-3 component csprojs — see `module-layout.md` § Allowed Dependencies. diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index 98a5790..2fdc715 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -3,17 +3,18 @@ **Status**: derived-from-code **Language**: csharp -**Layout Convention**: custom (flat service project, per-project component separation) +**Layout Convention**: custom (per-component .csproj per logical component) **Root**: ./ -**Last Updated**: 2026-05-10 +**Last Updated**: 2026-05-10 (post AZ-309 coupling refactor) ## Layout Rules -1. Each component owns ONE top-level project directory (`.csproj` boundary), except the Services project which hosts three logical components in a flat layout. +1. Each component owns ONE top-level project directory (`.csproj` boundary). The previous shared `SatelliteProvider.Services` project was split into three per-component csprojs in epic AZ-309. 2. Shared code lives under `SatelliteProvider.Common/` — the foundation layer. -3. Cross-cutting concerns (DTOs, interfaces, configs, geo-math) all reside in Common. +3. Cross-cutting concerns (DTOs, interfaces, configs, geo-math, common exceptions) all reside in Common. 4. Public API surface per component = `public` types in the namespace root. Everything marked `internal` or private is internal. 5. Tests live in separate projects: `SatelliteProvider.Tests/` (unit) and `SatelliteProvider.IntegrationTests/` (integration). +6. DI registration per component lives in a `ServiceCollectionExtensions.cs` adjacent to the component's classes (e.g. `TileDownloaderServiceCollectionExtensions.AddTileDownloader()`). ## Per-Component Mapping @@ -26,6 +27,7 @@ - `SatelliteProvider.Common/Configs/ProcessingConfig.cs` - `SatelliteProvider.Common/Configs/DatabaseConfig.cs` - `SatelliteProvider.Common/DTO/*.cs` (all DTOs) + - `SatelliteProvider.Common/Exceptions/RateLimitException.cs` - `SatelliteProvider.Common/Interfaces/*.cs` (all service interfaces) - `SatelliteProvider.Common/Utils/GeoUtils.cs` - **Internal**: (none — all types are public, shared across components) @@ -55,36 +57,45 @@ ### Component: TileDownloader -- **Directory**: `SatelliteProvider.Services/` (shared project) +- **Directory**: `SatelliteProvider.Services.TileDownloader/` +- **csproj**: `SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj` - **Public API**: - - `SatelliteProvider.Services/GoogleMapsDownloaderV2.cs` (implements `ISatelliteDownloader`) - - `SatelliteProvider.Services/TileService.cs` (implements `ITileService`) -- **Internal**: (none — flat project, classes are public) -- **Owns**: `SatelliteProvider.Services/GoogleMapsDownloaderV2.cs`, `SatelliteProvider.Services/TileService.cs` + - `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` (implements `ISatelliteDownloader`) + - `SatelliteProvider.Services.TileDownloader/TileService.cs` (implements `ITileService`) + - `SatelliteProvider.Services.TileDownloader/TileDownloaderServiceCollectionExtensions.cs` (DI: `AddTileDownloader()`) +- **Internal**: (none) +- **Owns**: `SatelliteProvider.Services.TileDownloader/**` +- **ProjectReferences**: `SatelliteProvider.Common`, `SatelliteProvider.DataAccess` - **Imports from**: Common, DataAccess -- **Consumed by**: RegionProcessing, WebApi +- **Consumed by**: RegionProcessing (via `ITileService` from Common; no direct ProjectReference), WebApi ### Component: RegionProcessing -- **Directory**: `SatelliteProvider.Services/` (shared project) +- **Directory**: `SatelliteProvider.Services.RegionProcessing/` +- **csproj**: `SatelliteProvider.Services.RegionProcessing/SatelliteProvider.Services.RegionProcessing.csproj` - **Public API**: - - `SatelliteProvider.Services/RegionService.cs` (implements `IRegionService`) - - `SatelliteProvider.Services/RegionProcessingService.cs` (background hosted service) - - `SatelliteProvider.Services/RegionRequestQueue.cs` (implements `IRegionRequestQueue`) + - `SatelliteProvider.Services.RegionProcessing/RegionService.cs` (implements `IRegionService`) + - `SatelliteProvider.Services.RegionProcessing/RegionProcessingService.cs` (background hosted service) + - `SatelliteProvider.Services.RegionProcessing/RegionRequestQueue.cs` (implements `IRegionRequestQueue`) + - `SatelliteProvider.Services.RegionProcessing/RegionProcessingServiceCollectionExtensions.cs` (DI: `AddRegionProcessing()`) - **Internal**: (none) -- **Owns**: `SatelliteProvider.Services/RegionService.cs`, `SatelliteProvider.Services/RegionProcessingService.cs`, `SatelliteProvider.Services/RegionRequestQueue.cs` -- **Imports from**: Common, DataAccess, TileDownloader -- **Consumed by**: RouteManagement, WebApi +- **Owns**: `SatelliteProvider.Services.RegionProcessing/**` +- **ProjectReferences**: `SatelliteProvider.Common`, `SatelliteProvider.DataAccess` +- **Imports from**: Common, DataAccess (uses `ITileService` from Common — no compile-time dependency on TileDownloader) +- **Consumed by**: RouteManagement (via `IRegionService` and `IRegionRequestQueue` from Common; no direct ProjectReference), WebApi ### Component: RouteManagement -- **Directory**: `SatelliteProvider.Services/` (shared project) +- **Directory**: `SatelliteProvider.Services.RouteManagement/` +- **csproj**: `SatelliteProvider.Services.RouteManagement/SatelliteProvider.Services.RouteManagement.csproj` - **Public API**: - - `SatelliteProvider.Services/RouteService.cs` (implements `IRouteService`) - - `SatelliteProvider.Services/RouteProcessingService.cs` (background hosted service) + - `SatelliteProvider.Services.RouteManagement/RouteService.cs` (implements `IRouteService`) + - `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs` (background hosted service) + - `SatelliteProvider.Services.RouteManagement/RouteManagementServiceCollectionExtensions.cs` (DI: `AddRouteManagement()`) - **Internal**: (none) -- **Owns**: `SatelliteProvider.Services/RouteService.cs`, `SatelliteProvider.Services/RouteProcessingService.cs` -- **Imports from**: Common, DataAccess, RegionProcessing +- **Owns**: `SatelliteProvider.Services.RouteManagement/**` +- **ProjectReferences**: `SatelliteProvider.Common`, `SatelliteProvider.DataAccess` +- **Imports from**: Common, DataAccess (uses `IRegionService` / `IRegionRequestQueue` from Common — no compile-time dependency on RegionProcessing) - **Consumed by**: WebApi ### Component: WebApi @@ -125,16 +136,17 @@ ## Allowed Dependencies (layering) -| Layer | Components | May import from | -|-------|------------|-----------------| -| 4. API / Entry | WebApi | 1, 2, 3 | -| 3. Application (Orchestration) | RouteManagement | 1, 2, 3 (RegionProcessing only) | -| 3. Application (Processing) | RegionProcessing | 1, 2, 3 (TileDownloader only) | -| 2. Domain Services | TileDownloader | 1 | +| Layer | Components | May import from (compile-time ProjectReferences) | +|-------|------------|--------------------------------------------------| +| 4. API / Entry | WebApi | Common, DataAccess, TileDownloader, RegionProcessing, RouteManagement | +| 3. Application | TileDownloader, RegionProcessing, RouteManagement | Common, DataAccess only — siblings communicate through interfaces in Common, never through direct ProjectReferences | | 1. Foundation | Common, DataAccess | Common: (none); DataAccess: (none) | -## Verification Needed +**Key constraint enforced by the AZ-309 split**: the three Layer-3 components are compile-time siblings. Any cross-sibling call (e.g. `RegionProcessing` invoking tile download) MUST go through an interface defined in `SatelliteProvider.Common.Interfaces` and resolved via DI — adding a `ProjectReference` between siblings is now structurally impossible without re-introducing the coupling the refactor removed. + +## Verification -- **Shared Services project**: TileDownloader, RegionProcessing, and RouteManagement coexist in a single `SatelliteProvider.Services/` project. File-level ownership is used (not directory-level) which is unusual for .NET. A future refactor into separate projects per component would make ownership boundaries cleaner. - **No detected cycles**: The dependency graph is a clean DAG. -- **DataAccess layer placement**: DataAccess is placed at Layer 1 (Foundation) alongside Common because it is consumed uniformly by all service components. An alternative layering could place it at Layer 2, but the current code treats repositories as infrastructure, not domain logic. +- **No cross-sibling ProjectReferences**: TileDownloader, RegionProcessing, and RouteManagement each reference only Common + DataAccess. Verified by inspecting all three csproj files. +- **DataAccess layer placement**: DataAccess sits at Layer 1 (Foundation) alongside Common because it is consumed uniformly by all service components. An alternative layering could place it at Layer 2, but the current code treats repositories as infrastructure, not domain logic. +- **DataAccess has no ProjectReference to Common**: confirmed by inspecting `SatelliteProvider.DataAccess.csproj`. DataAccess models and repositories are self-contained (do not use any types from Common). This contradicts an earlier baseline assumption (compliance baseline F5). diff --git a/_docs/02_document/modules/common_interfaces.md b/_docs/02_document/modules/common_interfaces.md index d5c92fd..dd525c7 100644 --- a/_docs/02_document/modules/common_interfaces.md +++ b/_docs/02_document/modules/common_interfaces.md @@ -1,7 +1,7 @@ # Module: Common/Interfaces ## Purpose -Service contracts defining the application's core operations. Implementations live in `SatelliteProvider.Services`. +Service contracts defining the application's core operations. Implementations live in the per-component service projects (`SatelliteProvider.Services.TileDownloader`, `SatelliteProvider.Services.RegionProcessing`, `SatelliteProvider.Services.RouteManagement`). Cross-component runtime calls between Layer-3 components flow exclusively through these interfaces — there are no compile-time `ProjectReference` entries between the three sibling service projects. ## Public Interface @@ -9,6 +9,8 @@ Service contracts defining the application's core operations. Implementations li - `DownloadAndStoreTilesAsync(double lat, double lon, double sizeMeters, int zoomLevel, CancellationToken) → Task>`: downloads missing tiles for a region and returns all tile metadata (existing + new) - `GetTileAsync(Guid id) → Task`: retrieve a single tile by ID - `GetTilesByRegionAsync(double lat, double lon, double sizeMeters, int zoomLevel) → Task>`: query tiles within a geographic region +- `GetOrDownloadTileAsync(int z, int x, int y, CancellationToken) → Task`: serve a tile by Z/X/Y, hitting cache, then repository, then downloader (added in AZ-310) +- `DownloadAndStoreSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken) → Task`: download one tile by lat/lon and persist (added in AZ-311) ### IRegionService - `RequestRegionAsync(Guid id, double lat, double lon, double sizeMeters, int zoomLevel, bool stitchTiles) → Task`: creates a region record and enqueues for async processing diff --git a/_docs/02_document/modules/services_google_maps_downloader.md b/_docs/02_document/modules/services_google_maps_downloader.md index e13f97c..33ac79d 100644 --- a/_docs/02_document/modules/services_google_maps_downloader.md +++ b/_docs/02_document/modules/services_google_maps_downloader.md @@ -3,6 +3,8 @@ ## Purpose Downloads satellite imagery tiles from Google Maps. Handles session token management, concurrent download throttling, retry logic with exponential backoff, and tile deduplication. +**csproj**: `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` + ## Public Interface ### GoogleMapsDownloaderV2 @@ -14,7 +16,7 @@ Downloads satellite imagery tiles from Google Maps. Handles session token manage - `X`, `Y` (int), `ZoomLevel` (int), `CenterLatitude`, `CenterLongitude` (double), `FilePath` (string), `TileSizeMeters` (double) ### RateLimitException (exception) -Custom exception thrown when Google Maps returns 429 Too Many Requests and retries are exhausted. +Lives in `SatelliteProvider.Common.Exceptions` (relocated from this module in epic AZ-309 so RegionProcessing can catch it without acquiring a `ProjectReference` to TileDownloader). Thrown when Google Maps returns 429 Too Many Requests and retries are exhausted. ## Internal Logic - **Allowed zoom levels**: 15, 16, 17, 18, 19 — throws `ArgumentException` for others @@ -30,13 +32,13 @@ Custom exception thrown when Google Maps returns 429 Too Many Requests and retri ## Dependencies - `SatelliteProvider.Common.Configs` — MapConfig, StorageConfig, ProcessingConfig - `SatelliteProvider.Common.DTO` — GeoPoint +- `SatelliteProvider.Common.Exceptions` — RateLimitException - `SatelliteProvider.Common.Utils` — GeoUtils - `SatelliteProvider.DataAccess.Models` — TileEntity (for existingTiles parameter) - NuGet: `Newtonsoft.Json`, `Microsoft.Extensions.Http`, `Microsoft.Extensions.Options` ## Consumers -- `TileService` — `GetTilesWithMetadataAsync` -- `Program.cs` (ServeTile, GetTileByLatLon) — `DownloadSingleTileAsync` +- `TileService` — `GetTilesWithMetadataAsync` and `DownloadSingleTileAsync` (the API endpoints reach this class only through `ITileService` post AZ-310 / AZ-311) ## Data Models Produces `DownloadedTileInfoV2` records; accepts `TileEntity` for cache checks. diff --git a/_docs/02_document/modules/services_tile_service.md b/_docs/02_document/modules/services_tile_service.md index 158654a..1134d2c 100644 --- a/_docs/02_document/modules/services_tile_service.md +++ b/_docs/02_document/modules/services_tile_service.md @@ -1,29 +1,35 @@ # Module: Services/TileService ## Purpose -Orchestrates tile downloading and persistence. Bridges the downloader (Google Maps) with the tile repository (PostgreSQL), handling cache checks, entity creation, and metadata mapping. +Orchestrates tile downloading and persistence. Bridges the downloader (Google Maps) with the tile repository (PostgreSQL), handling in-memory caching, entity creation, and metadata mapping. Single ownership point for all tile read/write business logic — both region-batch and single-tile API endpoints route through this service. + +**csproj**: `SatelliteProvider.Services.TileDownloader/TileService.cs` ## Public Interface ### TileService (implements ITileService) - `DownloadAndStoreTilesAsync(double lat, double lon, double sizeMeters, int zoomLevel, CancellationToken) → Task>`: 1. Queries existing tiles in the region from the repository (filtered to current year's version) - 2. Calls `GoogleMapsDownloaderV2.GetTilesWithMetadataAsync` with existing tiles to skip + 2. Calls `ISatelliteDownloader.GetTilesWithMetadataAsync` with existing tiles to skip 3. Creates `TileEntity` for each newly downloaded tile and inserts via repository (upsert) 4. Returns combined list of existing + new tile metadata - `GetTileAsync(Guid id) → Task`: single tile lookup - `GetTilesByRegionAsync(double lat, double lon, double sizeMeters, int zoomLevel) → Task>`: query tiles in a region +- `GetOrDownloadTileAsync(int z, int x, int y, CancellationToken) → Task` (AZ-310): cache → repository → downloader fallback for single Z/X/Y serving +- `DownloadAndStoreSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken) → Task` (AZ-311): download one tile by lat/lon, persist, return metadata ## Internal Logic - Version is `DateTime.UtcNow.Year` — tiles are considered fresh for the current calendar year - `MapToMetadata(TileEntity) → TileMetadata`: entity-to-DTO mapping (static helper) - Tile size hardcoded to 256 pixels, image type "jpg" - `MapsVersion` formatted as `"downloaded_{date}"` +- `IMemoryCache` keyed by `(z, x, y)` with 1h absolute / 30min sliding expiration; populated on first hit and on downloader fallback ## Dependencies -- `GoogleMapsDownloaderV2` (concrete class, not interface) +- `ISatelliteDownloader` (resolved via DI; concrete is `GoogleMapsDownloaderV2`) - `ITileRepository` -- `SatelliteProvider.Common.DTO` — GeoPoint, TileMetadata +- `IMemoryCache` (registered by `AddTileDownloader()`) +- `SatelliteProvider.Common.DTO` — GeoPoint, TileMetadata, TileBytes - `SatelliteProvider.DataAccess.Models` — TileEntity ## Consumers diff --git a/_docs/02_document/modules/tests_unit.md b/_docs/02_document/modules/tests_unit.md index e6ac934..b2635c0 100644 --- a/_docs/02_document/modules/tests_unit.md +++ b/_docs/02_document/modules/tests_unit.md @@ -12,8 +12,8 @@ Unit test project. Currently contains only a single dummy test as a placeholder. No meaningful test logic. ## Dependencies -- Project references: `SatelliteProvider.Services`, `SatelliteProvider.Common` -- NuGet: xUnit (2.5.3), Moq (4.20.72), FluentAssertions (8.8.0), coverlet.collector (6.0.0), Microsoft.NET.Test.Sdk (17.8.0), Microsoft.Extensions.* (Configuration, DI, Logging, Options, Http) +- Project references: `SatelliteProvider.Services.TileDownloader`, `SatelliteProvider.Services.RegionProcessing`, `SatelliteProvider.Services.RouteManagement`, `SatelliteProvider.Common`, `SatelliteProvider.DataAccess` +- NuGet: xUnit (2.5.3), Moq (4.20.72), FluentAssertions (8.8.0), coverlet.collector (6.0.0), Microsoft.NET.Test.Sdk (17.8.0), Microsoft.Extensions.* (Caching.Memory, Configuration, DI, Logging, Options, Http) - Has `appsettings.json` copied to output (empty config for potential future test setups) ## Consumers diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 4ca9e46..bd63e9e 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -17,12 +17,12 @@ | 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 | +| AZ-310 ServeTile via ITileService | — | 3 | Done (In Testing) | +| AZ-311 GetTileByLatLon via ITileService | AZ-310 | 2 | Done (In Testing) | +| AZ-312 Split Services into 3 csprojs | AZ-311 | 5 | Done (In Testing) | +| AZ-313 Update consumers (Api/Tests) | AZ-312 | 3 | Done (In Testing) | +| AZ-314 DI registration split | AZ-313 | 2 | Done (In Testing) | +| AZ-315 Documentation sync | AZ-314 | 2 | In Progress | ## Execution Order diff --git a/_docs/02_tasks/todo/AZ-315_refactor_documentation_sync.md b/_docs/02_tasks/done/AZ-315_refactor_documentation_sync.md similarity index 100% rename from _docs/02_tasks/todo/AZ-315_refactor_documentation_sync.md rename to _docs/02_tasks/done/AZ-315_refactor_documentation_sync.md diff --git a/_docs/03_implementation/batch_06_report.md b/_docs/03_implementation/batch_06_report.md new file mode 100644 index 0000000..b29b55d --- /dev/null +++ b/_docs/03_implementation/batch_06_report.md @@ -0,0 +1,70 @@ +# Batch Report + +**Batch**: 6 (refactor 02-coupling-refactoring · Phase C — final) +**Tasks**: AZ-315 +**Date**: 2026-05-10 + +## Task Results + +| Task | Status | Files Modified | Tests | AC Coverage | Issues | +|------|--------|---------------|-------|-------------|--------| +| AZ-315 Documentation sync | Done | 9 docs + dependencies table | n/a (docs-only) | 4/4 ACs | None | + +## Files Changed + +- `_docs/02_document/architecture.md` +- `_docs/02_document/module-layout.md` +- `_docs/02_document/architecture_compliance_baseline.md` +- `_docs/02_document/diagrams/components.md` +- `_docs/02_document/components/03_tile_downloader/description.md` +- `_docs/02_document/modules/common_interfaces.md` +- `_docs/02_document/modules/services_tile_service.md` +- `_docs/02_document/modules/services_google_maps_downloader.md` +- `_docs/02_document/modules/tests_unit.md` +- `_docs/02_tasks/_dependencies_table.md` +- `_docs/03_implementation/batch_06_report.md` (this file) +- `_docs/03_implementation/reviews/batch_06_review.md` + +## AC Test Coverage + +4 of 4 ACs verified by inspection: +- AC-1 (architecture.md reflects split) — verified +- AC-2 (module-layout.md accurate) — verified +- AC-3 (compliance baseline closed for F3/F4) — verified, also closed F5 +- AC-4 (no stale current-state references) — verified by grep; remaining matches are historical context + +## Code Review Verdict + +**PASS** — no findings. See `_docs/03_implementation/reviews/batch_06_review.md`. + +## Auto-Fix Attempts + +0 + +## Build / Tests + +N/A — documentation-only batch. No code changes; no build needed. + +## Architecture-Baseline Impact + +- F5 (Low — DataAccess documented as importing Common): **resolved** (module-layout.md updated). +- All five baseline findings (F1–F5) are now closed. Architecture compliance baseline shows 0 findings post AZ-309. + +## Epic AZ-309 — Closing Notes + +Epic AZ-309 (architecture coupling refactor) is complete with 6 tasks shipped across 3 batches: + +| Batch | Phase | Tasks | Outcome | +|-------|-------|-------|---------| +| 4 | A — endpoint routing | AZ-310, AZ-311 | Both tile endpoints now route through `ITileService`. F3 resolved. | +| 5 | B — project split | AZ-312, AZ-313, AZ-314 | `SatelliteProvider.Services` split into three per-component csprojs with compiler-enforced sibling boundary. F4 resolved. | +| 6 | C — docs | AZ-315 | All architecture docs synced. F5 resolved. | + +Aggregate verification: +- `dotnet build SatelliteProvider.sln` — 0 warn, 0 err +- `dotnet test SatelliteProvider.Tests` — 40/40 pass (35 baseline + 5 new from AZ-310/AZ-311) +- `./scripts/run-tests.sh --smoke` — full integration suite green end-to-end + +## Next Step (Autodev) + +Refactor 02-coupling-refactoring complete. Return control to autodev orchestrator (Step 8 sub-step `verification` then `documentation`, then exit Refactor and proceed to Step 9). diff --git a/_docs/03_implementation/reviews/batch_06_review.md b/_docs/03_implementation/reviews/batch_06_review.md new file mode 100644 index 0000000..c590a79 --- /dev/null +++ b/_docs/03_implementation/reviews/batch_06_review.md @@ -0,0 +1,89 @@ +# Code Review Report + +**Batch**: 6 (AZ-315 — documentation sync, Phase C of coupling refactor) +**Date**: 2026-05-10 +**Verdict**: PASS + +## Scope + +Documentation-only batch. No source code, build configuration, or test changes. Closes epic AZ-309 (architecture coupling refactor) by syncing the architecture / module-layout / compliance baseline / per-component module docs to the post-split reality. + +## Changed files + +- `_docs/02_document/architecture.md` — components list updated with csproj names; internal-communication table updated to reflect calls flowing through Common interfaces (no direct concrete dependencies). +- `_docs/02_document/module-layout.md` — three Layer-3 components now have their own per-component sections with csproj path, public API list, ProjectReferences, and explicit "no cross-sibling reference" note. Allowed-Dependencies table simplified to one Layer-3 row stating siblings communicate only through Common interfaces. Verification section corrects DataAccess → Common assumption (F5). +- `_docs/02_document/architecture_compliance_baseline.md` — F1, F2, F3, F4, F5 marked Resolved with task IDs and commit references; baseline summary now shows 0 findings post AZ-309. +- `_docs/02_document/diagrams/components.md` — component summary table updated with new csproj names; added clarifying note that Layer-3-to-Layer-3 arrows are logical-only via Common interfaces; removed incorrect `DA --> Common` edge. +- `_docs/02_document/components/03_tile_downloader/description.md` — csproj path added; `ITileService` extended interface enumerated; downloader/service consumers note clarified. +- `_docs/02_document/modules/common_interfaces.md` — implementations now mapped to per-component csprojs; new ITileService methods documented. +- `_docs/02_document/modules/services_tile_service.md` — csproj path added; `GetOrDownloadTileAsync` and `DownloadAndStoreSingleTileAsync` documented; `IMemoryCache` dependency declared. +- `_docs/02_document/modules/services_google_maps_downloader.md` — csproj path added; RateLimitException relocation noted. +- `_docs/02_document/modules/tests_unit.md` — ProjectReferences updated to the three new service projects. +- `_docs/02_tasks/_dependencies_table.md` — refactor tasks marked Done / In Progress. + +## Findings + +| # | Severity | Category | File:Line | Title | +|---|----------|----------|-----------|-------| + +No findings. + +## Phase results + +### Phase 2 — Spec compliance (AC verification) + +- **AC-1: Architecture doc reflects the split** ✓ + `architecture.md` Architecture Vision and Internal Communication sections name all three csprojs and explicitly state cross-sibling calls use Common interfaces. +- **AC-2: Module layout is accurate** ✓ + `module-layout.md` § Per-Component Mapping has one section per new csproj with `csproj`, `Public API`, `ProjectReferences`, `Imports from`, and `Consumed by` keyed to the actual on-disk layout. +- **AC-3: Compliance baseline closed for F3/F4** ✓ + `architecture_compliance_baseline.md` table has a Status column; F3/F4/F5 each cite the resolving task IDs (AZ-310/311 for F3, AZ-312/313/314 for F4, AZ-315 for F5) and commit hash where applicable. +- **AC-4: No stale references to current state** ✓ + `grep "SatelliteProvider\.Services[^.]"` against `_docs/02_document/` returns matches only in: + - `00_discovery.md` (historical baseline snapshot — pre-refactor by design) + - `architecture_compliance_baseline.md` (describing the resolved-state of historical findings) + - `module-layout.md` § Layout Rule #1 (one historical-context mention naming the project that was split) + - `components/03_tile_downloader/description.md` (one historical-context mention explaining the relocation) + No current-state description still claims a single Services project. + +### Phase 3 — Code quality + +N/A — no code changes. + +### Phase 4 — Security quick-scan + +N/A — no code changes. + +### Phase 5 — Performance scan + +N/A — no code changes. + +### Phase 6 — Cross-task consistency + +- All updated docs use consistent naming for the three new csprojs. +- Cross-doc references (architecture → module-layout → diagrams → per-component descriptions) form a coherent narrative without contradictions. +- F5 resolution in module-layout.md (Verification section) and architecture_compliance_baseline.md agree. + +### Phase 7 — Architecture compliance + +N/A on the code side — no code changes. On the doc side, the docs now accurately describe the architecture rather than the pre-refactor state. + +## Baseline Delta + +Final state of `architecture_compliance_baseline.md` after this batch: + +| Finding | Severity | Status | +|---------|----------|--------| +| F1 — Concrete dependency on GoogleMapsDownloaderV2 | High | Resolved (pre-AZ-309) | +| F2 — ISatelliteDownloader is dead code | High | Resolved (pre-AZ-309) | +| F3 — API endpoint bypasses service layer | Medium | Resolved (AZ-310 + AZ-311) | +| F4 — No physical boundary in Services project | Medium | Resolved (AZ-312 + AZ-313 + AZ-314) | +| F5 — DataAccess documented as importing Common (incorrect) | Low | Resolved (AZ-315) | + +**Newly introduced**: none. +**Carried over**: none. +**Resolved this batch**: F5. + +## Verdict + +**PASS** — all 4 ACs satisfied. Documentation now matches the actual project layout. Architecture compliance baseline is fully closed. diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 0b27334..2f8fcc2 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -6,8 +6,8 @@ step: 8 name: Refactor status: in_progress sub_step: - phase: 3 - name: safety-net + phase: 7 + name: documentation detail: "" retry_count: 0 cycle: 1