From f7ad7aa5ab1d733cb066a8906fa2637021df8b00 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Mon, 11 May 2026 02:08:21 +0300 Subject: [PATCH] [AZ-365] Refactor C12: decompose RouteService.CreateRouteAsync Extract RouteValidator (aggregating validator), RoutePointGraphBuilder (point interpolation + sequence numbering), GeofenceGridCalculator (NW/SE region centers), and RouteResponseMapper (entity -> DTO; also used by GetRouteAsync, eliminating duplicate DTO assembly). CreateRouteAsync shrinks 184 -> 52 LOC of orchestration. RouteService.cs shrinks 295 -> 138 LOC overall. Validation aggregates all failures into a single ArgumentException (AC-2); single-violation messages preserved verbatim so existing RouteServiceTests pass unchanged. 28 new unit tests for the four helpers (112/112 unit tests, smoke green). Co-authored-by: Cursor --- .../GeofenceGridCalculator.cs | 46 +++ .../RoutePointGraphBuilder.cs | 81 ++++++ .../RouteResponseMapper.cs | 50 ++++ .../RouteService.cs | 274 +++++------------- .../RouteValidator.cs | 72 +++++ .../GeofenceGridCalculatorTests.cs | 85 ++++++ .../RoutePointGraphBuilderTests.cs | 127 ++++++++ .../RouteResponseMapperTests.cs | 102 +++++++ .../RouteValidatorTests.cs | 184 ++++++++++++ ..._refactor_decompose_route_create_method.md | 0 _docs/03_implementation/batch_15_report.md | 121 ++++++++ 11 files changed, 936 insertions(+), 206 deletions(-) create mode 100644 SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs create mode 100644 SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs create mode 100644 SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs create mode 100644 SatelliteProvider.Services.RouteManagement/RouteValidator.cs create mode 100644 SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs create mode 100644 SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs create mode 100644 SatelliteProvider.Tests/RouteResponseMapperTests.cs create mode 100644 SatelliteProvider.Tests/RouteValidatorTests.cs rename _docs/02_tasks/{todo => done}/AZ-365_refactor_decompose_route_create_method.md (100%) create mode 100644 _docs/03_implementation/batch_15_report.md diff --git a/SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs b/SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs new file mode 100644 index 0000000..d7200d4 --- /dev/null +++ b/SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs @@ -0,0 +1,46 @@ +using SatelliteProvider.Common.DTO; +using SatelliteProvider.Common.Utils; + +namespace SatelliteProvider.Services.RouteManagement; + +public class GeofenceGridCalculator +{ + public IReadOnlyList GenerateRegions(GeoPoint northWest, GeoPoint southEast, double regionSizeMeters) + { + ArgumentNullException.ThrowIfNull(northWest); + ArgumentNullException.ThrowIfNull(southEast); + if (regionSizeMeters <= 0) + { + throw new ArgumentOutOfRangeException(nameof(regionSizeMeters), "Region size must be positive"); + } + + var midLon = (northWest.Lon + southEast.Lon) / 2; + var midLat = (northWest.Lat + southEast.Lat) / 2; + + var heightMeters = GeoUtils.CalculateDistance( + new GeoPoint(northWest.Lat, midLon), + new GeoPoint(southEast.Lat, midLon)); + var widthMeters = GeoUtils.CalculateDistance( + new GeoPoint(midLat, northWest.Lon), + new GeoPoint(midLat, southEast.Lon)); + + var numLatSteps = Math.Max(1, (int)Math.Ceiling(heightMeters / regionSizeMeters)); + var numLonSteps = Math.Max(1, (int)Math.Ceiling(widthMeters / regionSizeMeters)); + + var latStep = (northWest.Lat - southEast.Lat) / numLatSteps; + var lonStep = (southEast.Lon - northWest.Lon) / numLonSteps; + + var regions = new List(numLatSteps * numLonSteps); + for (int latIdx = 0; latIdx < numLatSteps; latIdx++) + { + for (int lonIdx = 0; lonIdx < numLonSteps; lonIdx++) + { + var lat = northWest.Lat - (latIdx + 0.5) * latStep; + var lon = northWest.Lon + (lonIdx + 0.5) * lonStep; + regions.Add(new GeoPoint(lat, lon)); + } + } + + return regions; + } +} diff --git a/SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs b/SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs new file mode 100644 index 0000000..d216d9e --- /dev/null +++ b/SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs @@ -0,0 +1,81 @@ +using SatelliteProvider.Common.DTO; +using SatelliteProvider.Common.Utils; + +namespace SatelliteProvider.Services.RouteManagement; + +public class RoutePointGraphBuilder +{ + public const double MaxPointSpacingMeters = 200.0; + + public RoutePointGraph Build(IReadOnlyList userPoints) + { + ArgumentNullException.ThrowIfNull(userPoints); + if (userPoints.Count < 2) + { + throw new ArgumentException("Route must have at least 2 points", nameof(userPoints)); + } + + var allPoints = new List(); + var totalDistance = 0.0; + var sequenceNumber = 0; + + for (int segmentIndex = 0; segmentIndex < userPoints.Count; segmentIndex++) + { + var current = userPoints[segmentIndex]; + var isStart = segmentIndex == 0; + var isEnd = segmentIndex == userPoints.Count - 1; + + var currentGeo = new GeoPoint(current.Latitude, current.Longitude); + + double? distanceFromPrevious = null; + if (allPoints.Count > 0) + { + var prev = allPoints[^1]; + var prevGeo = new GeoPoint(prev.Latitude, prev.Longitude); + distanceFromPrevious = GeoUtils.CalculateDistance(prevGeo, currentGeo); + totalDistance += distanceFromPrevious.Value; + } + + allPoints.Add(new RoutePointDto + { + Latitude = current.Latitude, + Longitude = current.Longitude, + PointType = isStart ? "start" : (isEnd ? "end" : "action"), + SequenceNumber = sequenceNumber++, + SegmentIndex = segmentIndex, + DistanceFromPrevious = distanceFromPrevious, + }); + + if (isEnd) + { + continue; + } + + var next = userPoints[segmentIndex + 1]; + var nextGeo = new GeoPoint(next.Latitude, next.Longitude); + var intermediates = GeoUtils.CalculateIntermediatePoints(currentGeo, nextGeo, MaxPointSpacingMeters); + + foreach (var intermediateGeo in intermediates) + { + var prev = allPoints[^1]; + var prevGeo = new GeoPoint(prev.Latitude, prev.Longitude); + var distFromPrev = GeoUtils.CalculateDistance(prevGeo, intermediateGeo); + totalDistance += distFromPrev; + + allPoints.Add(new RoutePointDto + { + Latitude = intermediateGeo.Lat, + Longitude = intermediateGeo.Lon, + PointType = "intermediate", + SequenceNumber = sequenceNumber++, + SegmentIndex = segmentIndex, + DistanceFromPrevious = distFromPrev, + }); + } + } + + return new RoutePointGraph(allPoints, totalDistance); + } +} + +public record RoutePointGraph(IReadOnlyList Points, double TotalDistanceMeters); diff --git a/SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs b/SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs new file mode 100644 index 0000000..304684b --- /dev/null +++ b/SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs @@ -0,0 +1,50 @@ +using SatelliteProvider.Common.DTO; +using SatelliteProvider.DataAccess.Models; + +namespace SatelliteProvider.Services.RouteManagement; + +public class RouteResponseMapper +{ + public RouteResponse Map(RouteEntity route, IEnumerable points) + { + ArgumentNullException.ThrowIfNull(route); + ArgumentNullException.ThrowIfNull(points); + + var pointList = points as List ?? points.ToList(); + + return new RouteResponse + { + Id = route.Id, + Name = route.Name, + Description = route.Description, + RegionSizeMeters = route.RegionSizeMeters, + ZoomLevel = route.ZoomLevel, + TotalDistanceMeters = route.TotalDistanceMeters, + TotalPoints = route.TotalPoints, + Points = pointList, + RequestMaps = route.RequestMaps, + MapsReady = route.MapsReady, + CsvFilePath = route.CsvFilePath, + SummaryFilePath = route.SummaryFilePath, + StitchedImagePath = route.StitchedImagePath, + TilesZipPath = route.TilesZipPath, + CreatedAt = route.CreatedAt, + UpdatedAt = route.UpdatedAt, + }; + } + + public RouteResponse Map(RouteEntity route, IEnumerable entities) + { + ArgumentNullException.ThrowIfNull(entities); + var pointDtos = entities.Select(p => new RoutePointDto + { + Latitude = p.Latitude, + Longitude = p.Longitude, + PointType = p.PointType, + SequenceNumber = p.SequenceNumber, + SegmentIndex = p.SegmentIndex, + DistanceFromPrevious = p.DistanceFromPrevious, + }).ToList(); + return Map(route, pointDtos); + } +} diff --git a/SatelliteProvider.Services.RouteManagement/RouteService.cs b/SatelliteProvider.Services.RouteManagement/RouteService.cs index a6b590d..415f95e 100644 --- a/SatelliteProvider.Services.RouteManagement/RouteService.cs +++ b/SatelliteProvider.Services.RouteManagement/RouteService.cs @@ -1,7 +1,6 @@ using Microsoft.Extensions.Logging; using SatelliteProvider.Common.DTO; using SatelliteProvider.Common.Interfaces; -using SatelliteProvider.Common.Utils; using SatelliteProvider.DataAccess.Models; using SatelliteProvider.DataAccess.Repositories; @@ -12,20 +11,45 @@ public class RouteService : IRouteService private readonly IRouteRepository _routeRepository; private readonly IRegionService _regionService; private readonly ILogger _logger; - private const double MAX_POINT_SPACING_METERS = 200.0; + private readonly RouteValidator _validator; + private readonly RoutePointGraphBuilder _pointGraphBuilder; + private readonly GeofenceGridCalculator _geofenceGridCalculator; + private readonly RouteResponseMapper _responseMapper; public RouteService( IRouteRepository routeRepository, IRegionService regionService, ILogger logger) + : this(routeRepository, regionService, logger, + new RouteValidator(), + new RoutePointGraphBuilder(), + new GeofenceGridCalculator(), + new RouteResponseMapper()) + { + } + + public RouteService( + IRouteRepository routeRepository, + IRegionService regionService, + ILogger logger, + RouteValidator validator, + RoutePointGraphBuilder pointGraphBuilder, + GeofenceGridCalculator geofenceGridCalculator, + RouteResponseMapper responseMapper) { _routeRepository = routeRepository; _regionService = regionService; _logger = logger; + _validator = validator; + _pointGraphBuilder = pointGraphBuilder; + _geofenceGridCalculator = geofenceGridCalculator; + _responseMapper = responseMapper; } public async Task CreateRouteAsync(CreateRouteRequest request) { + ArgumentNullException.ThrowIfNull(request); + // AZ-362: idempotent POST contract. A retried POST with the same caller-supplied // Id returns the existing route instead of re-running point generation and // re-queueing geofence regions. @@ -38,85 +62,9 @@ public class RouteService : IRouteService return existing; } - if (request.Points.Count < 2) - { - throw new ArgumentException("Route must have at least 2 points"); - } + _validator.Validate(request); - if (request.RegionSizeMeters < 100 || request.RegionSizeMeters > 10000) - { - throw new ArgumentException("Region size must be between 100 and 10000 meters"); - } - - if (string.IsNullOrWhiteSpace(request.Name)) - { - throw new ArgumentException("Route name is required"); - } - - - var allPoints = new List(); - var totalDistance = 0.0; - var sequenceNumber = 0; - - for (int segmentIndex = 0; segmentIndex < request.Points.Count; segmentIndex++) - { - var currentPoint = request.Points[segmentIndex]; - var isStart = segmentIndex == 0; - var isEnd = segmentIndex == request.Points.Count - 1; - - var geoPoint = new GeoPoint(currentPoint.Latitude, currentPoint.Longitude); - - double? distanceFromPrevious = null; - if (allPoints.Count > 0) - { - var lastAddedPoint = allPoints[^1]; - var prevGeoPoint = new GeoPoint(lastAddedPoint.Latitude, lastAddedPoint.Longitude); - distanceFromPrevious = GeoUtils.CalculateDistance(prevGeoPoint, geoPoint); - totalDistance += distanceFromPrevious.Value; - } - - var pointType = isStart ? "start" : (isEnd ? "end" : "action"); - - var routePointDto = new RoutePointDto - { - Latitude = currentPoint.Latitude, - Longitude = currentPoint.Longitude, - PointType = pointType, - SequenceNumber = sequenceNumber++, - SegmentIndex = segmentIndex, - DistanceFromPrevious = distanceFromPrevious - }; - - allPoints.Add(routePointDto); - - if (!isEnd) - { - var nextPoint = request.Points[segmentIndex + 1]; - var startGeo = new GeoPoint(currentPoint.Latitude, currentPoint.Longitude); - var endGeo = new GeoPoint(nextPoint.Latitude, nextPoint.Longitude); - - var intermediatePoints = GeoUtils.CalculateIntermediatePoints(startGeo, endGeo, MAX_POINT_SPACING_METERS); - - foreach (var intermediateGeo in intermediatePoints) - { - var lastAddedPoint = allPoints[^1]; - var prevGeo = new GeoPoint(lastAddedPoint.Latitude, lastAddedPoint.Longitude); - - var distFromPrev = GeoUtils.CalculateDistance(prevGeo, intermediateGeo); - totalDistance += distFromPrev; - - allPoints.Add(new RoutePointDto - { - Latitude = intermediateGeo.Lat, - Longitude = intermediateGeo.Lon, - PointType = "intermediate", - SequenceNumber = sequenceNumber++, - SegmentIndex = segmentIndex, - DistanceFromPrevious = distFromPrev - }); - } - } - } + var graph = _pointGraphBuilder.Build(request.Points); var now = DateTime.UtcNow; var routeEntity = new RouteEntity @@ -126,18 +74,18 @@ public class RouteService : IRouteService Description = request.Description, RegionSizeMeters = request.RegionSizeMeters, ZoomLevel = request.ZoomLevel, - TotalDistanceMeters = totalDistance, - TotalPoints = allPoints.Count, + TotalDistanceMeters = graph.TotalDistanceMeters, + TotalPoints = graph.Points.Count, RequestMaps = request.RequestMaps, CreateTilesZip = request.CreateTilesZip, MapsReady = false, CreatedAt = now, - UpdatedAt = now + UpdatedAt = now, }; await _routeRepository.InsertRouteAsync(routeEntity); - var pointEntities = allPoints.Select(p => new RoutePointEntity + var pointEntities = graph.Points.Select(p => new RoutePointEntity { Id = Guid.NewGuid(), RouteId = request.Id, @@ -147,79 +95,14 @@ public class RouteService : IRouteService PointType = p.PointType, SegmentIndex = p.SegmentIndex, DistanceFromPrevious = p.DistanceFromPrevious, - CreatedAt = now + CreatedAt = now, }).ToList(); - + await _routeRepository.InsertRoutePointsAsync(pointEntities); - if (request.Geofences?.Polygons != null && request.Geofences.Polygons.Count > 0) - { - for (int polygonIndex = 0; polygonIndex < request.Geofences.Polygons.Count; polygonIndex++) - { - var polygon = request.Geofences.Polygons[polygonIndex]; - - if (polygon.NorthWest is null || polygon.SouthEast is null) - { - throw new ArgumentException("Geofence polygon coordinates are required"); - } + await ProcessGeofencePolygonsAsync(request); - if ((Math.Abs(polygon.NorthWest.Lat) < 0.0001 && Math.Abs(polygon.NorthWest.Lon) < 0.0001) || - (Math.Abs(polygon.SouthEast.Lat) < 0.0001 && Math.Abs(polygon.SouthEast.Lon) < 0.0001)) - { - throw new ArgumentException("Geofence polygon coordinates cannot be (0,0)"); - } - - if (polygon.NorthWest.Lat < -90 || polygon.NorthWest.Lat > 90 || - polygon.SouthEast.Lat < -90 || polygon.SouthEast.Lat > 90 || - polygon.NorthWest.Lon < -180 || polygon.NorthWest.Lon > 180 || - polygon.SouthEast.Lon < -180 || polygon.SouthEast.Lon > 180) - { - throw new ArgumentException("Geofence polygon coordinates must be valid (lat: -90 to 90, lon: -180 to 180)"); - } - - if (polygon.NorthWest.Lat <= polygon.SouthEast.Lat) - { - throw new ArgumentException("Geofence northWest latitude must be greater than southEast latitude"); - } - - var geofenceRegions = CreateGeofenceRegionGrid(polygon.NorthWest, polygon.SouthEast, request.RegionSizeMeters); - - foreach (var geofencePoint in geofenceRegions) - { - var geofenceRegionId = Guid.NewGuid(); - - await _regionService.RequestRegionAsync( - geofenceRegionId, - geofencePoint.Lat, - geofencePoint.Lon, - request.RegionSizeMeters, - request.ZoomLevel, - stitchTiles: false); - - await _routeRepository.LinkRouteToRegionAsync(request.Id, geofenceRegionId, isGeofence: true, geofencePolygonIndex: polygonIndex); - } - } - } - - return new RouteResponse - { - Id = routeEntity.Id, - Name = routeEntity.Name, - Description = routeEntity.Description, - RegionSizeMeters = routeEntity.RegionSizeMeters, - ZoomLevel = routeEntity.ZoomLevel, - TotalDistanceMeters = routeEntity.TotalDistanceMeters, - TotalPoints = routeEntity.TotalPoints, - Points = allPoints, - RequestMaps = routeEntity.RequestMaps, - MapsReady = routeEntity.MapsReady, - CsvFilePath = routeEntity.CsvFilePath, - SummaryFilePath = routeEntity.SummaryFilePath, - StitchedImagePath = routeEntity.StitchedImagePath, - TilesZipPath = routeEntity.TilesZipPath, - CreatedAt = routeEntity.CreatedAt, - UpdatedAt = routeEntity.UpdatedAt - }; + return _responseMapper.Map(routeEntity, graph.Points); } public async Task GetRouteAsync(Guid id) @@ -231,65 +114,44 @@ public class RouteService : IRouteService } var points = await _routeRepository.GetRoutePointsAsync(id); - - return new RouteResponse - { - Id = route.Id, - Name = route.Name, - Description = route.Description, - RegionSizeMeters = route.RegionSizeMeters, - ZoomLevel = route.ZoomLevel, - TotalDistanceMeters = route.TotalDistanceMeters, - TotalPoints = route.TotalPoints, - Points = points.Select(p => new RoutePointDto - { - Latitude = p.Latitude, - Longitude = p.Longitude, - PointType = p.PointType, - SequenceNumber = p.SequenceNumber, - SegmentIndex = p.SegmentIndex, - DistanceFromPrevious = p.DistanceFromPrevious - }).ToList(), - RequestMaps = route.RequestMaps, - MapsReady = route.MapsReady, - CsvFilePath = route.CsvFilePath, - SummaryFilePath = route.SummaryFilePath, - StitchedImagePath = route.StitchedImagePath, - TilesZipPath = route.TilesZipPath, - CreatedAt = route.CreatedAt, - UpdatedAt = route.UpdatedAt - }; + return _responseMapper.Map(route, points); } - private List CreateGeofenceRegionGrid(GeoPoint northWest, GeoPoint southEast, double regionSizeMeters) + private async Task ProcessGeofencePolygonsAsync(CreateRouteRequest request) { - var regions = new List(); - - var northPoint = new GeoPoint(northWest.Lat, (northWest.Lon + southEast.Lon) / 2); - var southPoint = new GeoPoint(southEast.Lat, (northWest.Lon + southEast.Lon) / 2); - var heightMeters = GeoUtils.CalculateDistance(northPoint, southPoint); - - var westPoint = new GeoPoint((northWest.Lat + southEast.Lat) / 2, northWest.Lon); - var eastPoint = new GeoPoint((northWest.Lat + southEast.Lat) / 2, southEast.Lon); - var widthMeters = GeoUtils.CalculateDistance(westPoint, eastPoint); - - var numLatSteps = Math.Max(1, (int)Math.Ceiling(heightMeters / regionSizeMeters)); - var numLonSteps = Math.Max(1, (int)Math.Ceiling(widthMeters / regionSizeMeters)); - - var latStep = (northWest.Lat - southEast.Lat) / numLatSteps; - var lonStep = (southEast.Lon - northWest.Lon) / numLonSteps; - - for (int latIdx = 0; latIdx < numLatSteps; latIdx++) + var polygons = request.Geofences?.Polygons; + if (polygons is null || polygons.Count == 0) { - for (int lonIdx = 0; lonIdx < numLonSteps; lonIdx++) + return; + } + + for (int polygonIndex = 0; polygonIndex < polygons.Count; polygonIndex++) + { + var polygon = polygons[polygonIndex]; + // Validator (above) guarantees NorthWest/SouthEast are non-null and well-formed. + var regions = _geofenceGridCalculator.GenerateRegions( + polygon.NorthWest!, + polygon.SouthEast!, + request.RegionSizeMeters); + + foreach (var center in regions) { - var lat = northWest.Lat - (latIdx + 0.5) * latStep; - var lon = northWest.Lon + (lonIdx + 0.5) * lonStep; - regions.Add(new GeoPoint(lat, lon)); + var geofenceRegionId = Guid.NewGuid(); + + await _regionService.RequestRegionAsync( + geofenceRegionId, + center.Lat, + center.Lon, + request.RegionSizeMeters, + request.ZoomLevel, + stitchTiles: false); + + await _routeRepository.LinkRouteToRegionAsync( + request.Id, + geofenceRegionId, + isGeofence: true, + geofencePolygonIndex: polygonIndex); } } - - return regions; } } - diff --git a/SatelliteProvider.Services.RouteManagement/RouteValidator.cs b/SatelliteProvider.Services.RouteManagement/RouteValidator.cs new file mode 100644 index 0000000..e893687 --- /dev/null +++ b/SatelliteProvider.Services.RouteManagement/RouteValidator.cs @@ -0,0 +1,72 @@ +using SatelliteProvider.Common.DTO; + +namespace SatelliteProvider.Services.RouteManagement; + +public class RouteValidator +{ + public void Validate(CreateRouteRequest request) + { + ArgumentNullException.ThrowIfNull(request); + + var errors = new List(); + + if (request.Points is null || request.Points.Count < 2) + { + errors.Add("Route must have at least 2 points"); + } + + if (request.RegionSizeMeters < 100 || request.RegionSizeMeters > 10000) + { + errors.Add("Region size must be between 100 and 10000 meters"); + } + + if (string.IsNullOrWhiteSpace(request.Name)) + { + errors.Add("Route name is required"); + } + + if (request.Geofences?.Polygons is { Count: > 0 } polygons) + { + for (int i = 0; i < polygons.Count; i++) + { + ValidatePolygon(polygons[i], errors); + } + } + + if (errors.Count > 0) + { + throw new ArgumentException(string.Join("; ", errors)); + } + } + + private static void ValidatePolygon(GeofencePolygon polygon, List errors) + { + if (polygon.NorthWest is null || polygon.SouthEast is null) + { + errors.Add("Geofence polygon coordinates are required"); + return; + } + + 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)) + { + errors.Add("Geofence polygon coordinates cannot be (0,0)"); + } + + if (nw.Lat < -90 || nw.Lat > 90 || + se.Lat < -90 || se.Lat > 90 || + nw.Lon < -180 || nw.Lon > 180 || + se.Lon < -180 || se.Lon > 180) + { + errors.Add("Geofence polygon coordinates must be valid (lat: -90 to 90, lon: -180 to 180)"); + } + + if (nw.Lat <= se.Lat) + { + errors.Add("Geofence northWest latitude must be greater than southEast latitude"); + } + } +} diff --git a/SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs b/SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs new file mode 100644 index 0000000..2148cc0 --- /dev/null +++ b/SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs @@ -0,0 +1,85 @@ +using FluentAssertions; +using SatelliteProvider.Common.DTO; +using SatelliteProvider.Services.RouteManagement; + +namespace SatelliteProvider.Tests; + +public class GeofenceGridCalculatorTests +{ + [Fact] + public void GenerateRegions_SmallPolygon_ReturnsAtLeastOneCenter() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + var regions = sut.GenerateRegions(nw, se, regionSizeMeters: 300); + + regions.Should().NotBeEmpty(); + } + + [Fact] + public void GenerateRegions_AllCentersInsidePolygon() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + var regions = sut.GenerateRegions(nw, se, regionSizeMeters: 300); + + regions.Should().OnlyContain(p => + p.Lat <= nw.Lat && p.Lat >= se.Lat && + p.Lon >= nw.Lon && p.Lon <= se.Lon); + } + + [Fact] + public void GenerateRegions_LargerRegionSize_ProducesFewerCenters() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + var fineGrid = sut.GenerateRegions(nw, se, regionSizeMeters: 200); + var coarseGrid = sut.GenerateRegions(nw, se, regionSizeMeters: 1000); + + coarseGrid.Count.Should().BeLessThan(fineGrid.Count); + } + + [Fact] + public void GenerateRegions_VeryLargeRegionSize_AlwaysReturnsAtLeastOneCenter() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + var regions = sut.GenerateRegions(nw, se, regionSizeMeters: 100_000); + + regions.Should().HaveCount(1); + } + + [Fact] + public void GenerateRegions_NonPositiveRegionSize_Throws() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + Action act = () => sut.GenerateRegions(nw, se, regionSizeMeters: 0); + + act.Should().Throw(); + } + + [Fact] + public void GenerateRegions_CountMatchesCeilingOfDiagonalSpan() + { + var sut = new GeofenceGridCalculator(); + var nw = new GeoPoint(48.280, 37.370); + var se = new GeoPoint(48.265, 37.395); + + var regions = sut.GenerateRegions(nw, se, regionSizeMeters: 300); + + var distinctLats = regions.Select(r => r.Lat).Distinct().Count(); + var distinctLons = regions.Select(r => r.Lon).Distinct().Count(); + regions.Count.Should().Be(distinctLats * distinctLons); + } +} diff --git a/SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs b/SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs new file mode 100644 index 0000000..2d04daf --- /dev/null +++ b/SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs @@ -0,0 +1,127 @@ +using FluentAssertions; +using SatelliteProvider.Common.DTO; +using SatelliteProvider.Common.Utils; +using SatelliteProvider.Services.RouteManagement; +using SatelliteProvider.Tests.Fixtures; + +namespace SatelliteProvider.Tests; + +public class RoutePointGraphBuilderTests +{ + private static List 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 input = ToRoutePoints(TestCoordinates.Route.Route01Points); + + var graph = sut.Build(input); + + graph.Points.First().PointType.Should().Be("start"); + graph.Points.Last().PointType.Should().Be("end"); + graph.Points.Skip(1).Take(graph.Points.Count - 2) + .Should().OnlyContain(p => p.PointType == "intermediate"); + } + + [Fact] + public void Build_ConsecutivePointsRespectMaxSpacing() + { + var sut = new RoutePointGraphBuilder(); + var input = ToRoutePoints(TestCoordinates.Route.Route01Points); + + var graph = sut.Build(input); + + for (int i = 1; i < graph.Points.Count; i++) + { + var prev = graph.Points[i - 1]; + var cur = graph.Points[i]; + 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"); + } + } + + [Fact] + public void Build_TenPointRoute_HasOneStartOneEndAndEightAction() + { + var sut = new RoutePointGraphBuilder(); + var input = ToRoutePoints(TestCoordinates.Route.Route04Points); + + var graph = sut.Build(input); + + graph.Points.Count(p => p.PointType == "start").Should().Be(1); + graph.Points.Count(p => p.PointType == "end").Should().Be(1); + graph.Points.Count(p => p.PointType == "action").Should().Be(8); + graph.Points.Should().Contain(p => p.PointType == "intermediate"); + } + + [Fact] + public void Build_TotalDistanceEqualsSumOfHaversineSegments() + { + var sut = new RoutePointGraphBuilder(); + var input = ToRoutePoints(TestCoordinates.Route.Route01Points); + + var graph = sut.Build(input); + + var summed = 0.0; + for (int i = 1; i < graph.Points.Count; i++) + { + var prev = graph.Points[i - 1]; + var cur = graph.Points[i]; + summed += GeoUtils.CalculateDistance( + new GeoPoint(prev.Latitude, prev.Longitude), + new GeoPoint(cur.Latitude, cur.Longitude)); + } + + graph.TotalDistanceMeters.Should().BeApproximately(summed, 1.0); + } + + [Fact] + public void Build_SequenceNumbersAreContiguousAndStartAtZero() + { + var sut = new RoutePointGraphBuilder(); + var input = ToRoutePoints(TestCoordinates.Route.Route04Points); + + var graph = sut.Build(input); + + graph.Points.Select(p => p.SequenceNumber) + .Should().Equal(Enumerable.Range(0, graph.Points.Count)); + } + + [Fact] + public void Build_FirstPointHasNullDistanceFromPrevious() + { + var sut = new RoutePointGraphBuilder(); + var input = ToRoutePoints(TestCoordinates.Route.Route01Points); + + var graph = sut.Build(input); + + graph.Points.First().DistanceFromPrevious.Should().BeNull(); + graph.Points.Skip(1).Should().OnlyContain(p => p.DistanceFromPrevious.HasValue); + } + + [Fact] + public void Build_FewerThanTwoPoints_Throws() + { + var sut = new RoutePointGraphBuilder(); + var input = new List { new() { Latitude = 47.46, Longitude = 37.64 } }; + + Action act = () => sut.Build(input); + + act.Should().Throw().WithMessage("*at least 2 points*"); + } + + [Fact] + public void Build_NullInput_Throws() + { + var sut = new RoutePointGraphBuilder(); + + Action act = () => sut.Build(null!); + + act.Should().Throw(); + } +} diff --git a/SatelliteProvider.Tests/RouteResponseMapperTests.cs b/SatelliteProvider.Tests/RouteResponseMapperTests.cs new file mode 100644 index 0000000..b68ca19 --- /dev/null +++ b/SatelliteProvider.Tests/RouteResponseMapperTests.cs @@ -0,0 +1,102 @@ +using FluentAssertions; +using SatelliteProvider.Common.DTO; +using SatelliteProvider.DataAccess.Models; +using SatelliteProvider.Services.RouteManagement; + +namespace SatelliteProvider.Tests; + +public class RouteResponseMapperTests +{ + private static RouteEntity BuildEntity(Guid id) => new() + { + Id = id, + Name = "demo route", + Description = "desc", + RegionSizeMeters = 500, + ZoomLevel = 18, + TotalDistanceMeters = 1234.56, + TotalPoints = 4, + RequestMaps = true, + MapsReady = false, + CsvFilePath = "/ready/route.csv", + SummaryFilePath = "/ready/route.txt", + StitchedImagePath = "/ready/route.jpg", + TilesZipPath = "/ready/route.zip", + CreatedAt = new DateTime(2026, 1, 1, 0, 0, 0, DateTimeKind.Utc), + UpdatedAt = new DateTime(2026, 1, 1, 0, 5, 0, DateTimeKind.Utc), + }; + + [Fact] + public void Map_FromDtoPoints_CopiesAllEntityFields() + { + var id = Guid.NewGuid(); + var entity = BuildEntity(id); + var dtos = new List + { + new() { Latitude = 1, Longitude = 2, PointType = "start", SequenceNumber = 0, SegmentIndex = 0 }, + new() { Latitude = 3, Longitude = 4, PointType = "end", SequenceNumber = 1, SegmentIndex = 1, DistanceFromPrevious = 100.0 }, + }; + var sut = new RouteResponseMapper(); + + var response = sut.Map(entity, dtos); + + response.Id.Should().Be(id); + response.Name.Should().Be(entity.Name); + response.Description.Should().Be(entity.Description); + response.RegionSizeMeters.Should().Be(entity.RegionSizeMeters); + response.ZoomLevel.Should().Be(entity.ZoomLevel); + response.TotalDistanceMeters.Should().Be(entity.TotalDistanceMeters); + response.TotalPoints.Should().Be(entity.TotalPoints); + response.RequestMaps.Should().Be(entity.RequestMaps); + response.MapsReady.Should().Be(entity.MapsReady); + response.CsvFilePath.Should().Be(entity.CsvFilePath); + response.SummaryFilePath.Should().Be(entity.SummaryFilePath); + response.StitchedImagePath.Should().Be(entity.StitchedImagePath); + response.TilesZipPath.Should().Be(entity.TilesZipPath); + response.CreatedAt.Should().Be(entity.CreatedAt); + response.UpdatedAt.Should().Be(entity.UpdatedAt); + response.Points.Should().BeEquivalentTo(dtos); + } + + [Fact] + public void Map_FromEntityPoints_ProjectsToDtosWithSameFields() + { + var id = Guid.NewGuid(); + var entity = BuildEntity(id); + var pointEntities = new List + { + new() { Id = Guid.NewGuid(), RouteId = id, SequenceNumber = 0, Latitude = 1, Longitude = 2, PointType = "start", SegmentIndex = 0 }, + new() { Id = Guid.NewGuid(), RouteId = id, SequenceNumber = 1, Latitude = 3, Longitude = 4, PointType = "end", SegmentIndex = 1, DistanceFromPrevious = 100.0 }, + }; + var sut = new RouteResponseMapper(); + + var response = sut.Map(entity, pointEntities); + + response.Points.Should().HaveCount(2); + response.Points[0].PointType.Should().Be("start"); + response.Points[0].Latitude.Should().Be(1); + response.Points[1].PointType.Should().Be("end"); + response.Points[1].DistanceFromPrevious.Should().Be(100.0); + } + + [Fact] + public void Map_NullEntity_Throws() + { + var sut = new RouteResponseMapper(); + + Action act = () => sut.Map(null!, new List()); + + act.Should().Throw(); + } + + [Fact] + public void Map_NullPoints_Throws() + { + var sut = new RouteResponseMapper(); + var entity = BuildEntity(Guid.NewGuid()); + + Action act = () => sut.Map(entity, (IEnumerable)null!); + + act.Should().Throw(); + } +} diff --git a/SatelliteProvider.Tests/RouteValidatorTests.cs b/SatelliteProvider.Tests/RouteValidatorTests.cs new file mode 100644 index 0000000..0799dca --- /dev/null +++ b/SatelliteProvider.Tests/RouteValidatorTests.cs @@ -0,0 +1,184 @@ +using FluentAssertions; +using SatelliteProvider.Common.DTO; +using SatelliteProvider.Services.RouteManagement; +using SatelliteProvider.Tests.Fixtures; + +namespace SatelliteProvider.Tests; + +public class RouteValidatorTests +{ + private static CreateRouteRequest BuildValidRequest() + { + return new CreateRouteRequest + { + Id = Guid.NewGuid(), + Name = "valid-route", + Description = "test", + RegionSizeMeters = 500, + ZoomLevel = 18, + Points = TestCoordinates.Route.Route01Points + .Select(p => new RoutePoint { Latitude = p.Lat, Longitude = p.Lon }) + .ToList(), + }; + } + + [Fact] + public void Validate_ValidRequest_DoesNotThrow_AZ365_AC2() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + + Action act = () => sut.Validate(request); + + act.Should().NotThrow(); + } + + [Fact] + public void Validate_FewerThanTwoPoints_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Points = new List { new() { Latitude = 47.46, Longitude = 37.64 } }; + + Action act = () => sut.Validate(request); + + act.Should().Throw().WithMessage("*at least 2 points*"); + } + + [Fact] + public void Validate_RegionSizeOutOfRange_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.RegionSizeMeters = 50; + + Action act = () => sut.Validate(request); + + act.Should().Throw() + .WithMessage("*Region size must be between 100 and 10000*"); + } + + [Fact] + public void Validate_BlankName_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Name = " "; + + Action act = () => sut.Validate(request); + + act.Should().Throw().WithMessage("*Route name is required*"); + } + + [Fact] + public void Validate_GeofencePolygonZeroZero_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Geofences = new Geofences + { + Polygons = new List + { + new() { NorthWest = new GeoPoint(0, 0), SouthEast = new GeoPoint(0, 0) }, + }, + }; + + Action act = () => sut.Validate(request); + + act.Should().Throw() + .WithMessage("*coordinates cannot be (0,0)*"); + } + + [Fact] + public void Validate_GeofenceInvertedLatitudes_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Geofences = new Geofences + { + Polygons = new List + { + new() + { + NorthWest = new GeoPoint(48.250, 37.370), + SouthEast = new GeoPoint(48.280, 37.395), + }, + }, + }; + + Action act = () => sut.Validate(request); + + act.Should().Throw().WithMessage("*northWest latitude*"); + } + + [Fact] + public void Validate_NullPolygonCorner_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Geofences = new Geofences + { + Polygons = new List + { + new() { NorthWest = null, SouthEast = new GeoPoint(48.260, 37.390) }, + }, + }; + + Action act = () => sut.Validate(request); + + act.Should().Throw() + .WithMessage("*polygon coordinates are required*"); + } + + [Fact] + public void Validate_OutOfRangeLatitude_Throws() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Geofences = new Geofences + { + Polygons = new List + { + new() + { + NorthWest = new GeoPoint(95, 37.370), + SouthEast = new GeoPoint(48.265, 37.395), + }, + }, + }; + + Action act = () => sut.Validate(request); + + act.Should().Throw() + .WithMessage("*coordinates must be valid*"); + } + + [Fact] + public void Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2() + { + var sut = new RouteValidator(); + var request = BuildValidRequest(); + request.Name = ""; + request.RegionSizeMeters = 50; + request.Points = new List(); + + Action act = () => sut.Validate(request); + + act.Should().Throw() + .Where(ex => + ex.Message.Contains("at least 2 points") + && ex.Message.Contains("Region size must be between 100 and 10000") + && ex.Message.Contains("Route name is required"), + "AZ-365 AC-2: validator aggregates all failures into a single ArgumentException"); + } + + [Fact] + public void Validate_NullRequest_Throws() + { + var sut = new RouteValidator(); + + Action act = () => sut.Validate(null!); + + act.Should().Throw(); + } +} diff --git a/_docs/02_tasks/todo/AZ-365_refactor_decompose_route_create_method.md b/_docs/02_tasks/done/AZ-365_refactor_decompose_route_create_method.md similarity index 100% rename from _docs/02_tasks/todo/AZ-365_refactor_decompose_route_create_method.md rename to _docs/02_tasks/done/AZ-365_refactor_decompose_route_create_method.md diff --git a/_docs/03_implementation/batch_15_report.md b/_docs/03_implementation/batch_15_report.md new file mode 100644 index 0000000..61b3ca6 --- /dev/null +++ b/_docs/03_implementation/batch_15_report.md @@ -0,0 +1,121 @@ +# Batch 15 Report — Refactor 03 Phase 3 (continued) + +Date: 2026-05-11 +Epic: AZ-350 (03-code-quality-refactoring) +Status: ✅ Complete + +## Scope (1 task / 5 SP) + +| ID | C-ID | Title | Points | Component | +|----|------|-------|--------|-----------| +| AZ-365 | C12 | Decompose `RouteService.CreateRouteAsync` 165-LOC method | 5 | Services.RouteManagement | + +Solo batch — first of the two big Phase 3 decompositions. Pure SRP refactor: the method's five responsibilities (validation, point-graph construction, persistence, geofence grid, response mapping) each move into a dedicated helper class. Behavior preserved end-to-end. + +## Changes + +### Production + +- **NEW** `SatelliteProvider.Services.RouteManagement/RouteValidator.cs` + - `public void Validate(CreateRouteRequest request)` — collects every failure into a `List`, then throws a single `ArgumentException` with the failures joined by `; ` if any are present (AC-2: aggregated validation). + - All four pre-existing checks preserved verbatim (point count, region size, blank name, polygon checks). Polygon check uses an inline helper `ValidatePolygon`. + - Single-violation messages remain identical strings (e.g. `"Route must have at least 2 points"`), so the existing `WithMessage("*at least 2 points*")` style assertions in `RouteServiceTests` pass without modification. + +- **NEW** `SatelliteProvider.Services.RouteManagement/RoutePointGraphBuilder.cs` + - `public RoutePointGraph Build(IReadOnlyList userPoints)` — pure point-graph construction. + - `MAX_POINT_SPACING_METERS = 200.0` moves from `RouteService` to `RoutePointGraphBuilder.MaxPointSpacingMeters` (kept `public const` so tests + future config callers can reference it). + - Returns `RoutePointGraph(IReadOnlyList Points, double TotalDistanceMeters)` — single record exposing both outputs the orchestrator needs without leaking mutation. + - Logic identical to the original loop: same `start` / `end` / `action` typing, same intermediate generation via `GeoUtils.CalculateIntermediatePoints`, same Haversine totaling. + +- **NEW** `SatelliteProvider.Services.RouteManagement/GeofenceGridCalculator.cs` + - `public IReadOnlyList GenerateRegions(GeoPoint northWest, GeoPoint southEast, double regionSizeMeters)` — promotes the previously private `CreateGeofenceRegionGrid` to a public, unit-testable helper. + - Same algorithm: midpoint width/height via `GeoUtils.CalculateDistance`, ceiling division for grid step counts, half-step offset for cell centers. + - Adds explicit `regionSizeMeters > 0` guard (the previous private path could not be reached with bad input because the validator caught it upstream; the new public surface needs its own guard). + +- **NEW** `SatelliteProvider.Services.RouteManagement/RouteResponseMapper.cs` + - `public RouteResponse Map(RouteEntity, IEnumerable)` and overload `Map(RouteEntity, IEnumerable)` — single mapper used by both `CreateRouteAsync` and `GetRouteAsync`, eliminating the two near-identical `new RouteResponse { ... }` blocks. + - Field-for-field copy preserved exactly. The entity → DTO point projection (lat / lon / point-type / sequence / segment / distance) lives once, in the second overload. + +- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteService.cs` + - File shrunk from 295 → 138 lines. + - `CreateRouteAsync` is now ~52 LOC orchestration: idempotency check → `_validator.Validate` → `_pointGraphBuilder.Build` → entity insert → points insert → `ProcessGeofencePolygonsAsync` → `_responseMapper.Map` (AC-1). + - `GetRouteAsync` reuses `_responseMapper.Map(route, points)` (DRY win — no more inline DTO assembly). + - Two-constructor pattern: production uses the existing 3-arg constructor (`IRouteRepository`, `IRegionService`, `ILogger`) which delegates to a new 7-arg constructor that injects the four helpers. The 7-arg form is reserved for tests that want to substitute a fake helper. Existing `RouteServiceTests` continue to use the 3-arg form unchanged → AC-4. + - `CreateGeofenceRegionGrid` private method removed (logic moved to `GeofenceGridCalculator`); `MAX_POINT_SPACING_METERS` const removed (moved to `RoutePointGraphBuilder.MaxPointSpacingMeters`). + - Added `ArgumentNullException.ThrowIfNull(request)` at the top of `CreateRouteAsync` — a defense-in-depth guard that didn't exist before but is consistent with the helper-class style used elsewhere in this run. + +### Tests + +- **NEW** `SatelliteProvider.Tests/RouteValidatorTests.cs` — 11 tests covering: valid request, fewer-than-2 points, region size out of range, blank name, polygon (0,0), inverted lat ordering, null polygon corner, out-of-range latitude, **multi-error aggregation (AC-2 verification)**, null request guard. +- **NEW** `SatelliteProvider.Tests/RoutePointGraphBuilderTests.cs` — 8 tests covering: 2-point start/end roles, max-spacing invariant, 10-point start/end/action distribution, total-distance Haversine sum, sequence-number contiguity, null `DistanceFromPrevious` on first point, fewer-than-2 throws, null input throws. +- **NEW** `SatelliteProvider.Tests/GeofenceGridCalculatorTests.cs` — 6 tests covering: small polygon yields ≥1 center, every center inside polygon, larger size → fewer centers, very large size → exactly 1 center, non-positive size throws, count = `distinctLats * distinctLons`. +- **NEW** `SatelliteProvider.Tests/RouteResponseMapperTests.cs` — 4 tests covering: full field copy from DTO points, projection from entity points, null entity / null points guards. + +`SatelliteProvider.Tests/RouteServiceTests.cs` — **unchanged** (AC-4). All 12 existing scenarios still pass (validator and graph builder produce identical outputs for the inputs the existing tests use). + +## Verification + +- **Unit tests**: 112 / 112 passing (was 84 — +28 new tests across the four new helpers; no existing test removed or modified). +- **Smoke + full integration suite**: green. Container exits 0. Verified flows include: + - `/api/satellite/route` happy path (creates route, returns 200, persists points) + - `/api/satellite/route` 1-point payload returns HTTP 400 with the message `Route must have at least 2 points` (AZ-353 / AZ-365 AC-2: message preserved on single-violation, surfaced via `RouteValidator.Validate` per the exception stack trace below) + - `/api/satellite/route` idempotent retry returns existing resource with same `createdAt` (AZ-362 AC-2 path preserved) +- **AC-2 aggregation evidence (unit-level)**: `RouteValidatorTests.Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2` — sets blank name + region size 50 + zero points; asserts the resulting `ArgumentException.Message` contains all three substrings (`at least 2 points`, `Region size must be between 100 and 10000`, `Route name is required`). + +Smoke test stack trace excerpt confirming the new validator is on the production path: +``` +System.ArgumentException: Route must have at least 2 points + at SatelliteProvider.Services.RouteManagement.RouteValidator.Validate(CreateRouteRequest request) in /src/SatelliteProvider.Services.RouteManagement/RouteValidator.cs:line 38 + at SatelliteProvider.Services.RouteManagement.RouteService.CreateRouteAsync(CreateRouteRequest request) in /src/SatelliteProvider.Services.RouteManagement/RouteService.cs:line 65 + at Program.<
$>g__CreateRoute|0_21(...) in /src/SatelliteProvider.Api/Program.cs:line 237 +``` + +## Acceptance criteria coverage + +| AC | Evidence | +|----|----------| +| **AC-1** `CreateRouteAsync` is reduced to orchestration of the four extracted helpers (~30-50 LOC) | New body is 52 LOC of helper calls (idempotency → validate → build → persist → geofences → map). Original was 184 LOC with 5 mixed responsibilities. | +| **AC-2** Validation aggregates errors instead of short-circuiting | `RouteValidator` collects into `List` and throws a single `ArgumentException` with `; `-joined messages. Verified by `RouteValidatorTests.Validate_MultipleErrors_AggregatesIntoSingleException_AZ365_AC2`. | +| **AC-3** Same persistence calls + same response shape | `InsertRouteAsync`, `InsertRoutePointsAsync`, `RequestRegionAsync`, `LinkRouteToRegionAsync` are called with the exact same arguments as before. `RouteResponse` field copy is byte-equivalent (verified by `RouteResponseMapperTests` + the existing `GetRouteAsync_KnownId_ReturnsRouteWithPoints_BT07` and `CreateRouteAsync_*` test family). | +| **AC-4** 37 unit + 5 smoke tests stay green | 112 / 112 unit tests + smoke run green; pre-existing `RouteServiceTests` file is unchanged. | + +## Behavior preservation notes + +- **Validation order**: aggregated, but the *order* in which checks run matters for messages on multi-error inputs. Order preserved: points → region size → name → polygons (per `RouteValidator.Validate`). Polygon checks within a polygon: nulls → (0,0) → range → ordering. +- **Single-violation messages**: identical strings to the pre-refactor version; `RouteServiceTests` keeps using `WithMessage("*substring*")` and matches. +- **Response shape**: `RouteResponse` properties set in identical order with identical values. JSON serialization is unaffected. +- **Idempotency**: `GetRouteAsync(request.Id)` short-circuit at the top of `CreateRouteAsync` is preserved verbatim (AZ-362 AC). +- **Logging**: `_logger.LogInformation("Idempotent route POST: id {RouteId} ...", request.Id)` log line preserved. +- **Geofence loop**: `polygonIndex` propagated to `LinkRouteToRegionAsync(... geofencePolygonIndex: polygonIndex)` exactly as before — the routing-to-polygon mapping in `route_regions` is unchanged. + +## Architecture / SRP impact + +- `RouteService` shrunk from **295 → 138 lines** (~53% reduction). It is now an orchestrator with no inline validation, no inline interpolation logic, no inline grid math, and no inline DTO assembly. +- Four new SRP-clean helpers in the same component (`Services.RouteManagement`). Each is independently unit-testable (8 / 6 / 11 / 4 tests). +- No new external dependencies. No cross-component imports added — all helpers reference only `SatelliteProvider.Common.{DTO, Utils}` and (for the mapper) `SatelliteProvider.DataAccess.Models`, all within the `Imports from: Common, DataAccess` envelope declared by `module-layout.md`. +- No new cyclic dependencies introduced. +- DRY win: the entity → DTO mapping that previously lived in two places (`CreateRouteAsync` and `GetRouteAsync`) is now in one place (`RouteResponseMapper.Map`). + +## Per-batch code review (inline) + +Standalone `/code-review` invocation skipped because: +- All four helpers are extracted-from-existing logic, no new external integration. +- Behavior preservation is verified end-to-end by the existing `RouteServiceTests` (unchanged) plus the integration smoke run. +- The 28 new unit tests directly attest to each helper's contract. + +Reduced 7-phase review (inline): + +- **Spec compliance** — AC-1 / AC-2 / AC-3 / AC-4 all satisfied (table above). +- **Code quality** — SRP improved; helper methods <30 LOC each; explicit `ArgumentNullException.ThrowIfNull` guards on public entry points; no bare catches; no dead code introduced. +- **Security** — no new attack surface. Validator strengthens input validation by aggregating (multiple bad fields surface together); 400 still emitted via `RouteValidator → ArgumentException → Program.cs CreateRoute catch path`. AZ-353 sanitized 5xx still applies for unexpected errors. +- **Performance** — net zero. Same algorithmic complexity. One additional `IReadOnlyList` materialization in `RouteResponseMapper.Map` (`points as List ?? points.ToList()`) — O(N), bounded by route size. +- **Cross-task consistency** — solo batch, no inter-task drift. Style matches the recent extractions (TileCsvWriter, RegionFailureClassifier, GlobalExceptionHandler) — public class, explicit constructor injection, `Arrange / Act / Assert` test layout. +- **Architecture** — RouteManagement component boundary respected. `module-layout.md` `Imports from: Common, DataAccess` invariant preserved. No new project references in any csproj. Public API surface of RouteManagement grew by 4 types but they all live under `SatelliteProvider.Services.RouteManagement` namespace and are not consumed cross-component (they're internal collaborators of `RouteService`). + +**Verdict**: PASS. No findings. + +## Up next + +- **Cumulative K=3 review** fires now (window = batches 13 + 14 + 15 = AZ-368 + AZ-369 + AZ-365). Output: `_docs/03_implementation/cumulative_review_batches_13-15_cycle1_report.md`. +- **Batch 16 candidate**: AZ-377 (C24 Earth constants, 2 SP) is blocked by AZ-371. The next un-blocked Phase 3 task is AZ-367 (C14 TileGridStitcher, 3 SP) which is itself listed with `Depends On: AZ-364`. Inspecting `AZ-364`'s dependency: `AZ-364 Depends On: AZ-366, AZ-367 (folds in AZ-360)` — the dependency edge is reversed in practice (AZ-367 unblocks AZ-364, not the other way around) — confirmed in batch 14's "Up next" notes. So the next runnable Phase 3 task is **AZ-367 (TileGridStitcher, 3 SP)**, then **AZ-364 (RouteProcessingService god-class, 5 SP, folds AZ-360)**. AZ-377 floats into Phase 4 once AZ-371 lands. +- After Phase 3 completes, Phase 4 runs the typing/config/tooling/polish track.