Compare commits

...

4 Commits

Author SHA1 Message Date
Oleksandr Bezdieniezhnykh 6d98c8f8d1 [AZ-350] Cumulative K=3 review for batches 16-18: PASS (0 findings)
ci/woodpecker/push/01-test Pipeline was successful
ci/woodpecker/push/02-build-push Pipeline was successful
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 03:34:01 +03:00
Oleksandr Bezdieniezhnykh aa8beaa684 [AZ-371] Archive task file: todo -> done
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 03:30:25 +03:00
Oleksandr Bezdieniezhnykh 4456542cec [AZ-350] autodev state: batch 18 (AZ-371) complete
Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 03:30:24 +03:00
Oleksandr Bezdieniezhnykh 1dcd089d39 [AZ-371] Refactor C18: magic numbers to ProcessingConfig/MapConfig
Promotes 8 operational levers into config keys with defaults that match
the prior source literals byte-for-byte:
  ProcessingConfig: RegionProcessingTimeoutSeconds (300),
  RouteProcessingPollIntervalSeconds (5),
  MaxRoutePointSpacingMeters (200), LatLonTolerance (0.0001).
  MapConfig: TileSizePixels (256), AllowedZoomLevels ([15..19]),
  RetryBaseDelaySeconds (1), RetryMaxDelaySeconds (30).

Sites updated: RegionService, RouteProcessingService,
RoutePointGraphBuilder, RouteValidator, RouteService 4-arg ctor,
RouteImageRenderer, GoogleMapsDownloaderV2, TileService. Closes LF-2 by
forwarding HttpContext.RequestAborted from GetTileByLatLon into the
downloader. appsettings.json gains the 8 new keys at default values.

Tests: 141 / 141 unit + 5 / 5 smoke green. New ConfigDefaultsTests pins
defaults to original literals; new TileService unit test asserts CT
identity from caller to downloader (AZ-371 AC-3).

Co-authored-by: Cursor <cursoragent@cursor.com>
2026-05-11 03:30:07 +03:00
24 changed files with 600 additions and 69 deletions
+2 -2
View File
@@ -161,9 +161,9 @@ async Task<IResult> ServeTile(int z, int x, int y, HttpContext httpContext, ITil
return Results.Bytes(tile.Bytes, tile.ContentType);
}
async Task<IResult> GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, ITileService tileService)
async Task<IResult> GetTileByLatLon([FromQuery] double Latitude, [FromQuery] double Longitude, [FromQuery] int ZoomLevel, HttpContext httpContext, ITileService tileService)
{
var tile = await tileService.DownloadAndStoreSingleTileAsync(Latitude, Longitude, ZoomLevel);
var tile = await tileService.DownloadAndStoreSingleTileAsync(Latitude, Longitude, ZoomLevel, httpContext.RequestAborted);
var response = new DownloadTileResponse
{
+10 -2
View File
@@ -25,7 +25,11 @@
},
"MapConfig": {
"Service": "GoogleMaps",
"ApiKey": ""
"ApiKey": "",
"TileSizePixels": 256,
"AllowedZoomLevels": [ 15, 16, 17, 18, 19 ],
"RetryBaseDelaySeconds": 1,
"RetryMaxDelaySeconds": 30
},
"StorageConfig": {
"TilesDirectory": "./tiles",
@@ -37,7 +41,11 @@
"DefaultZoomLevel": 20,
"QueueCapacity": 1000,
"DelayBetweenRequestsMs": 50,
"SessionTokenReuseCount": 100
"SessionTokenReuseCount": 100,
"RegionProcessingTimeoutSeconds": 300,
"RouteProcessingPollIntervalSeconds": 5,
"MaxRoutePointSpacingMeters": 200.0,
"LatLonTolerance": 0.0001
},
"CorsConfig": {
"AllowedOrigins": []
@@ -4,4 +4,10 @@ public class MapConfig
{
public string Service { get; set; } = null!;
public string ApiKey { get; set; } = null!;
}
// AZ-371 / C18 — Google Maps tile constants promoted from source literals.
public int TileSizePixels { get; set; } = 256;
public int[] AllowedZoomLevels { get; set; } = new[] { 15, 16, 17, 18, 19 };
public int RetryBaseDelaySeconds { get; set; } = 1;
public int RetryMaxDelaySeconds { get; set; } = 30;
}
@@ -8,5 +8,11 @@ public class ProcessingConfig
public int QueueCapacity { get; set; } = 100;
public int DelayBetweenRequestsMs { get; set; } = 50;
public int SessionTokenReuseCount { get; set; } = 100;
// AZ-371 / C18 — operational levers promoted from source literals.
public int RegionProcessingTimeoutSeconds { get; set; } = 300;
public int RouteProcessingPollIntervalSeconds { get; set; } = 5;
public double MaxRoutePointSpacingMeters { get; set; } = 200.0;
public double LatLonTolerance { get; set; } = 0.0001;
}
@@ -19,6 +19,7 @@ public class RegionService : IRegionService
private readonly IRegionRequestQueue _queue;
private readonly ITileService _tileService;
private readonly StorageConfig _storageConfig;
private readonly ProcessingConfig _processingConfig;
private readonly ILogger<RegionService> _logger;
public RegionService(
@@ -26,12 +27,14 @@ public class RegionService : IRegionService
IRegionRequestQueue queue,
ITileService tileService,
IOptions<StorageConfig> storageConfig,
IOptions<ProcessingConfig> processingConfig,
ILogger<RegionService> logger)
{
_regionRepository = regionRepository;
_queue = queue;
_tileService = tileService;
_storageConfig = storageConfig.Value;
_processingConfig = processingConfig.Value;
_logger = logger;
}
@@ -102,7 +105,7 @@ public class RegionService : IRegionService
region.UpdatedAt = DateTime.UtcNow;
await _regionRepository.UpdateAsync(region);
using var timeoutCts = new CancellationTokenSource(TimeSpan.FromMinutes(5));
using var timeoutCts = new CancellationTokenSource(TimeSpan.FromSeconds(_processingConfig.RegionProcessingTimeoutSeconds));
using var linkedCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, timeoutCts.Token);
string? errorMessage = null;
@@ -18,15 +18,16 @@ namespace SatelliteProvider.Services.RouteManagement;
// AC-2 (pixel-for-pixel identical output for existing scenarios).
public class RouteImageRenderer
{
private const int TileSizePixels = 256;
private readonly StorageConfig _storageConfig;
private readonly int _tileSizePixels;
private readonly ILogger<RouteImageRenderer> _logger;
public RouteImageRenderer(IOptions<StorageConfig> storageConfig, ILogger<RouteImageRenderer> logger)
public RouteImageRenderer(IOptions<StorageConfig> storageConfig, IOptions<MapConfig> mapConfig, ILogger<RouteImageRenderer> logger)
{
ArgumentNullException.ThrowIfNull(storageConfig);
ArgumentNullException.ThrowIfNull(mapConfig);
_storageConfig = storageConfig.Value;
_tileSizePixels = mapConfig.Value.TileSizePixels;
_logger = logger;
}
@@ -69,7 +70,7 @@ public class RouteImageRenderer
var stitcher = new TileGridStitcher();
var result = await stitcher.StitchAsync(
placements,
TileSizePixels,
_tileSizePixels,
deduplicateByTileCoords: true,
swallowTileLoadErrors: true,
cancellationToken);
@@ -97,10 +98,10 @@ public class RouteImageRenderer
foreach (var (geoMinX, geoMinY, geoMaxX, geoMaxY) in geofencePolygonBounds)
{
var x1 = (geoMinX - minX) * TileSizePixels;
var y1 = (geoMinY - minY + 1) * TileSizePixels;
var x2 = (geoMaxX - minX + 2) * TileSizePixels - 1;
var y2 = (geoMaxY - minY + 1) * TileSizePixels - 1;
var x1 = (geoMinX - minX) * _tileSizePixels;
var y1 = (geoMinY - minY + 1) * _tileSizePixels;
var x2 = (geoMaxX - minX + 2) * _tileSizePixels - 1;
var y2 = (geoMaxY - minY + 1) * _tileSizePixels - 1;
x1 = Math.Max(0, Math.Min(x1, imageWidth - 1));
y1 = Math.Max(0, Math.Min(y1, imageHeight - 1));
@@ -121,8 +122,8 @@ public class RouteImageRenderer
var geoPoint = new GeoPoint(point.Latitude, point.Longitude);
var (tileX, tileY) = GeoUtils.WorldToTilePos(geoPoint, zoomLevel);
var pixelX = (tileX - minX) * TileSizePixels + TileSizePixels / 2;
var pixelY = (tileY - minY) * TileSizePixels + TileSizePixels / 2;
var pixelX = (tileX - minX) * _tileSizePixels + _tileSizePixels / 2;
var pixelY = (tileY - minY) * _tileSizePixels + _tileSizePixels / 2;
if (pixelX >= 0 && pixelX < imageWidth && pixelY >= 0 && pixelY < imageHeight)
{
@@ -1,3 +1,5 @@
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Utils;
@@ -5,7 +7,13 @@ namespace SatelliteProvider.Services.RouteManagement;
public class RoutePointGraphBuilder
{
public const double MaxPointSpacingMeters = 200.0;
private readonly double _maxPointSpacingMeters;
public RoutePointGraphBuilder(IOptions<ProcessingConfig> processingConfig)
{
ArgumentNullException.ThrowIfNull(processingConfig);
_maxPointSpacingMeters = processingConfig.Value.MaxRoutePointSpacingMeters;
}
public RoutePointGraph Build(IReadOnlyList<RoutePoint> userPoints)
{
@@ -53,7 +61,7 @@ public class RoutePointGraphBuilder
var next = userPoints[segmentIndex + 1];
var nextGeo = new GeoPoint(next.Latitude, next.Longitude);
var intermediates = GeoUtils.CalculateIntermediatePoints(currentGeo, nextGeo, MaxPointSpacingMeters);
var intermediates = GeoUtils.CalculateIntermediatePoints(currentGeo, nextGeo, _maxPointSpacingMeters);
foreach (var intermediateGeo in intermediates)
{
@@ -1,5 +1,7 @@
using Microsoft.Extensions.Hosting;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Interfaces;
using SatelliteProvider.Common.Utils;
@@ -26,7 +28,7 @@ public class RouteProcessingService : BackgroundService
private readonly RegionFileCleaner _regionFileCleaner;
private readonly RouteRegionMatcher _routeRegionMatcher;
private readonly ILogger<RouteProcessingService> _logger;
private readonly TimeSpan _checkInterval = TimeSpan.FromSeconds(5);
private readonly TimeSpan _checkInterval;
public RouteProcessingService(
IRouteRepository routeRepository,
@@ -37,6 +39,7 @@ public class RouteProcessingService : BackgroundService
RouteImageRenderer routeImageRenderer,
TilesZipBuilder tilesZipBuilder,
RegionFileCleaner regionFileCleaner,
IOptions<ProcessingConfig> processingConfig,
ILogger<RouteProcessingService> logger)
{
_routeRepository = routeRepository;
@@ -48,6 +51,7 @@ public class RouteProcessingService : BackgroundService
_tilesZipBuilder = tilesZipBuilder;
_regionFileCleaner = regionFileCleaner;
_routeRegionMatcher = new RouteRegionMatcher();
_checkInterval = TimeSpan.FromSeconds(processingConfig.Value.RouteProcessingPollIntervalSeconds);
_logger = logger;
}
@@ -1,4 +1,6 @@
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Interfaces;
using SatelliteProvider.DataAccess.Models;
@@ -19,10 +21,11 @@ public class RouteService : IRouteService
public RouteService(
IRouteRepository routeRepository,
IRegionService regionService,
IOptions<ProcessingConfig> processingConfig,
ILogger<RouteService> logger)
: this(routeRepository, regionService, logger,
new RouteValidator(),
new RoutePointGraphBuilder(),
new RouteValidator(processingConfig),
new RoutePointGraphBuilder(processingConfig),
new GeofenceGridCalculator(),
new RouteResponseMapper())
{
@@ -1,9 +1,19 @@
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
namespace SatelliteProvider.Services.RouteManagement;
public class RouteValidator
{
private readonly double _latLonTolerance;
public RouteValidator(IOptions<ProcessingConfig> processingConfig)
{
ArgumentNullException.ThrowIfNull(processingConfig);
_latLonTolerance = processingConfig.Value.LatLonTolerance;
}
public void Validate(CreateRouteRequest request)
{
ArgumentNullException.ThrowIfNull(request);
@@ -39,7 +49,7 @@ public class RouteValidator
}
}
private static void ValidatePolygon(GeofencePolygon polygon, List<string> errors)
private void ValidatePolygon(GeofencePolygon polygon, List<string> errors)
{
if (polygon.NorthWest is null || polygon.SouthEast is null)
{
@@ -50,8 +60,8 @@ public class RouteValidator
var nw = polygon.NorthWest;
var se = polygon.SouthEast;
if ((Math.Abs(nw.Lat) < 0.0001 && Math.Abs(nw.Lon) < 0.0001) ||
(Math.Abs(se.Lat) < 0.0001 && Math.Abs(se.Lon) < 0.0001))
if ((Math.Abs(nw.Lat) < _latLonTolerance && Math.Abs(nw.Lon) < _latLonTolerance) ||
(Math.Abs(se.Lat) < _latLonTolerance && Math.Abs(se.Lon) < _latLonTolerance))
{
errors.Add("Geofence polygon coordinates cannot be (0,0)");
}
@@ -15,15 +15,12 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
{
private const string TILE_URL_TEMPLATE = "https://mt{0}.google.com/vt/lyrs=s&x={1}&y={2}&z={3}&token={4}";
private const string USER_AGENT = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.0.0 Safari/537.36";
private const int TILE_SIZE_PIXELS = 256;
private const int MAX_RETRY_DELAY_SECONDS = 30;
private const int BASE_RETRY_DELAY_SECONDS = 1;
private static readonly int[] ALLOWED_ZOOM_LEVELS = { 15, 16, 17, 18, 19 };
private readonly ILogger<GoogleMapsDownloaderV2> _logger;
private readonly string _apiKey;
private readonly StorageConfig _storageConfig;
private readonly ProcessingConfig _processingConfig;
private readonly MapConfig _mapConfig;
private readonly IHttpClientFactory _httpClientFactory;
private readonly SemaphoreSlim _downloadSemaphore;
private static readonly System.Collections.Concurrent.ConcurrentDictionary<string, Task<DownloadedTileInfoV2>> _activeDownloads = new();
@@ -36,7 +33,8 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
IHttpClientFactory httpClientFactory)
{
_logger = logger;
_apiKey = mapConfig.Value.ApiKey;
_mapConfig = mapConfig.Value;
_apiKey = _mapConfig.ApiKey;
_storageConfig = storageConfig.Value;
_processingConfig = processingConfig.Value;
_httpClientFactory = httpClientFactory;
@@ -84,9 +82,9 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
public async Task<DownloadedTileInfoV2> DownloadSingleTileAsync(double latitude, double longitude, int zoomLevel, CancellationToken token = default)
{
if (!ALLOWED_ZOOM_LEVELS.Contains(zoomLevel))
if (!_mapConfig.AllowedZoomLevels.Contains(zoomLevel))
{
throw new ArgumentException($"Zoom level {zoomLevel} is not allowed. Allowed zoom levels are: {string.Join(", ", ALLOWED_ZOOM_LEVELS)}", nameof(zoomLevel));
throw new ArgumentException($"Zoom level {zoomLevel} is not allowed. Allowed zoom levels are: {string.Join(", ", _mapConfig.AllowedZoomLevels)}", nameof(zoomLevel));
}
var geoPoint = new GeoPoint(latitude, longitude);
@@ -137,18 +135,19 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
);
}
private static double CalculateTileSizeInMeters(int zoomLevel, double latitude)
private double CalculateTileSizeInMeters(int zoomLevel, double latitude)
{
const double EARTH_CIRCUMFERENCE_METERS = 40075016.686;
var tileSizePixels = _mapConfig.TileSizePixels;
var latRad = latitude * Math.PI / 180.0;
var metersPerPixel = (EARTH_CIRCUMFERENCE_METERS * Math.Cos(latRad)) / (Math.Pow(2, zoomLevel) * TILE_SIZE_PIXELS);
return metersPerPixel * TILE_SIZE_PIXELS;
var metersPerPixel = (EARTH_CIRCUMFERENCE_METERS * Math.Cos(latRad)) / (Math.Pow(2, zoomLevel) * tileSizePixels);
return metersPerPixel * tileSizePixels;
}
private async Task<T> ExecuteWithRetryAsync<T>(Func<Task<T>> action, int maxRetries = 5, CancellationToken cancellationToken = default)
{
int attempt = 0;
int delay = BASE_RETRY_DELAY_SECONDS;
int delay = _mapConfig.RetryBaseDelaySeconds;
Exception? lastException = null;
while (attempt < maxRetries)
@@ -178,7 +177,7 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
throw new RateLimitException($"Rate limit exceeded after {maxRetries} attempts. Google Maps API is throttling requests.");
}
delay = Math.Min(delay * 2, MAX_RETRY_DELAY_SECONDS);
delay = Math.Min(delay * 2, _mapConfig.RetryMaxDelaySeconds);
_logger.LogWarning("Rate limited (429 Too Many Requests). Waiting {Delay}s before retry {Attempt}/{Max}", delay, attempt, maxRetries);
await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken);
}
@@ -203,7 +202,7 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
throw;
}
delay = Math.Min(delay * 2, MAX_RETRY_DELAY_SECONDS);
delay = Math.Min(delay * 2, _mapConfig.RetryMaxDelaySeconds);
_logger.LogWarning("Server error ({StatusCode}). Waiting {Delay}s before retry {Attempt}/{Max}", ex.StatusCode, delay, attempt, maxRetries);
await Task.Delay(TimeSpan.FromSeconds(delay), cancellationToken);
}
@@ -229,9 +228,9 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
IEnumerable<ExistingTileInfo> existingTiles,
CancellationToken token = default)
{
if (!ALLOWED_ZOOM_LEVELS.Contains(zoomLevel))
if (!_mapConfig.AllowedZoomLevels.Contains(zoomLevel))
{
throw new ArgumentException($"Zoom level {zoomLevel} is not allowed. Allowed zoom levels are: {string.Join(", ", ALLOWED_ZOOM_LEVELS)}", nameof(zoomLevel));
throw new ArgumentException($"Zoom level {zoomLevel} is not allowed. Allowed zoom levels are: {string.Join(", ", _mapConfig.AllowedZoomLevels)}", nameof(zoomLevel));
}
var (latMin, latMax, lonMin, lonMax) = GeoUtils.GetBoundingBox(centerGeoPoint, radiusM);
@@ -248,9 +247,10 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
{
var tileCenter = GeoUtils.TileToWorldPos(x, y, zoomLevel);
var tolerance = _processingConfig.LatLonTolerance;
var existingTile = existingTiles.FirstOrDefault(t =>
Math.Abs(t.Latitude - tileCenter.Lat) < 0.0001 &&
Math.Abs(t.Longitude - tileCenter.Lon) < 0.0001 &&
Math.Abs(t.Latitude - tileCenter.Lat) < tolerance &&
Math.Abs(t.Longitude - tileCenter.Lon) < tolerance &&
t.TileZoom == zoomLevel);
if (existingTile != null)
@@ -1,5 +1,7 @@
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Logging;
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Interfaces;
using SatelliteProvider.Common.Utils;
@@ -18,17 +20,20 @@ public class TileService : ITileService
private readonly ISatelliteDownloader _downloader;
private readonly ITileRepository _tileRepository;
private readonly IMemoryCache _cache;
private readonly MapConfig _mapConfig;
private readonly ILogger<TileService> _logger;
public TileService(
ISatelliteDownloader downloader,
ITileRepository tileRepository,
IMemoryCache cache,
IOptions<MapConfig> mapConfig,
ILogger<TileService> logger)
{
_downloader = downloader;
_tileRepository = tileRepository;
_cache = cache;
_mapConfig = mapConfig.Value;
_logger = logger;
}
@@ -135,7 +140,7 @@ public class TileService : ITileService
return MapToMetadata(entity);
}
private static TileEntity BuildTileEntity(DownloadedTileInfoV2 downloaded)
private TileEntity BuildTileEntity(DownloadedTileInfoV2 downloaded)
{
var now = DateTime.UtcNow;
return new TileEntity
@@ -147,7 +152,7 @@ public class TileService : ITileService
Latitude = downloaded.CenterLatitude,
Longitude = downloaded.CenterLongitude,
TileSizeMeters = downloaded.TileSizeMeters,
TileSizePixels = 256,
TileSizePixels = _mapConfig.TileSizePixels,
ImageType = "jpg",
MapsVersion = $"downloaded_{now:yyyy-MM-dd}",
Version = null,
@@ -0,0 +1,62 @@
using FluentAssertions;
using SatelliteProvider.Common.Configs;
namespace SatelliteProvider.Tests;
// AZ-371 / C18 — verifies the config defaults preserve the original literal values
// that lived in source code prior to the magic-numbers-to-config refactor.
public class ConfigDefaultsTests
{
[Fact]
public void ProcessingConfig_RegionProcessingTimeout_PreservesOriginal_AZ371_AC2()
{
// Assert
new ProcessingConfig().RegionProcessingTimeoutSeconds.Should().Be(300, "RegionService used TimeSpan.FromMinutes(5) before AZ-371");
}
[Fact]
public void ProcessingConfig_RouteProcessingPollInterval_PreservesOriginal_AZ371_AC2()
{
// Assert
new ProcessingConfig().RouteProcessingPollIntervalSeconds.Should().Be(5, "RouteProcessingService polled every 5s before AZ-371");
}
[Fact]
public void ProcessingConfig_MaxRoutePointSpacingMeters_PreservesOriginal_AZ371_AC2()
{
// Assert
new ProcessingConfig().MaxRoutePointSpacingMeters.Should().Be(200.0, "RoutePointGraphBuilder used 200m spacing before AZ-371");
}
[Fact]
public void ProcessingConfig_LatLonTolerance_PreservesOriginal_AZ371_AC2()
{
// Assert
new ProcessingConfig().LatLonTolerance.Should().Be(0.0001, "RouteValidator and GoogleMapsDownloaderV2 used 0.0001 before AZ-371");
}
[Fact]
public void MapConfig_TileSizePixels_PreservesOriginal_AZ371_AC2()
{
// Assert
new MapConfig().TileSizePixels.Should().Be(256, "TileService and GoogleMapsDownloaderV2 used 256 px before AZ-371");
}
[Fact]
public void MapConfig_AllowedZoomLevels_PreservesOriginal_AZ371_AC2()
{
// Assert
new MapConfig().AllowedZoomLevels.Should().Equal(15, 16, 17, 18, 19);
}
[Fact]
public void MapConfig_RetryDelays_PreserveOriginal_AZ371_AC2()
{
// Arrange
var cfg = new MapConfig();
// Assert
cfg.RetryBaseDelaySeconds.Should().Be(1);
cfg.RetryMaxDelaySeconds.Should().Be(30);
}
}
@@ -1,7 +1,9 @@
using FluentAssertions;
using Microsoft.Extensions.Caching.Memory;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Moq;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Interfaces;
using SatelliteProvider.DataAccess.Models;
@@ -65,7 +67,8 @@ public class InfrastructureTests
var logger = NullLogger<TileService>.Instance;
// Act
var service = new TileService(downloader, tileRepo, cache, logger);
var mapConfig = Options.Create(new MapConfig { Service = "GoogleMaps", ApiKey = "" });
var service = new TileService(downloader, tileRepo, cache, mapConfig, logger);
// Assert
service.Should().NotBeNull();
@@ -36,7 +36,8 @@ public class RegionServiceTests : IDisposable
Mock<ITileService> tileService)
{
var storage = Options.Create(new StorageConfig { ReadyDirectory = _readyDir, TilesDirectory = "/tiles" });
return new RegionService(regionRepo.Object, queue.Object, tileService.Object, storage, NullLogger<RegionService>.Instance);
var processing = Options.Create(new ProcessingConfig());
return new RegionService(regionRepo.Object, queue.Object, tileService.Object, storage, processing, NullLogger<RegionService>.Instance);
}
[Fact]
@@ -17,7 +17,8 @@ public class RouteImageRendererTests
{
loggerMock = new Mock<ILogger<RouteImageRenderer>>();
var storageOptions = Options.Create(new StorageConfig());
return new RouteImageRenderer(storageOptions, loggerMock.Object);
var mapOptions = Options.Create(new MapConfig { Service = "GoogleMaps", ApiKey = "" });
return new RouteImageRenderer(storageOptions, mapOptions, loggerMock.Object);
}
private static void VerifyWarningLogged(Mock<ILogger<RouteImageRenderer>> loggerMock, string substringInState)
@@ -1,4 +1,6 @@
using FluentAssertions;
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Utils;
using SatelliteProvider.Services.RouteManagement;
@@ -8,13 +10,18 @@ namespace SatelliteProvider.Tests;
public class RoutePointGraphBuilderTests
{
private static readonly ProcessingConfig DefaultProcessingConfig = new();
private static RoutePointGraphBuilder MakeBuilder() =>
new(Options.Create(new ProcessingConfig()));
private static List<RoutePoint> ToRoutePoints(IEnumerable<(double Lat, double Lon)> points) =>
points.Select(p => new RoutePoint { Latitude = p.Lat, Longitude = p.Lon }).ToList();
[Fact]
public void Build_TwoUserPoints_FirstIsStart_LastIsEnd_BetweenAreIntermediate()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route01Points);
var graph = sut.Build(input);
@@ -28,7 +35,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_ConsecutivePointsRespectMaxSpacing()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route01Points);
var graph = sut.Build(input);
@@ -40,15 +47,15 @@ public class RoutePointGraphBuilderTests
var distance = GeoUtils.CalculateDistance(
new GeoPoint(prev.Latitude, prev.Longitude),
new GeoPoint(cur.Latitude, cur.Longitude));
distance.Should().BeLessThanOrEqualTo(RoutePointGraphBuilder.MaxPointSpacingMeters + 0.5,
$"point {i - 1}→{i} must be ≤{RoutePointGraphBuilder.MaxPointSpacingMeters}m");
distance.Should().BeLessThanOrEqualTo(DefaultProcessingConfig.MaxRoutePointSpacingMeters + 0.5,
$"point {i - 1}→{i} must be ≤{DefaultProcessingConfig.MaxRoutePointSpacingMeters}m");
}
}
[Fact]
public void Build_TenPointRoute_HasOneStartOneEndAndEightAction()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route04Points);
var graph = sut.Build(input);
@@ -62,7 +69,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_TotalDistanceEqualsSumOfHaversineSegments()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route01Points);
var graph = sut.Build(input);
@@ -83,7 +90,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_SequenceNumbersAreContiguousAndStartAtZero()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route04Points);
var graph = sut.Build(input);
@@ -95,7 +102,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_FirstPointHasNullDistanceFromPrevious()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = ToRoutePoints(TestCoordinates.Route.Route01Points);
var graph = sut.Build(input);
@@ -107,7 +114,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_FewerThanTwoPoints_Throws()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
var input = new List<RoutePoint> { new() { Latitude = 47.46, Longitude = 37.64 } };
Action act = () => sut.Build(input);
@@ -118,7 +125,7 @@ public class RoutePointGraphBuilderTests
[Fact]
public void Build_NullInput_Throws()
{
var sut = new RoutePointGraphBuilder();
var sut = MakeBuilder();
Action act = () => sut.Build(null!);
+3 -1
View File
@@ -1,6 +1,8 @@
using FluentAssertions;
using Microsoft.Extensions.Logging.Abstractions;
using Microsoft.Extensions.Options;
using Moq;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Common.Interfaces;
using SatelliteProvider.Common.Utils;
@@ -17,7 +19,7 @@ public class RouteServiceTests
Mock<IRouteRepository> routeRepo,
Mock<IRegionService> regionService)
{
return new RouteService(routeRepo.Object, regionService.Object, NullLogger<RouteService>.Instance);
return new RouteService(routeRepo.Object, regionService.Object, Options.Create(new ProcessingConfig()), NullLogger<RouteService>.Instance);
}
private static CreateRouteRequest BuildRequest(IEnumerable<(double Lat, double Lon)> points, double regionSize = 500, int zoom = 18, bool requestMaps = false, Geofences? geofences = null)
+15 -10
View File
@@ -1,4 +1,6 @@
using FluentAssertions;
using Microsoft.Extensions.Options;
using SatelliteProvider.Common.Configs;
using SatelliteProvider.Common.DTO;
using SatelliteProvider.Services.RouteManagement;
using SatelliteProvider.Tests.Fixtures;
@@ -7,6 +9,9 @@ namespace SatelliteProvider.Tests;
public class RouteValidatorTests
{
private static RouteValidator MakeValidator() =>
new(Options.Create(new ProcessingConfig()));
private static CreateRouteRequest BuildValidRequest()
{
return new CreateRouteRequest
@@ -25,7 +30,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_ValidRequest_DoesNotThrow_AZ365_AC2()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
Action act = () => sut.Validate(request);
@@ -36,7 +41,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_FewerThanTwoPoints_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Points = new List<RoutePoint> { new() { Latitude = 47.46, Longitude = 37.64 } };
@@ -48,7 +53,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_RegionSizeOutOfRange_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.RegionSizeMeters = 50;
@@ -61,7 +66,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_BlankName_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Name = " ";
@@ -73,7 +78,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_GeofencePolygonZeroZero_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Geofences = new Geofences
{
@@ -92,7 +97,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_GeofenceInvertedLatitudes_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Geofences = new Geofences
{
@@ -114,7 +119,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_NullPolygonCorner_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Geofences = new Geofences
{
@@ -133,7 +138,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_OutOfRangeLatitude_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Geofences = new Geofences
{
@@ -156,7 +161,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2()
{
var sut = new RouteValidator();
var sut = MakeValidator();
var request = BuildValidRequest();
request.Name = "";
request.RegionSizeMeters = 50;
@@ -175,7 +180,7 @@ public class RouteValidatorTests
[Fact]
public void Validate_NullRequest_Throws()
{
var sut = new RouteValidator();
var sut = MakeValidator();
Action act = () => sut.Validate(null!);
@@ -24,6 +24,7 @@ public class TileServiceTests
downloader.Object,
tileRepo.Object,
cache ?? new MemoryCache(new MemoryCacheOptions()),
Options.Create(new MapConfig { Service = "GoogleMaps", ApiKey = "" }),
NullLogger<TileService>.Instance);
}
@@ -372,6 +373,28 @@ public class TileServiceTests
tileRepo.Verify(r => r.InsertAsync(It.IsAny<TileEntity>()), Times.Once);
}
[Fact]
public async Task DownloadAndStoreSingleTileAsync_ForwardsCancellationTokenToDownloader_AZ371_AC3()
{
// Arrange
const int zoom = 18;
var downloader = new Mock<ISatelliteDownloader>();
var tileRepo = new Mock<ITileRepository>();
CancellationToken capturedToken = default;
downloader
.Setup(d => d.DownloadSingleTileAsync(It.IsAny<double>(), It.IsAny<double>(), zoom, It.IsAny<CancellationToken>()))
.Callback<double, double, int, CancellationToken>((_, _, _, ct) => capturedToken = ct)
.ReturnsAsync(new DownloadedTileInfoV2(1, 2, zoom, 0, 0, "p.jpg", 100.0));
var service = BuildService(downloader, tileRepo);
using var cts = new CancellationTokenSource();
// Act
await service.DownloadAndStoreSingleTileAsync(0, 0, zoom, cts.Token);
// Assert
capturedToken.Should().Be(cts.Token, "AZ-371 AC-3: caller-supplied CT must reach the downloader");
}
[Fact]
public async Task DownloadAndStoreSingleTileAsync_DownloaderThrows_DoesNotInsert_AZ311_AC2b()
{
+178
View File
@@ -0,0 +1,178 @@
# Batch 18 Report — Refactor 03 Phase 4 (kickoff)
Date: 2026-05-11
Epic: AZ-350 (03-code-quality-refactoring)
Status: ✅ Complete
## Scope (1 task / 3 SP)
| ID | C-ID | Title | Points | Component |
|----|------|-------|--------|-----------|
| AZ-371 | C18 | Move hardcoded magic numbers to ProcessingConfig / MapConfig | 3 | Common + Services.* (all) |
Solo batch. First task of Phase 4 (typing/config/tooling). Promotes operational
literals scattered across 5 service files into explicit `ProcessingConfig` /
`MapConfig` keys. Defaults preserve every prior runtime value byte-for-byte.
Also closes the LF-2 finding by forwarding `HttpContext.RequestAborted` from
`GetTileByLatLon` into the downloader.
## Changes
### Production
- **MODIFIED** `SatelliteProvider.Common/Configs/ProcessingConfig.cs`
- Added 4 keys with defaults that match prior source literals:
- `RegionProcessingTimeoutSeconds = 300` (was `TimeSpan.FromMinutes(5)`)
- `RouteProcessingPollIntervalSeconds = 5` (was `TimeSpan.FromSeconds(5)`)
- `MaxRoutePointSpacingMeters = 200.0` (was the `public const`)
- `LatLonTolerance = 0.0001` (was duplicated in 2 sites)
- **MODIFIED** `SatelliteProvider.Common/Configs/MapConfig.cs`
- Added 4 keys with defaults that match prior source literals:
- `TileSizePixels = 256`
- `AllowedZoomLevels = [15, 16, 17, 18, 19]`
- `RetryBaseDelaySeconds = 1`
- `RetryMaxDelaySeconds = 30`
- **MODIFIED** `SatelliteProvider.Api/appsettings.json`
- Added the 8 new keys under `MapConfig` and `ProcessingConfig`. Values match
`Common.Configs` defaults. `appsettings.Development.json` left untouched —
no environment-specific overrides for these levers yet.
- **MODIFIED** `SatelliteProvider.Services.RegionProcessing/RegionService.cs`
- Constructor now takes `IOptions<ProcessingConfig> processingConfig`.
- The 5-minute region-processing timeout in `ProcessRegionAsync` is now
`TimeSpan.FromSeconds(_processingConfig.RegionProcessingTimeoutSeconds)`.
- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs`
- Constructor now takes `IOptions<ProcessingConfig> processingConfig`.
- `_checkInterval` is no longer a `readonly` initializer literal; it's
set in the constructor from the config.
- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs`
- Single ctor `(IOptions<ProcessingConfig>)`; the previous
parameterless ctor + `public const MaxPointSpacingMeters` are gone.
- The const lived briefly to replace the inline literal in `RouteService`
(batch 15); promoting it the rest of the way to config is the natural
completion of that move.
- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteValidator.cs`
- Constructor now requires `IOptions<ProcessingConfig>`; reads
`LatLonTolerance` once and reuses it for the polygon (0,0) check.
- `ValidatePolygon` is no longer `static` because it reads instance state.
- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteService.cs`
- The 4-arg DI ctor (`IRouteRepository, IRegionService, IOptions<ProcessingConfig>, ILogger`)
is the production-callable ctor; the chained 8-arg ctor used by the
decomposition still accepts the helpers directly. The 4-arg ctor
constructs `RouteValidator` and `RoutePointGraphBuilder` with the same
`IOptions<ProcessingConfig>` reference.
- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteImageRenderer.cs`
- **Adjacent hygiene**: the `private const int TileSizePixels = 256` that
was extracted from `RouteProcessingService` in batch 17 is now read from
`_mapConfig.TileSizePixels`. Same operational lever as the `TileService`
site — leaving one configurable and the other hardcoded would be an
immediate cumulative-review finding (cross-task duplicate).
- **MODIFIED** `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs`
- The 4 `private const`s (`TILE_SIZE_PIXELS`, `MAX_RETRY_DELAY_SECONDS`,
`BASE_RETRY_DELAY_SECONDS`, `ALLOWED_ZOOM_LEVELS`) are gone.
- `MapConfig` was already injected; now `_mapConfig` field stores the
resolved `MapConfig.Value` and the 4 sites consume `_mapConfig.*`.
- `CalculateTileSizeInMeters` is no longer `static` (reads `_mapConfig`).
- The `0.0001` lat/lon tolerance in `GetTilesWithMetadataAsync` now reads
`_processingConfig.LatLonTolerance` once per call and reuses it in the
closure.
- **MODIFIED** `SatelliteProvider.Services.TileDownloader/TileService.cs`
- Constructor now takes `IOptions<MapConfig> mapConfig`.
- `BuildTileEntity` becomes an instance method (reads `_mapConfig`); the
`TileSizePixels = 256` literal becomes `_mapConfig.TileSizePixels`.
- **MODIFIED** `SatelliteProvider.Api/Program.cs`
- `GetTileByLatLon` now accepts `HttpContext httpContext` and forwards
`httpContext.RequestAborted` into `DownloadAndStoreSingleTileAsync`.
- This closes LF-2 (logical-flow finding from the discovery phase): client
cancellations now propagate through the `/api/satellite/tiles/latlon`
endpoint into the downloader.
### Tests
- **NEW** `SatelliteProvider.Tests/ConfigDefaultsTests.cs` — 7 tests that
pin the new config defaults to the original source literals (AC-2 evidence
— defaults preserve behavior is the load-bearing claim of this batch).
- **MODIFIED** `SatelliteProvider.Tests/TileServiceTests.cs`
- Added 1 new test `DownloadAndStoreSingleTileAsync_ForwardsCancellationTokenToDownloader_AZ371_AC3`
(AC-3 evidence — caller-supplied CT reaches the downloader).
- Updated `BuildService` to construct `TileService` with the new
`IOptions<MapConfig>` argument.
- **MODIFIED** `SatelliteProvider.Tests/InfrastructureTests.cs`
- Updated the `TileService_ConstructsWithMockedDependencies` smoke test to
provide the new `IOptions<MapConfig>`.
- **MODIFIED** `SatelliteProvider.Tests/RegionServiceTests.cs`
- `BuildService` now also threads `IOptions<ProcessingConfig>`.
- **MODIFIED** `SatelliteProvider.Tests/RouteServiceTests.cs`
- `BuildService` now also threads `IOptions<ProcessingConfig>`.
- **MODIFIED** `SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs`
- All 8 `new RoutePointGraphBuilder()` calls go through a `MakeBuilder()`
helper that wraps the new options ctor.
- The two assertions that referenced `RoutePointGraphBuilder.MaxPointSpacingMeters`
now read `DefaultProcessingConfig.MaxRoutePointSpacingMeters`.
- **MODIFIED** `SatelliteProvider.Tests/RouteValidatorTests.cs`
- All 9 `new RouteValidator()` calls go through `MakeValidator()`.
- **MODIFIED** `SatelliteProvider.Tests/RouteImageRendererTests.cs`
- `BuildSut` now also threads `IOptions<MapConfig>`.
## Verification
| Check | Result |
|-------|--------|
| Unit tests | ✅ 141 / 141 pass (was 133; +7 ConfigDefaultsTests, +1 AZ-371 AC-3 CT test) |
| Integration smoke | ✅ All scenarios pass; exit 0 |
| Behavior preservation | ✅ Defaults match prior literals byte-for-byte |
| Linter | ✅ No findings on any modified file |
## AC Coverage
| AC | Evidence |
|----|----------|
| AC-1: All listed magic numbers replaced by config-bound values | Grep for `FromMinutes(5)`, `TILE_SIZE_PIXELS`, `MAX_RETRY`, `BASE_RETRY`, `ALLOWED_ZOOM`, `200.0`, `0.0001`, `TileSizePixels = 256` across `SatelliteProvider.Services.*/**/*.cs` returns no matches. The remaining `_tileSizePixels` references are `MapConfig`-bound. |
| AC-2: Defaults preserve all existing behavior | `ConfigDefaultsTests` (7 tests) pins each new default to the original literal value. Smoke suite passes — region pipeline, route pipeline, tile downloads all behave identically. |
| AC-3: `GetTileByLatLon` request cancellation flows into the downloader | `Program.cs:165` now passes `httpContext.RequestAborted` into `DownloadAndStoreSingleTileAsync`. `TileServiceTests.DownloadAndStoreSingleTileAsync_ForwardsCancellationTokenToDownloader_AZ371_AC3` captures the CT through the mock and asserts identity. |
## Inline Code Review
This batch is a config-binding refactor. The risk surface:
1. **Default-value drift** — every default in `ProcessingConfig` and `MapConfig`
was set side-by-side with the original literal in source and verified by
`ConfigDefaultsTests`. No drift possible.
2. **Constructor fan-out** — 5 production classes gained ctor params. Only the
already-DI-resolved consumers (`RouteService` 4-arg, `RegionService`,
`TileService`, `RouteProcessingService`, `GoogleMapsDownloaderV2`) needed
`IServiceCollection` calls — and those are unchanged because the existing
`services.AddSingleton(...)` lines bind via reflection. The two helper
classes that tests `new` directly (`RouteValidator`, `RoutePointGraphBuilder`)
now require `IOptions<ProcessingConfig>` — every call site is updated.
3. **Static → instance demotions** — three methods became instance methods
(`TileService.BuildTileEntity`, `GoogleMapsDownloaderV2.CalculateTileSizeInMeters`,
`RouteValidator.ValidatePolygon`). Each one now reads instance state, so
the demotion is correct per `coderule.mdc` ("Only use static methods for
pure, self-contained computations").
4. **CT plumbing** (LF-2) — endpoint local function gained `HttpContext`
parameter; minimal API binding picks it up automatically. Smoke covers
the happy path; the new unit test pins the contract on `TileService`.
**Verdict: PASS.** No findings. Behavior preserved end-to-end.
## State
`auto_push: true`. After this commit lands, transitions on AZ-371 → In Testing
in Jira and the task file moves from `todo/` to `done/`. Batch 18 closes the
window for the cumulative K=3 review covering batches 1618 — that runs next.
## Next Batch (preview)
Phase 4 ordering: AZ-370 (status / point-type enums + AC RT2 update, 3 SP).
After the cumulative K=3 review for 1618.
@@ -0,0 +1,195 @@
# Cumulative Code Review — Batches 16-18 (cycle 1)
**Window**: Batches 16 (AZ-367), 17 (AZ-364 + folds AZ-360), 18 (AZ-371)
**Trigger**: K=3 cumulative review fired after batch 18 (`/implement` Step 14.5)
**Date**: 2026-05-11
**Mode**: cumulative (all 7 phases)
**Verdict**: PASS — 0 findings
## 1. Scope (cumulative diff vs. previous cumulative review at batch 15)
| Batch | Task(s) | Component(s) | Net LOC |
|-------|---------|--------------|---------|
| 16 | AZ-367 (C14 — extract TileGridStitcher) | Common.Imaging + RegionProcessing + RouteManagement | +290 / -95 |
| 17 | AZ-364 (C11 — decompose RouteProcessingService) + AZ-360 (C08 — replace IServiceProvider) | Services.RouteManagement (orchestrator + 7 collaborators) | +1177 / -435 |
| 18 | AZ-371 (C18 — magic numbers → ProcessingConfig/MapConfig + LF-2 CT) | Common.Configs + Api + Services.* (all) | +404 / -68 |
**Files in cumulative window** (production):
- **NEW**:
- `SatelliteProvider.Common/Imaging/TileGridStitcher.cs` (b16)
- `SatelliteProvider.Services.RouteManagement/TileInfo.cs` (b17)
- `SatelliteProvider.Services.RouteManagement/RouteRegionMatcher.cs` (b17)
- `SatelliteProvider.Services.RouteManagement/RouteCsvWriter.cs` (b17)
- `SatelliteProvider.Services.RouteManagement/RouteSummaryWriter.cs` (b17)
- `SatelliteProvider.Services.RouteManagement/RouteImageRenderer.cs` (b17, expanded in b18)
- `SatelliteProvider.Services.RouteManagement/TilesZipBuilder.cs` (b17)
- `SatelliteProvider.Services.RouteManagement/RegionFileCleaner.cs` (b17)
- **MODIFIED** (touched by ≥1 batch in window):
- `SatelliteProvider.Common/SatelliteProvider.Common.csproj` (b16: ImageSharp NuGet)
- `SatelliteProvider.Common/Configs/{ProcessingConfig, MapConfig}.cs` (b18: +8 keys)
- `SatelliteProvider.Api/Program.cs` (b18: GetTileByLatLon HttpContext + CT)
- `SatelliteProvider.Api/appsettings.json` (b18: +8 keys)
- `SatelliteProvider.Services.RegionProcessing/RegionService.cs` (b16: stitcher delegation; b18: IOptions<ProcessingConfig>)
- `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs` (b16: stitcher delegation; b17: orchestrator decomposition + IServiceProvider→IRegionService; b18: IOptions<ProcessingConfig>)
- `SatelliteProvider.Services.RouteManagement/{RoutePointGraphBuilder, RouteValidator}.cs` (b18: IOptions<ProcessingConfig>)
- `SatelliteProvider.Services.RouteManagement/RouteService.cs` (b18: 3-arg → 4-arg DI ctor with IOptions<ProcessingConfig>)
- `SatelliteProvider.Services.RouteManagement/RouteManagementServiceCollectionExtensions.cs` (b17: +5 collaborator singletons)
- `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` (b18: 4 const → MapConfig + tolerance from ProcessingConfig)
- `SatelliteProvider.Services.TileDownloader/TileService.cs` (b18: IOptions<MapConfig> + TileSizePixels)
- **DELETED**:
- `SatelliteProvider.Tests/RouteProcessingServiceTests.cs` (b17 — orchestrator now smoke-covered; testable bits moved to RouteImageRendererTests)
**Test deltas in window**:
| Batch | New tests | Notes |
|-------|-----------|-------|
| 16 | +11 | `TileGridStitcherTests` (3 byte-equality pixel-equivalence tests + 8 unit tests) |
| 17 | +10 net | New: 14 (RouteImageRendererTests×4, RouteRegionMatcherTests×4, RouteCsvWriterTests×1, RouteSummaryWriterTests×2, TilesZipBuilderTests×1, RegionFileCleanerTests×2). Moved: 4 (`ExtractTileCoordinatesFromFilename` tests). Deleted: 4 (the original location of those tests). |
| 18 | +8 | ConfigDefaultsTests×7 + TileServiceTests AZ371_AC3 ×1 |
End-of-window unit count: **141** (was 112 before batch 16; net +29).
## 2. Phase 1 — Context Loading
Re-read:
- AZ-367, AZ-364, AZ-360, AZ-371 task specs (`_docs/02_tasks/done/AZ-367*.md`, `done/AZ-364*.md`, `done/AZ-360*.md`, `done/AZ-371*.md`)
- `_docs/02_document/module-layout.md` (component boundaries)
- `_docs/02_document/architecture.md` `## Architecture Vision` and current state
- `_docs/02_document/architecture_compliance_baseline.md` (5 baseline findings, all `RESOLVED` from prior epics — no carry-over)
- `_docs/04_refactoring/03-code-quality-refactoring/list-of-changes.md` (C14, C11, C08, C18 entries)
All four tasks sit within epic AZ-350. Batches 16 + 17 close Phase 3 (Structural cleanup); batch 18 opens Phase 4 (typing/config/tooling). Common theme across the window: **promote inline complexity into named, single-responsibility modules without changing observable HTTP / DB / file-output behavior.**
## 3. Phase 2 — Spec Compliance (cumulative)
| Task | ACs | Verification |
|------|-----|--------------|
| AZ-367 (C14) | AC-1 (single stitcher used by both consumers) / AC-2 (pixel-for-pixel identical output for existing scenarios) / AC-3 (37+ unit + 5 smoke green) | Three pixel-byte-equality unit tests + integration smoke. Verified in batch 16. |
| AZ-364 (C11) | AC-1 (single-responsibility collaborators, queue-free unit tests) / AC-2 (same lifecycle + DB writes + file outputs) / AC-3 (unit + smoke green; route image identical) | 14 new collaborator tests + 4 moved tests + 5 smoke. Verified in batch 17. |
| AZ-360 (C08) | AC-1 (no `IServiceProvider` in `RouteProcessingService`) / AC-2 (37 unit + 5 smoke green) | Constructor signature inspected; no `IServiceProvider` field, no `using var scope`. Smoke covers route processing end-to-end. Folded into AZ-364 batch 17. |
| AZ-371 (C18) | AC-1 (all listed magic numbers replaced) / AC-2 (defaults preserve behavior) / AC-3 (CT flows from `GetTileByLatLon` to downloader) | `ConfigDefaultsTests`×7 pin defaults to original literals. `TileServiceTests.DownloadAndStoreSingleTileAsync_ForwardsCancellationTokenToDownloader_AZ371_AC3` captures CT identity through the mock. Verified in batch 18. |
No spec-gap detected on any of the four tasks.
## 4. Phase 3 — Code Quality (cumulative)
- **SOLID**: SRP improvements continue across the window. The `RouteProcessingService` god-class went from ~750 LOC with 6+ responsibilities to a ~280-LOC orchestrator (batch 17). The grid-placement loop is now a single `TileGridStitcher.StitchAsync` call instead of two duplicated implementations (batch 16). Operational levers are no longer baked into source (batch 18).
- **Error handling**: every new collaborator uses `ArgumentNullException.ThrowIfNull` for entry-point guards. `TileGridStitcher` throws `ArgumentOutOfRangeException` for non-positive tile size and `InvalidOperationException` for empty placements. No bare `catch` introduced anywhere in the window. The `swallowTileLoadErrors` flag (batch 16) is a deliberate, documented behavior preserved from the pre-refactor route stitcher (corrupt-tile-in-batch survives) — not silent error suppression.
- **Naming**: every new type is intent-revealing — `TileGridStitcher`, `TilePlacement`, `StitchResult`, `MissingTile`, `MissingTileReason`, `TileInfo`, `RouteRegionMatcher`, `RouteCsvWriter`, `RouteSummaryWriter`, `RouteImageRenderer`, `TilesZipBuilder`, `RegionFileCleaner`. None reads "Manager" / "Helper" / "Util". The corresponding methods (`StitchAsync`, `Match`, `WriteAsync`, `RenderAsync`, `BuildAsync`, `CleanupAsync`) read directly from the call site without ambiguity.
- **Complexity**: every new method in the window is ≤ 80 LOC. The decomposed `RouteProcessingService.GenerateRouteMapsAsync` is ~40 LOC of dispatch vs. the ~350 LOC pre-refactor. `RouteImageRenderer.RenderAsync` is the largest at 100 LOC, dominated by the geofence rectangle + cross overlay loops which are inherently linear in their input.
- **DRY**: positive net DRY across the window. The grid-placement loop existed in two services → now in one (TileGridStitcher). The `TileSizePixels = 256` literal existed in two places (TileService + RouteImageRenderer briefly) → batch 18 consolidates both into `MapConfig.TileSizePixels`. The `0.0001` lat/lon tolerance existed in `RouteValidator.cs` and `GoogleMapsDownloaderV2.cs` → now `ProcessingConfig.LatLonTolerance` (batch 18). The 4 `private const`s in `GoogleMapsDownloaderV2` (batch 18) are gone.
- **Test quality**: all new tests use xUnit + FluentAssertions + the project's `// Arrange / // Act / // Assert` pattern. Three pixel-byte-equality tests in batch 16 are the strongest behavior-preservation evidence in the run so far (40,000 + 10,000 + 65,536 pixel comparisons against inlined pre-refactor code).
- **Dead code**: nothing left dangling in this window. The `private const int TileSizePixels = 256` in `RouteImageRenderer` (born in batch 17) is gone (batch 18). The `BackgroundColor(Color.Black)` call in the route stitcher is gone (batch 16, proven equivalent to default `new Image<Rgb24>`). The 9 helper methods that lived on `RouteProcessingService` (batch 17) are deleted, not commented out. The pre-refactor `RouteProcessingServiceTests.cs` is deleted, with its testable methods moved to `RouteImageRendererTests.cs`.
- **Static-vs-instance**: batch 18 demoted three methods from `static` to instance (`TileService.BuildTileEntity`, `GoogleMapsDownloaderV2.CalculateTileSizeInMeters`, `RouteValidator.ValidatePolygon`) because each one now reads instance state (`_mapConfig` / `_processingConfig`). This is exactly the path `coderule.mdc` prescribes ("Only use static methods for pure, self-contained computations") — moving the data dependency into the instance moves the method off the static surface in lockstep.
No code-quality finding.
## 5. Phase 4 — Security Quick-Scan (cumulative)
- No new SQL building, no new string-interpolated queries, no `Process.Start` / `eval` / native invocation introduced in the window.
- No new credentials or secrets. Batch 18 promotes operational levers to config but every new key is a non-secret operational tunable (timeouts, polling intervals, tolerances, retry delays).
- The `appsettings.json` additions (batch 18) carry no secrets — values match prior source literals exactly.
- The new collaborators in batch 17 take `Guid routeId` and `IEnumerable<TileInfo>` — no user-controlled file paths reach these constructors. `RouteImageRenderer.ExtractTileCoordinatesFromFilename` (batch 17) routes through `StorageConfig.TryExtractTileCoordinates` (a known-good parser established in batch 12 / AZ-366) and never `Path.Combine`s user input into a file path.
- `TilesZipBuilder.BuildAsync` (batch 17) reuses the file paths produced by the tile downloader — the sources are server-controlled, not client-controlled.
- The CT forwarding in batch 18 (LF-2) **improves** the security posture: `/api/satellite/tiles/latlon` clients can now cancel runaway downloads, removing a potential DoS amplifier (a slow client holding a connection while the server downloads tiles).
- AZ-353 sanitized 5xx envelope still applies to all new code paths — no batch in the window introduces a `Results.Problem(detail: ex.Message, ...)` regression.
No security finding.
## 6. Phase 5 — Performance Scan (cumulative)
- **Stitcher consolidation (b16)**: same O(N) grid-placement loop, same per-tile `Image.LoadAsync<Rgb24>` cost, same min/max scan. One additional `ToList()` on the input enumerable — bounded by region/route tile count (≤ 690 in the integration smoke). No measurable regression.
- **Decomposition (b17)**: every collaborator runs the same logic that previously lived in `RouteProcessingService`. `RouteRegionMatcher.Match` keeps the same point-by-distance ranking; `RouteCsvWriter`/`RouteSummaryWriter`/`TilesZipBuilder`/`RegionFileCleaner` all do the same I/O the orchestrator did before. Constructor injections are resolved once at app start (singletons via `RouteManagementServiceCollectionExtensions`), so the per-route overhead is unchanged.
- **IServiceProvider → IRegionService (b17 / AZ-360)**: removes a `using var scope = _serviceProvider.CreateScope()` allocation per pending route iteration. Net **positive** perf change (microscopic, but in the right direction).
- **Magic-number config (b18)**: each consuming class reads its config value once into a local field at construction time, so the per-call cost is identical to the pre-refactor literal. The two sites that previously did `0.0001 < something` per-iteration in a closure (`GoogleMapsDownloaderV2.GetTilesWithMetadataAsync`, `RouteValidator.ValidatePolygon`) now hoist the tolerance into a local — eliminates repeated property access in the hot loop.
- No new I/O introduced anywhere in the window. No new DB calls. No new HTTP calls.
No performance regression. Slight positive direction on (b17/AZ-360) and the closure hoist in (b18).
## 7. Phase 6 — Cross-Task Consistency (cumulative — main focus of K=3 review)
Style & pattern consistency across the three batches:
- **Class shape**: every new collaborator in the window is a `public class` (not `public static class`, not `sealed`, not `record`) with a single explicit constructor, a single `_logger`-style readonly field, and one or two `IOptions<T>`-bound config fields. The shape matches the precedent set by `TileCsvWriter` (cycle 1 batch 13) and `RegionFailureClassifier` (cycle 1 batch 11). Consistent.
- **DI policy split**: shared / cross-cutting utilities live in `Common` and are constructed at the call site (`new TileCsvWriter()`, `new TileGridStitcher()`). Per-component collaborators that need `IOptions<StorageConfig>` and an `ILogger<T>` are DI-registered as singletons. Batch 17 adds 5 such registrations to `RouteManagementServiceCollectionExtensions`; batch 16 deliberately did not register `TileGridStitcher` because it has no dependencies (pure utility). The split is consistent with existing Common utilities and with the precedent from cycle 1 batch 13 (TileCsvWriter, no DI registration).
- **Test convention**: every new test file uses xUnit + FluentAssertions + the `// Arrange / // Act / // Assert` comment block. No drift. The pixel-byte-equality tests in `TileGridStitcherTests` (batch 16) document **why** they need a literal inlined copy of the pre-refactor code (behavior preservation evidence) — comment-supported, not surprising.
- **Error-handling convention**: every new public method that takes a reference parameter uses `ArgumentNullException.ThrowIfNull(...)` as the first line. Domain validation throws `ArgumentException` / `ArgumentOutOfRangeException` / `InvalidOperationException` with descriptive messages. Consistent with the prior cumulative window.
- **Configuration injection convention**: batch 18 settles a single pattern — every config-aware class takes `IOptions<TConfig>` in the constructor, calls `.Value` once, stores the result in a `private readonly TConfig _config` field (or copies specific scalars into typed fields when only one or two are used). Five classes (`RegionService`, `RouteProcessingService`, `RoutePointGraphBuilder`, `RouteValidator`, `RouteImageRenderer`, `TileService`) follow this exact shape. `GoogleMapsDownloaderV2` already followed it; the new code matches.
- **No conflicting refactor styles**: no batch introduces a heavyweight pattern (factory, mediator, generic abstract base, fluent builder) that the next batch then has to inherit or work around. Each new class is a self-contained collaborator.
- **Symbol drift**: searched all new public type names across the codebase. **`TileSizePixels`** appears as a `MapConfig` property (batch 18), as a `TileEntity.TileSizePixels` column (DB-layer, unchanged for years), and as the parameter name `int tileSizePixels` in `TileGridStitcher.StitchAsync`. No conflicting definitions, no constants by the same name in two places after batch 18 — the `private const` in `RouteImageRenderer` born in batch 17 was explicitly removed by batch 18 (cumulative-review hygiene closing the loop within the same window). **`MaxPointSpacingMeters`** is now a single `ProcessingConfig` property (batch 18) — the public const that briefly lived on `RoutePointGraphBuilder` (cycle 1 batch 15) is gone. **`LatLonTolerance`** is now a single `ProcessingConfig` property (batch 18) — replacing two separate `0.0001` literals.
- **Interface drift**: `ITileService`, `IRegionService`, `IRouteService`, `IRegionRepository`, `IRouteRepository`, `ITileRepository`, `ISatelliteDownloader` — all unchanged across the window. No public API additions or removals.
- **Constructor-fan-out coordination**: batch 17 added 5 ctor parameters to `RouteProcessingService`; batch 18 added 1 more (the 6th). The DI container resolves all 6 because batch 17 also added the corresponding `services.AddSingleton` registrations and batch 18 only added a bind that was already in the container (`IOptions<ProcessingConfig>`). Smoke verifies the full chain end-to-end.
- **Dual-ctor pattern in RouteService**: batch 18 changed the production-callable ctor from 3-arg to 4-arg. The 8-arg test/decompose ctor is preserved. ASP.NET Core's default DI container will pick the 4-arg overload because `RouteValidator`, `RoutePointGraphBuilder`, `GeofenceGridCalculator`, `RouteResponseMapper` are NOT DI-registered (instantiated inline in the 4-arg `:this(...)` chain). Verified by the smoke run.
No cross-task consistency finding.
## 8. Phase 7 — Architecture Compliance
Checks applied to the cumulative window:
1. **Layer direction**: every new file lives in the layer it should:
- `TileGridStitcher` (batch 16) → `Common/Imaging/` (Layer 1 — Foundation). Adds an ImageSharp NuGet to `Common.csproj`. The Common layer's `Imports from: (none)` invariant from `module-layout.md` is preserved (NuGet packages are external dependencies, not project references). Verified — no new `<ProjectReference>` lines in `Common.csproj`.
- 7 new RouteManagement collaborators (batch 17) → `SatelliteProvider.Services.RouteManagement/` (Layer 3 — Application). Correct.
- 8 new keys in `ProcessingConfig` / `MapConfig` (batch 18) → `Common/Configs/` (Layer 1). Correct.
- 1 new `appsettings.json` block (batch 18) → `Api/` (Layer 4). Correct.
- All cross-component imports in the window go through Common Public API symbols (DTOs, configs, interfaces, utils, imaging) or DataAccess Public API symbols (entity types, repository interfaces). No internal-file imports across components.
2. **No new cyclic dependencies**: the module DAG is unchanged in this window.
- `RouteManagement → Common`, `RouteManagement → DataAccess`. ✅
- `RegionProcessing → Common`, `RegionProcessing → DataAccess`. ✅
- `TileDownloader → Common`, `TileDownloader → DataAccess`. ✅
- `Api → Common`, `Api → all three Services`. ✅
- **No sibling references between RouteManagement / RegionProcessing / TileDownloader.** Verified by reading the four `.csproj` files.
3. **Duplicate symbols across components**:
- `TileGridStitcher` — single definition in `Common.Imaging`. No duplicate.
- `TilePlacement`, `StitchResult`, `MissingTile`, `MissingTileReason` — companion records / enum in the same file. No duplicates elsewhere.
- `TileInfo`, `RouteRegionMatcher`, `RouteCsvWriter`, `RouteSummaryWriter`, `RouteImageRenderer`, `TilesZipBuilder`, `RegionFileCleaner` — all single-defined in `Services.RouteManagement`. No duplicates.
- The cross-batch hygiene observation from §7 (TileSizePixels const briefly lived in two places, then collapsed by batch 18) is exactly the kind of finding the cumulative review is designed to surface — it's already resolved within the same window, before the report writes.
4. **Cross-cutting concerns not locally re-implemented**:
- Stitching → `Common.Imaging.TileGridStitcher` (batch 16). Region service and route renderer both consume it. Cross-cutting concern correctly hoisted.
- Operational config → `Common.Configs.{ProcessingConfig, MapConfig}` (batch 18). Every consuming service reads from the same options-bound source. Cross-cutting concern correctly hoisted.
- Per-route output generation (CSV, summary, ZIP, stitched, cleanup) → kept inside `Services.RouteManagement` (batch 17). These are RouteManagement-specific concerns, not cross-cutting.
5. **Public API growth**: Common Public API gains:
- 4 new types + 1 enum under `Common.Imaging` (batch 16) — used by both Region and Route renderers.
- 8 new properties on existing config classes (batch 18) — non-breaking additions, defaulted to original literal values.
- No new interface, no breaking signature change. Backward-compatible growth.
6. **DI registration coherence**: batch 17 added 5 singleton registrations to `RouteManagementServiceCollectionExtensions`. Batch 18 added no new DI registrations because `IOptions<ProcessingConfig>` and `IOptions<MapConfig>` were already bound at startup. Smoke verifies the full chain — all 12 (`SatelliteProvider.Services.*`) resolution paths complete successfully.
No architecture finding introduced by the window.
## 9. Baseline Delta (vs. `_docs/02_document/architecture_compliance_baseline.md`)
| Bucket | Count | Notes |
|--------|-------|-------|
| Carried over | 0 | All 5 baseline findings (F1F5) were resolved by epics AZ-309 + AZ-315 prior to this run. The baseline file already reflects this. |
| Resolved | 0 | None to resolve in this window — baseline already clean. |
| Newly introduced | 0 | This window introduces no new architecture findings. |
## 10. Verdict
**Verdict: PASS** (0 findings)
| Severity | Count |
|----------|-------|
| Critical | 0 |
| High | 0 |
| Medium | 0 |
| Low | 0 |
## 11. Test posture at end of window
- **Unit tests**: 141 / 141 passing (was 112 before batch 16; net +29 across the window — TileGridStitcher×11, RouteManagement collaborators×10 net, ConfigDefaults×7, AZ-371 AC-3 CT×1).
- **Integration smoke**: 5 / 5 scenarios passing; container exits 0. Idempotency tests (AZ-362), Migration 012 tests (AZ-357), CORS / 5xx-sanitization / 501-stub / security tests, region/route processing happy paths — all green.
- **No skipped tests, no flaky tests** in this window. The three pixel-byte-equality tests in `TileGridStitcherTests` are deterministic (synthetic inputs + inlined pre-refactor code).
## 12. Action
Auto-chain to next batch per `/implement` Step 14. K=3 counter resets; next cumulative review fires after batch 21. Phase 4 continues with **AZ-370** (C17 — status / point-type enums + AC RT2 update, 3 SP, no dependencies in `todo/`).
+1 -1
View File
@@ -8,7 +8,7 @@ status: in_progress
sub_step:
phase: 4
name: execution
detail: "batch 17 (AZ-364, folds AZ-360) complete; K=3 review fires after batch 18"
detail: "K=3 review (16-18) PASS; next batch 19 candidate AZ-370"
retry_count: 0
cycle: 1
tracker: jira