mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-22 13:21:14 +00:00
[AZ-310] [AZ-311] Route tile endpoints through ITileService
Move cache+DB+download logic for /tiles/{z}/{x}/{y} and
/api/satellite/tiles/latlon out of Program.cs into TileService.
Endpoints now inject only ITileService + ILogger. Service owns
IMemoryCache (1h absolute / 30min sliding preserved). Added
TileBytes DTO; ITileService gains GetOrDownloadTileAsync and
DownloadAndStoreSingleTileAsync. 5 new unit tests cover cache
hit, repo hit, downloader fallback, and AZ-311 happy + error.
Build clean (0/0), unit suite 40/40. Resolves architecture
baseline F3 in code (docs handled by AZ-315).
Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,80 @@
|
||||
# Code Review Report
|
||||
|
||||
**Batch**: 4 (refactor 02-coupling-refactoring · Phase A)
|
||||
**Tasks**: AZ-310, AZ-311
|
||||
**Date**: 2026-05-10
|
||||
**Verdict**: PASS
|
||||
|
||||
## Scope
|
||||
|
||||
Changed files:
|
||||
- `SatelliteProvider.Common/DTO/TileBytes.cs` (new)
|
||||
- `SatelliteProvider.Common/Interfaces/ITileService.cs` (added 2 methods)
|
||||
- `SatelliteProvider.Services/SatelliteProvider.Services.csproj` (added `Microsoft.Extensions.Caching.Memory` 9.0.10)
|
||||
- `SatelliteProvider.Services/TileService.cs` (added IMemoryCache dependency + 2 methods + private `BuildTileEntity` helper)
|
||||
- `SatelliteProvider.Api/Program.cs` (thinned `ServeTile` and `GetTileByLatLon`; dropped `IMemoryCache`/`ITileRepository`/`ISatelliteDownloader` injections + redundant `using` directives)
|
||||
- `SatelliteProvider.Tests/SatelliteProvider.Tests.csproj` (added `Microsoft.Extensions.Caching.Memory` 9.0.10)
|
||||
- `SatelliteProvider.Tests/TileServiceTests.cs` (5 new tests; constructor helper takes optional `IMemoryCache`)
|
||||
- `SatelliteProvider.Tests/InfrastructureTests.cs` (constructor signature update)
|
||||
|
||||
## Phase 1 — Context
|
||||
|
||||
Both tasks target architecture-baseline F3 (endpoints bypass `ITileService`). Phase A of `02-coupling-refactoring` consolidates tile-serving logic in `TileService`.
|
||||
|
||||
## Phase 2 — Spec Compliance
|
||||
|
||||
| AC | Status | Evidence |
|
||||
|----|--------|----------|
|
||||
| AZ-310 AC-1 | Pass | Route preserved (`Program.cs:113`); response keeps `image/jpeg`, `ETag`, `Cache-Control: public, max-age=86400` |
|
||||
| AZ-310 AC-2 | Pass | `GetOrDownloadTileAsync_CacheHit_ReturnsCachedBytes_AZ310_AC2` (downloader+repo strict-mock asserted untouched) |
|
||||
| AZ-310 AC-3 | Pass | `GetOrDownloadTileAsync_RepoHit_ReadsFromDisk_NoDownloader_AZ310_AC3` (writes a real temp file, asserts no downloader call, no insert) |
|
||||
| AZ-310 AC-4 | Pass | `GetOrDownloadTileAsync_DownloaderFallback_InsertsAndReturnsBytes_AZ310_AC4` (asserts insert + downloader call) |
|
||||
| AZ-310 AC-5 | Pass | `ServeTile(int z, int x, int y, HttpContext, ITileService, ILogger<Program>)` — no `ISatelliteDownloader`/`ITileRepository`/`IMemoryCache` |
|
||||
| AZ-311 AC-1 | Pass | Same route, same query params, same `DownloadTileResponse` shape; field-by-field projection in `Program.cs:155-172` |
|
||||
| AZ-311 AC-2 | Pass | `DownloadAndStoreSingleTileAsync_HappyPath_…` + `…_DownloaderThrows_DoesNotInsert_…` cover both branches |
|
||||
| AZ-311 AC-3 | Pass | `GetTileByLatLon(... ITileService, ILogger<Program>)` — no `ISatelliteDownloader`/`ITileRepository` |
|
||||
|
||||
**Constraints check (AZ-310)**:
|
||||
- Cache lifetime: `TimeSpan.FromHours(1)` absolute + `TimeSpan.FromMinutes(30)` sliding — preserved verbatim.
|
||||
- Cache key `tile_{z}_{x}_{y}`, ETag `"{z}_{x}_{y}"` — preserved verbatim.
|
||||
- `Cache-Control: public, max-age=86400` — built from `TileResponseMaxAge = TimeSpan.FromDays(1)` (`(long)TotalSeconds` = 86400).
|
||||
|
||||
**Constraints check (AZ-311)**: query string and JSON shape unchanged.
|
||||
|
||||
## Phase 3 — Code Quality
|
||||
|
||||
- SOLID: `TileService` now has 5 public methods, all single-responsibility. Private `BuildTileEntity` removes 3-fold duplication of `TileEntity` construction.
|
||||
- Error handling: cancellation token propagates through every async boundary in the new methods. Endpoints still wrap with try/catch + `Results.Problem`.
|
||||
- Tests use `MockBehavior.Strict` where it matters (cache-hit path) and explicit `VerifyNoOtherCalls()` to assert untouched dependencies.
|
||||
- Naming: `GetOrDownloadTileAsync` and `DownloadAndStoreSingleTileAsync` accurately describe behavior.
|
||||
- DRY: deduplication of `TileEntity` construction is a net win. Cache-key + ETag string formats are localized to one method (`GetOrDownloadTileAsync`); externalizing as constants would not improve readability for two-use values.
|
||||
|
||||
## Phase 4 — Security Quick-Scan
|
||||
|
||||
No new SQL string concatenation, no new external input parsing, no secrets. `TileBytes` only carries bytes/headers.
|
||||
|
||||
## Phase 5 — Performance Scan
|
||||
|
||||
Identical I/O pattern as pre-refactor — same number of cache lookups, DB queries, downloads, and file reads.
|
||||
|
||||
## Phase 6 — Cross-Task Consistency
|
||||
|
||||
- AZ-310 and AZ-311 share `BuildTileEntity` — consistent entity construction.
|
||||
- Both new methods accept `CancellationToken cancellationToken = default` — consistent signature style.
|
||||
- Both endpoints follow same flat shape: validate (route binding) → call service → project → return.
|
||||
|
||||
## Phase 7 — Architecture Compliance
|
||||
|
||||
- Layer direction: `TileService` (Layer 2) imports `Microsoft.Extensions.Caching.Memory` (system) — fine. `ITileService` stays in Common (Layer 1).
|
||||
- Public API: no cross-component internal imports added.
|
||||
- No new cycles.
|
||||
- Layer 4 (`Program.cs`) no longer reaches into Layer 1 (`ITileRepository`) for tile serving / single-tile flow — the very thing baseline F3 flagged.
|
||||
- Baseline F3 (`Program.cs/ServeTile bypasses ITileService`) is **resolved by this batch**. Documentation update happens in AZ-315.
|
||||
|
||||
## Findings
|
||||
|
||||
None.
|
||||
|
||||
## Verdict
|
||||
|
||||
**PASS** — no Critical, High, Medium, or Low findings. Build clean (0/0). Unit suite 40/40 green.
|
||||
Reference in New Issue
Block a user