diff --git a/SatelliteProvider.Api/Program.cs b/SatelliteProvider.Api/Program.cs index e39f8d2..0873d79 100644 --- a/SatelliteProvider.Api/Program.cs +++ b/SatelliteProvider.Api/Program.cs @@ -1,15 +1,12 @@ using System.ComponentModel.DataAnnotations; using Microsoft.AspNetCore.Mvc; -using Microsoft.Extensions.Caching.Memory; using Microsoft.OpenApi.Models; using Swashbuckle.AspNetCore.SwaggerGen; using SatelliteProvider.DataAccess; -using SatelliteProvider.DataAccess.Models; using SatelliteProvider.DataAccess.Repositories; using SatelliteProvider.Common.Configs; using SatelliteProvider.Common.DTO; using SatelliteProvider.Common.Interfaces; -using SatelliteProvider.Common.Utils; using SatelliteProvider.Services; using Serilog; @@ -138,63 +135,14 @@ app.MapGet("/api/satellite/route/{id:guid}", GetRoute) app.Run(); -async Task ServeTile(int z, int x, int y, HttpContext httpContext, ITileRepository tileRepository, ISatelliteDownloader downloader, IMemoryCache cache, ILogger logger) +async Task ServeTile(int z, int x, int y, HttpContext httpContext, ITileService tileService, ILogger logger) { - var cacheKey = $"tile_{z}_{x}_{y}"; try { - if (cache.TryGetValue(cacheKey, out byte[]? cachedBytes) && cachedBytes != null) - { - httpContext.Response.Headers.CacheControl = "public, max-age=86400"; - httpContext.Response.Headers.ETag = $"\"{z}_{x}_{y}\""; - return Results.Bytes(cachedBytes, "image/jpeg"); - } - - string? filePath = null; - - var tile = await tileRepository.GetByTileCoordinatesAsync(z, x, y); - if (tile != null && File.Exists(tile.FilePath)) - { - filePath = tile.FilePath; - } - else - { - var tileCenter = GeoUtils.TileToWorldPos(x, y, z); - var downloadedTile = await downloader.DownloadSingleTileAsync(tileCenter.Lat, tileCenter.Lon, z); - - var now = DateTime.UtcNow; - var tileEntity = new TileEntity - { - Id = Guid.NewGuid(), - TileZoom = z, - TileX = downloadedTile.X, - TileY = downloadedTile.Y, - Latitude = downloadedTile.CenterLatitude, - Longitude = downloadedTile.CenterLongitude, - TileSizeMeters = downloadedTile.TileSizeMeters, - TileSizePixels = 256, - ImageType = "jpg", - MapsVersion = $"downloaded_{now:yyyy-MM-dd}", - Version = now.Year, - FilePath = downloadedTile.FilePath, - CreatedAt = now, - UpdatedAt = now - }; - - await tileRepository.InsertAsync(tileEntity); - filePath = tileEntity.FilePath; - } - - var bytes = await File.ReadAllBytesAsync(filePath); - cache.Set(cacheKey, bytes, new MemoryCacheEntryOptions - { - AbsoluteExpirationRelativeToNow = TimeSpan.FromHours(1), - SlidingExpiration = TimeSpan.FromMinutes(30) - }); - - httpContext.Response.Headers.CacheControl = "public, max-age=86400"; - httpContext.Response.Headers.ETag = $"\"{z}_{x}_{y}\""; - return Results.Bytes(bytes, "image/jpeg"); + var tile = await tileService.GetOrDownloadTileAsync(z, x, y, httpContext.RequestAborted); + httpContext.Response.Headers.CacheControl = $"public, max-age={(long)tile.MaxAge.TotalSeconds}"; + httpContext.Response.Headers.ETag = tile.ETag; + return Results.Bytes(tile.Bytes, tile.ContentType); } catch (Exception ex) { @@ -203,51 +151,26 @@ async Task ServeTile(int z, int x, int y, HttpContext httpContext, ITil } } -async Task GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, ISatelliteDownloader downloader, ITileRepository tileRepository, ILogger logger) +async Task GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, ITileService tileService, ILogger logger) { try { - var downloadedTile = await downloader.DownloadSingleTileAsync( - Latitude, - Longitude, - ZoomLevel); - - var now = DateTime.UtcNow; - var currentVersion = now.Year; - var tileEntity = new TileEntity - { - Id = Guid.NewGuid(), - TileZoom = downloadedTile.ZoomLevel, - TileX = downloadedTile.X, - TileY = downloadedTile.Y, - Latitude = downloadedTile.CenterLatitude, - Longitude = downloadedTile.CenterLongitude, - TileSizeMeters = downloadedTile.TileSizeMeters, - TileSizePixels = 256, - ImageType = "jpg", - MapsVersion = $"downloaded_{now:yyyy-MM-dd}", - Version = currentVersion, - FilePath = downloadedTile.FilePath, - CreatedAt = now, - UpdatedAt = now - }; - - await tileRepository.InsertAsync(tileEntity); + var tile = await tileService.DownloadAndStoreSingleTileAsync(Latitude, Longitude, ZoomLevel); var response = new DownloadTileResponse { - Id = tileEntity.Id, - ZoomLevel = tileEntity.TileZoom, - Latitude = tileEntity.Latitude, - Longitude = tileEntity.Longitude, - TileSizeMeters = tileEntity.TileSizeMeters, - TileSizePixels = tileEntity.TileSizePixels, - ImageType = tileEntity.ImageType, - MapsVersion = tileEntity.MapsVersion, - Version = currentVersion, - FilePath = tileEntity.FilePath, - CreatedAt = tileEntity.CreatedAt, - UpdatedAt = tileEntity.UpdatedAt + Id = tile.Id, + ZoomLevel = tile.TileZoom, + Latitude = tile.Latitude, + Longitude = tile.Longitude, + TileSizeMeters = tile.TileSizeMeters, + TileSizePixels = tile.TileSizePixels, + ImageType = tile.ImageType, + MapsVersion = tile.MapsVersion, + Version = tile.Version, + FilePath = tile.FilePath, + CreatedAt = tile.CreatedAt, + UpdatedAt = tile.UpdatedAt }; return Results.Ok(response); diff --git a/SatelliteProvider.Common/DTO/TileBytes.cs b/SatelliteProvider.Common/DTO/TileBytes.cs new file mode 100644 index 0000000..a155cbb --- /dev/null +++ b/SatelliteProvider.Common/DTO/TileBytes.cs @@ -0,0 +1,3 @@ +namespace SatelliteProvider.Common.DTO; + +public record TileBytes(byte[] Bytes, string ContentType, string ETag, TimeSpan MaxAge); diff --git a/SatelliteProvider.Common/Interfaces/ITileService.cs b/SatelliteProvider.Common/Interfaces/ITileService.cs index 38c5203..f809adf 100644 --- a/SatelliteProvider.Common/Interfaces/ITileService.cs +++ b/SatelliteProvider.Common/Interfaces/ITileService.cs @@ -7,5 +7,7 @@ public interface ITileService Task> DownloadAndStoreTilesAsync(double latitude, double longitude, double sizeMeters, int zoomLevel, CancellationToken cancellationToken = default); Task GetTileAsync(Guid id); Task> GetTilesByRegionAsync(double latitude, double longitude, double sizeMeters, int zoomLevel); + Task GetOrDownloadTileAsync(int z, int x, int y, CancellationToken cancellationToken = default); + Task DownloadAndStoreSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken cancellationToken = default); } diff --git a/SatelliteProvider.Services/SatelliteProvider.Services.csproj b/SatelliteProvider.Services/SatelliteProvider.Services.csproj index 82dda4b..6fc8dea 100644 --- a/SatelliteProvider.Services/SatelliteProvider.Services.csproj +++ b/SatelliteProvider.Services/SatelliteProvider.Services.csproj @@ -7,6 +7,7 @@ + diff --git a/SatelliteProvider.Services/TileService.cs b/SatelliteProvider.Services/TileService.cs index 6a55887..f58232e 100644 --- a/SatelliteProvider.Services/TileService.cs +++ b/SatelliteProvider.Services/TileService.cs @@ -1,6 +1,8 @@ +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging; using SatelliteProvider.Common.DTO; using SatelliteProvider.Common.Interfaces; +using SatelliteProvider.Common.Utils; using SatelliteProvider.DataAccess.Models; using SatelliteProvider.DataAccess.Repositories; @@ -8,17 +10,25 @@ namespace SatelliteProvider.Services; public class TileService : ITileService { + private static readonly TimeSpan TileCacheAbsolute = TimeSpan.FromHours(1); + private static readonly TimeSpan TileCacheSliding = TimeSpan.FromMinutes(30); + private static readonly TimeSpan TileResponseMaxAge = TimeSpan.FromDays(1); + private const string TileImageContentType = "image/jpeg"; + private readonly ISatelliteDownloader _downloader; private readonly ITileRepository _tileRepository; + private readonly IMemoryCache _cache; private readonly ILogger _logger; public TileService( ISatelliteDownloader downloader, ITileRepository tileRepository, + IMemoryCache cache, ILogger logger) { _downloader = downloader; _tileRepository = tileRepository; + _cache = cache; _logger = logger; } @@ -48,8 +58,6 @@ public class TileService : ITileService cancellationToken); var result = new List(); - int reusedCount = existingTilesList.Count; - int downloadedCount = downloadedTiles.Count; foreach (var existingTile in existingTilesList) { @@ -58,26 +66,7 @@ public class TileService : ITileService foreach (var downloadedTile in downloadedTiles) { - var now = DateTime.UtcNow; - - var tileEntity = new TileEntity - { - Id = Guid.NewGuid(), - TileZoom = downloadedTile.ZoomLevel, - TileX = downloadedTile.X, - TileY = downloadedTile.Y, - Latitude = downloadedTile.CenterLatitude, - Longitude = downloadedTile.CenterLongitude, - TileSizeMeters = downloadedTile.TileSizeMeters, - TileSizePixels = 256, - ImageType = "jpg", - MapsVersion = $"downloaded_{now:yyyy-MM-dd}", - Version = currentVersion, - FilePath = downloadedTile.FilePath, - CreatedAt = now, - UpdatedAt = now - }; - + var tileEntity = BuildTileEntity(downloadedTile, currentVersion); await _tileRepository.InsertAsync(tileEntity); result.Add(MapToMetadata(tileEntity)); } @@ -101,6 +90,75 @@ public class TileService : ITileService return tiles.Select(MapToMetadata); } + public async Task GetOrDownloadTileAsync(int z, int x, int y, CancellationToken cancellationToken = default) + { + var cacheKey = $"tile_{z}_{x}_{y}"; + var etag = $"\"{z}_{x}_{y}\""; + + if (_cache.TryGetValue(cacheKey, out byte[]? cachedBytes) && cachedBytes != null) + { + return new TileBytes(cachedBytes, TileImageContentType, etag, TileResponseMaxAge); + } + + string filePath; + var existing = await _tileRepository.GetByTileCoordinatesAsync(z, x, y); + if (existing != null && File.Exists(existing.FilePath)) + { + filePath = existing.FilePath; + } + else + { + var tileCenter = GeoUtils.TileToWorldPos(x, y, z); + var downloaded = await _downloader.DownloadSingleTileAsync(tileCenter.Lat, tileCenter.Lon, z, cancellationToken); + var entity = BuildTileEntity(downloaded, DateTime.UtcNow.Year); + await _tileRepository.InsertAsync(entity); + filePath = entity.FilePath; + } + + var bytes = await File.ReadAllBytesAsync(filePath, cancellationToken); + _cache.Set(cacheKey, bytes, new MemoryCacheEntryOptions + { + AbsoluteExpirationRelativeToNow = TileCacheAbsolute, + SlidingExpiration = TileCacheSliding + }); + + return new TileBytes(bytes, TileImageContentType, etag, TileResponseMaxAge); + } + + public async Task DownloadAndStoreSingleTileAsync( + double latitude, + double longitude, + int zoomLevel, + CancellationToken cancellationToken = default) + { + var downloaded = await _downloader.DownloadSingleTileAsync(latitude, longitude, zoomLevel, cancellationToken); + var entity = BuildTileEntity(downloaded, DateTime.UtcNow.Year); + await _tileRepository.InsertAsync(entity); + return MapToMetadata(entity); + } + + private static TileEntity BuildTileEntity(DownloadedTileInfoV2 downloaded, int currentVersion) + { + var now = DateTime.UtcNow; + return new TileEntity + { + Id = Guid.NewGuid(), + TileZoom = downloaded.ZoomLevel, + TileX = downloaded.X, + TileY = downloaded.Y, + Latitude = downloaded.CenterLatitude, + Longitude = downloaded.CenterLongitude, + TileSizeMeters = downloaded.TileSizeMeters, + TileSizePixels = 256, + ImageType = "jpg", + MapsVersion = $"downloaded_{now:yyyy-MM-dd}", + Version = currentVersion, + FilePath = downloaded.FilePath, + CreatedAt = now, + UpdatedAt = now + }; + } + private static TileMetadata MapToMetadata(TileEntity entity) { return new TileMetadata diff --git a/SatelliteProvider.Tests/InfrastructureTests.cs b/SatelliteProvider.Tests/InfrastructureTests.cs index ee92abb..3882989 100644 --- a/SatelliteProvider.Tests/InfrastructureTests.cs +++ b/SatelliteProvider.Tests/InfrastructureTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging.Abstractions; using Moq; using SatelliteProvider.Common.DTO; @@ -60,10 +61,11 @@ public class InfrastructureTests // Arrange var downloader = new Mock().Object; var tileRepo = new Mock().Object; + var cache = new MemoryCache(new MemoryCacheOptions()); var logger = NullLogger.Instance; // Act - var service = new TileService(downloader, tileRepo, logger); + var service = new TileService(downloader, tileRepo, cache, logger); // Assert service.Should().NotBeNull(); diff --git a/SatelliteProvider.Tests/SatelliteProvider.Tests.csproj b/SatelliteProvider.Tests/SatelliteProvider.Tests.csproj index ebecbd8..10bd3b1 100644 --- a/SatelliteProvider.Tests/SatelliteProvider.Tests.csproj +++ b/SatelliteProvider.Tests/SatelliteProvider.Tests.csproj @@ -12,6 +12,7 @@ + diff --git a/SatelliteProvider.Tests/TileServiceTests.cs b/SatelliteProvider.Tests/TileServiceTests.cs index 5121f85..83178bf 100644 --- a/SatelliteProvider.Tests/TileServiceTests.cs +++ b/SatelliteProvider.Tests/TileServiceTests.cs @@ -1,4 +1,5 @@ using FluentAssertions; +using Microsoft.Extensions.Caching.Memory; using Microsoft.Extensions.Logging.Abstractions; using Microsoft.Extensions.Options; using Moq; @@ -16,9 +17,14 @@ public class TileServiceTests { private static TileService BuildService( Mock downloader, - Mock tileRepo) + Mock tileRepo, + IMemoryCache? cache = null) { - return new TileService(downloader.Object, tileRepo.Object, NullLogger.Instance); + return new TileService( + downloader.Object, + tileRepo.Object, + cache ?? new MemoryCache(new MemoryCacheOptions()), + NullLogger.Instance); } private static DownloadedTileInfoV2 MakeDownloaded(int x, int y, int zoom, double lat, double lon, string filePath = "tiles/18/0/0/tile.jpg") @@ -216,6 +222,148 @@ public class TileServiceTests // Assert result.Should().BeNull(); } + + [Fact] + public async Task GetOrDownloadTileAsync_CacheHit_ReturnsCachedBytes_AZ310_AC2() + { + // Arrange + const int z = 18, x = 123, y = 456; + var downloader = new Mock(MockBehavior.Strict); + var tileRepo = new Mock(MockBehavior.Strict); + var cache = new MemoryCache(new MemoryCacheOptions()); + var cached = new byte[] { 1, 2, 3, 4 }; + cache.Set($"tile_{z}_{x}_{y}", cached); + var service = BuildService(downloader, tileRepo, cache); + + // Act + var result = await service.GetOrDownloadTileAsync(z, x, y); + + // Assert + result.Bytes.Should().BeSameAs(cached); + result.ContentType.Should().Be("image/jpeg"); + result.ETag.Should().Be($"\"{z}_{x}_{y}\""); + result.MaxAge.Should().Be(TimeSpan.FromDays(1)); + downloader.VerifyNoOtherCalls(); + tileRepo.VerifyNoOtherCalls(); + } + + [Fact] + public async Task GetOrDownloadTileAsync_RepoHit_ReadsFromDisk_NoDownloader_AZ310_AC3() + { + // Arrange + const int z = 18, x = 100, y = 200; + var tempPath = Path.Combine(Path.GetTempPath(), $"sp-tests-tile-{Guid.NewGuid():N}.jpg"); + var fileBytes = new byte[] { 9, 8, 7 }; + await File.WriteAllBytesAsync(tempPath, fileBytes); + + try + { + var downloader = new Mock(MockBehavior.Strict); + var tileRepo = new Mock(); + tileRepo + .Setup(r => r.GetByTileCoordinatesAsync(z, x, y)) + .ReturnsAsync(new TileEntity { Id = Guid.NewGuid(), TileZoom = z, TileX = x, TileY = y, FilePath = tempPath }); + + var service = BuildService(downloader, tileRepo); + + // Act + var result = await service.GetOrDownloadTileAsync(z, x, y); + + // Assert + result.Bytes.Should().Equal(fileBytes); + result.ContentType.Should().Be("image/jpeg"); + result.ETag.Should().Be($"\"{z}_{x}_{y}\""); + tileRepo.Verify(r => r.GetByTileCoordinatesAsync(z, x, y), Times.Once); + tileRepo.Verify(r => r.InsertAsync(It.IsAny()), Times.Never); + downloader.VerifyNoOtherCalls(); + } + finally + { + File.Delete(tempPath); + } + } + + [Fact] + public async Task GetOrDownloadTileAsync_DownloaderFallback_InsertsAndReturnsBytes_AZ310_AC4() + { + // Arrange + const int z = 18, x = 50, y = 60; + var tempPath = Path.Combine(Path.GetTempPath(), $"sp-tests-dl-{Guid.NewGuid():N}.jpg"); + var fileBytes = new byte[] { 5, 5, 5, 5 }; + await File.WriteAllBytesAsync(tempPath, fileBytes); + + try + { + var downloader = new Mock(); + var tileRepo = new Mock(); + tileRepo.Setup(r => r.GetByTileCoordinatesAsync(z, x, y)).ReturnsAsync((TileEntity?)null); + downloader + .Setup(d => d.DownloadSingleTileAsync(It.IsAny(), It.IsAny(), z, It.IsAny())) + .ReturnsAsync(new DownloadedTileInfoV2(x, y, z, 47.46, 37.65, tempPath, 100.0)); + + var service = BuildService(downloader, tileRepo); + + // Act + var result = await service.GetOrDownloadTileAsync(z, x, y); + + // Assert + result.Bytes.Should().Equal(fileBytes); + tileRepo.Verify(r => r.InsertAsync(It.Is(t => + t.TileZoom == z && t.TileX == x && t.TileY == y && t.FilePath == tempPath)), Times.Once); + downloader.Verify(d => d.DownloadSingleTileAsync(It.IsAny(), It.IsAny(), z, It.IsAny()), Times.Once); + } + finally + { + File.Delete(tempPath); + } + } + + [Fact] + public async Task DownloadAndStoreSingleTileAsync_HappyPath_CallsDownloaderAndRepo_AZ311_AC2() + { + // Arrange + const int zoom = 18; + var downloader = new Mock(); + var tileRepo = new Mock(); + downloader + .Setup(d => d.DownloadSingleTileAsync(TestCoordinates.TileLat, TestCoordinates.TileLon, zoom, It.IsAny())) + .ReturnsAsync(new DownloadedTileInfoV2(123, 456, zoom, TestCoordinates.TileLat, TestCoordinates.TileLon, "tiles/18/123/456.jpg", 100.0)); + + var service = BuildService(downloader, tileRepo); + + // Act + var result = await service.DownloadAndStoreSingleTileAsync(TestCoordinates.TileLat, TestCoordinates.TileLon, zoom); + + // Assert + result.TileZoom.Should().Be(zoom); + result.TileX.Should().Be(123); + result.TileY.Should().Be(456); + result.FilePath.Should().Be("tiles/18/123/456.jpg"); + result.TileSizePixels.Should().Be(256); + result.ImageType.Should().Be("jpg"); + downloader.Verify(d => d.DownloadSingleTileAsync(TestCoordinates.TileLat, TestCoordinates.TileLon, zoom, It.IsAny()), Times.Once); + tileRepo.Verify(r => r.InsertAsync(It.IsAny()), Times.Once); + } + + [Fact] + public async Task DownloadAndStoreSingleTileAsync_DownloaderThrows_DoesNotInsert_AZ311_AC2b() + { + // Arrange + var downloader = new Mock(); + var tileRepo = new Mock(); + downloader + .Setup(d => d.DownloadSingleTileAsync(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) + .ThrowsAsync(new InvalidOperationException("network down")); + + var service = BuildService(downloader, tileRepo); + + // Act + Func act = () => service.DownloadAndStoreSingleTileAsync(TestCoordinates.TileLat, TestCoordinates.TileLon, 18); + + // Assert + await act.Should().ThrowAsync().WithMessage("network down"); + tileRepo.Verify(r => r.InsertAsync(It.IsAny()), Times.Never); + } } public class GoogleMapsDownloaderZoomValidationTests diff --git a/_docs/02_tasks/todo/AZ-310_refactor_servetile_via_tileservice.md b/_docs/02_tasks/done/AZ-310_refactor_servetile_via_tileservice.md similarity index 100% rename from _docs/02_tasks/todo/AZ-310_refactor_servetile_via_tileservice.md rename to _docs/02_tasks/done/AZ-310_refactor_servetile_via_tileservice.md diff --git a/_docs/02_tasks/todo/AZ-311_refactor_gettilebylatlon_via_tileservice.md b/_docs/02_tasks/done/AZ-311_refactor_gettilebylatlon_via_tileservice.md similarity index 100% rename from _docs/02_tasks/todo/AZ-311_refactor_gettilebylatlon_via_tileservice.md rename to _docs/02_tasks/done/AZ-311_refactor_gettilebylatlon_via_tileservice.md diff --git a/_docs/03_implementation/batch_04_report.md b/_docs/03_implementation/batch_04_report.md new file mode 100644 index 0000000..f6f81fb --- /dev/null +++ b/_docs/03_implementation/batch_04_report.md @@ -0,0 +1,48 @@ +# Batch Report + +**Batch**: 4 (refactor 02-coupling-refactoring · Phase A) +**Tasks**: AZ-310, AZ-311 +**Date**: 2026-05-10 + +## Task Results + +| Task | Status | Files Modified | Tests | AC Coverage | Issues | +|------|--------|---------------|-------|-------------|--------| +| AZ-310 ServeTile via ITileService | Done | 5 src + 2 tests | 40/40 | 5/5 ACs covered | None | +| AZ-311 GetTileByLatLon via ITileService | Done | (shared) | (shared) | 3/3 ACs covered | None | + +## Files Changed + +- `SatelliteProvider.Common/DTO/TileBytes.cs` (new) +- `SatelliteProvider.Common/Interfaces/ITileService.cs` +- `SatelliteProvider.Services/SatelliteProvider.Services.csproj` +- `SatelliteProvider.Services/TileService.cs` +- `SatelliteProvider.Api/Program.cs` +- `SatelliteProvider.Tests/SatelliteProvider.Tests.csproj` +- `SatelliteProvider.Tests/TileServiceTests.cs` +- `SatelliteProvider.Tests/InfrastructureTests.cs` + +## AC Test Coverage + +8 of 8 ACs covered (5 unit-tested, 3 verified by static inspection of `Program.cs` + reachable via integration smoke run). + +## Code Review Verdict + +**PASS** — no findings. See `_docs/03_implementation/reviews/batch_04_review.md`. + +## Auto-Fix Attempts + +0 + +## Build / Tests + +- `dotnet build SatelliteProvider.sln --configuration Release` → 0 warnings, 0 errors +- `dotnet test SatelliteProvider.Tests` → 40/40 passed (35 baseline + 5 new) + +## Architecture-Baseline Impact + +- F3 (Medium — endpoint routing bypasses ITileService): resolved by this batch (code change). Doc resolution deferred to AZ-315. + +## Next Batch + +AZ-312 + AZ-313 + AZ-314 (Phase B — split `SatelliteProvider.Services` into 3 csprojs, rewire consumers, DI extension methods). diff --git a/_docs/03_implementation/reviews/batch_04_review.md b/_docs/03_implementation/reviews/batch_04_review.md new file mode 100644 index 0000000..8d22e1d --- /dev/null +++ b/_docs/03_implementation/reviews/batch_04_review.md @@ -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)` — 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)` — 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.