mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 23:21:13 +00:00
Oleksandr Bezdieniezhnykh
08451df027
Phase 6 (Verification): smoke run green (format gate + 200/200 unit + integration smoke). verification_report.md captures metric deltas vs Phase 0 baseline; all 5 ACs met, all 4 constraints honored, 0 regressions. Phase 7 (Documentation): - module-layout.md: corrected DataAccess->Common dependency (was mistakenly documented as "Imports from: (none)" by prior AZ-315 baseline; csproj reference + 7 import sites have actually been there since AZ-309). - architecture_compliance_baseline.md: F5 entry revised to reflect the actual layering invariant (one-way: Common MUST NOT import from DataAccess, but DataAccess MAY import from Common). - 00_discovery.md: added "Updates Since Baseline" section enumerating the AZ-309 split + AZ-350 27-change run + AZ-372 tooling additions; original tree kept as a 2026-05-10 snapshot. FINAL_report: complete run summary (10 batches, 27 tasks, 3 K=3 cumulative reviews, baseline->final metric table, remaining items, lessons learned). Autodev state: advance Step 8 -> Step 9 (New Task); sub_step reset to phase 0 awaiting-invocation. Co-authored-by: Cursor <cursoragent@cursor.com>
73 lines
7.4 KiB
Markdown
73 lines
7.4 KiB
Markdown
# Architecture Compliance Baseline
|
|
|
|
**Date**: 2026-05-10
|
|
**Mode**: Baseline (Phase 1 + Phase 7)
|
|
**Scope**: Full existing codebase
|
|
**Verdict**: PASS_WITH_WARNINGS (baseline) → all findings resolved by epic AZ-309
|
|
|
|
## Findings
|
|
|
|
| # | 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
|
|
|
|
**F1: Concrete dependency on GoogleMapsDownloaderV2** (High / Architecture)
|
|
- Location: `SatelliteProvider.Services/TileService.cs:11`
|
|
- Description: `TileService` depends on the concrete class `GoogleMapsDownloaderV2` instead of `ISatelliteDownloader`. DI registration is also concrete (`AddSingleton<GoogleMapsDownloaderV2>()`). This couples the entire tile pipeline to a single provider.
|
|
- Impact: Adding a new satellite imagery provider requires modifying TileService and Program.cs DI wiring rather than just registering a new implementation.
|
|
- Suggestion: Have `GoogleMapsDownloaderV2` implement `ISatelliteDownloader`, update DI to register via interface, inject interface into TileService.
|
|
|
|
**F2: ISatelliteDownloader is dead code** (High / Architecture)
|
|
- Location: `SatelliteProvider.Common/Interfaces/ISatelliteDownloader.cs`
|
|
- Description: The interface exists (declares `GetTiles(GeoPoint, double, int, CancellationToken)`) but NO class implements it and NO code references it. The actual downloader method used is `GoogleMapsDownloaderV2.GetTilesWithMetadataAsync()` which has a different signature.
|
|
- 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) — **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) — **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 vs DataAccess→Common dependency** (Low / Architecture) — **REVISED 2026-05-11**
|
|
- Location: `_docs/02_document/module-layout.md`
|
|
- Original baseline claim: DataAccess was documented as "Imports from: Common" but the inspector concluded DataAccess had no ProjectReference to Common nor any `using SatelliteProvider.Common`.
|
|
- **Re-verified 2026-05-11** during 03-code-quality-refactoring (AZ-377): the original baseline scan was inaccurate. `SatelliteProvider.DataAccess.csproj` line 18 has a `<ProjectReference Include="..\SatelliteProvider.Common\SatelliteProvider.Common.csproj" />` that has been present since at least the AZ-309 split, and 5 DataAccess files import `SatelliteProvider.Common.Enums` (`RegionRepository.cs`, `IRegionRepository.cs`, `Models/RegionEntity.cs`, `Models/RoutePointEntity.cs`, `TypeHandlers/EnumStringTypeHandler.cs`).
|
|
- Impact: Documentation inaccuracy. Caused the AZ-315 doc-sync to "resolve" the finding by claiming DataAccess has no Common dep — that resolution itself was incorrect.
|
|
- **Resolution (AZ-377 + Phase 7 of 03-code-quality-refactoring run, commit 9a53bff)**: `module-layout.md` now correctly lists DataAccess `ProjectReferences: SatelliteProvider.Common` and enumerates the 7 import sites (5 enum + 1 `MapConfig.DefaultTileSizePixels` + 1 `GeoUtils.*`). The §Verification section now states the actual constraint: Common MUST NOT import from DataAccess (one-way leaf invariant). AZ-377 also widened the dependency surface from `Common.Enums` only to also include `Common.Configs` and `Common.Utils` — see batch 24 cumulative review F1.
|
|
|
|
## Summary
|
|
|
|
| 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.
|