mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 06:51:13 +00:00
[AZ-369] Refactor C16: move inline DTOs out of Program.cs
Move 5 cross-component DTOs (GetSatelliteTilesResponse, SatelliteTile, SaveResult, DownloadTileResponse, RequestRegionRequest) to SatelliteProvider.Common/DTO/. Keep UploadImageRequest in the API project under SatelliteProvider.Api.DTOs (IFormFile depends on Microsoft.AspNetCore.Http; pulling it into Common would force an ASP.NET framework reference into the foundation layer and break the module-layout "Common: Imports from: (none)" invariant). Move ParameterDescriptionFilter to SatelliteProvider.Api.Swagger. Program.cs shrinks from 366 to 257 lines and now contains only endpoint wiring (AC-1). JSON wire shape and Swagger schema names are preserved (AC-2). 84 unit + full integration suite green (AC-3). Co-authored-by: Cursor <cursoragent@cursor.com>
This commit is contained in:
@@ -0,0 +1,30 @@
|
||||
using System.ComponentModel.DataAnnotations;
|
||||
|
||||
namespace SatelliteProvider.Api.DTOs;
|
||||
|
||||
public record UploadImageRequest
|
||||
{
|
||||
[Required]
|
||||
public DateTime Timestamp { get; set; }
|
||||
|
||||
[Required]
|
||||
public IFormFile? Image { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Lat { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Lon { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Height { get; set; }
|
||||
|
||||
[Required]
|
||||
public double FocalLength { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SensorWidth { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SensorHeight { get; set; }
|
||||
}
|
||||
@@ -1,8 +1,9 @@
|
||||
using System.ComponentModel.DataAnnotations;
|
||||
using Microsoft.AspNetCore.Mvc;
|
||||
using Microsoft.OpenApi.Models;
|
||||
using Swashbuckle.AspNetCore.SwaggerGen;
|
||||
using SatelliteProvider.Api;
|
||||
using SatelliteProvider.Api.DTOs;
|
||||
using SatelliteProvider.Api.Swagger;
|
||||
using SatelliteProvider.DataAccess;
|
||||
using SatelliteProvider.DataAccess.Repositories;
|
||||
using SatelliteProvider.Common.Configs;
|
||||
@@ -254,113 +255,3 @@ async Task<IResult> GetRoute(Guid id, IRouteService routeService)
|
||||
|
||||
return Results.Ok(route);
|
||||
}
|
||||
|
||||
public record GetSatelliteTilesResponse
|
||||
{
|
||||
public List<SatelliteTile> Tiles { get; set; } = new();
|
||||
}
|
||||
|
||||
public record SatelliteTile
|
||||
{
|
||||
public string TileId { get; set; } = string.Empty;
|
||||
public byte[] ImageData { get; set; } = Array.Empty<byte>();
|
||||
public double Lat { get; set; }
|
||||
public double Lon { get; set; }
|
||||
public int ZoomLevel { get; set; }
|
||||
}
|
||||
|
||||
public record UploadImageRequest
|
||||
{
|
||||
[Required]
|
||||
public DateTime Timestamp { get; set; }
|
||||
|
||||
[Required]
|
||||
public IFormFile? Image { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Lat { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Lon { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Height { get; set; }
|
||||
|
||||
[Required]
|
||||
public double FocalLength { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SensorWidth { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SensorHeight { get; set; }
|
||||
}
|
||||
|
||||
public record SaveResult
|
||||
{
|
||||
public bool Success { get; set; }
|
||||
public string? Exception { get; set; }
|
||||
}
|
||||
|
||||
public record DownloadTileResponse
|
||||
{
|
||||
public Guid Id { get; set; }
|
||||
public int ZoomLevel { get; set; }
|
||||
public double Latitude { get; set; }
|
||||
public double Longitude { get; set; }
|
||||
public double TileSizeMeters { get; set; }
|
||||
public int TileSizePixels { get; set; }
|
||||
public string ImageType { get; set; } = string.Empty;
|
||||
public string? MapsVersion { get; set; }
|
||||
public int? Version { get; set; }
|
||||
public string FilePath { get; set; } = string.Empty;
|
||||
public DateTime CreatedAt { get; set; }
|
||||
public DateTime UpdatedAt { get; set; }
|
||||
}
|
||||
|
||||
public record RequestRegionRequest
|
||||
{
|
||||
[Required]
|
||||
public Guid Id { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Latitude { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Longitude { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SizeMeters { get; set; }
|
||||
|
||||
[Required]
|
||||
public int ZoomLevel { get; set; } = 18;
|
||||
|
||||
public bool StitchTiles { get; set; } = false;
|
||||
}
|
||||
|
||||
public class ParameterDescriptionFilter : IOperationFilter
|
||||
{
|
||||
public void Apply(OpenApiOperation operation, OperationFilterContext context)
|
||||
{
|
||||
if (operation.Parameters == null) return;
|
||||
|
||||
var parameterDescriptions = new Dictionary<string, string>
|
||||
{
|
||||
["lat"] = "Latitude coordinate where image was captured",
|
||||
["lon"] = "Longitude coordinate where image was captured",
|
||||
["mgrs"] = "MGRS coordinate string",
|
||||
["squareSideMeters"] = "Square side size in meters",
|
||||
["Latitude"] = "Latitude coordinate of the tile center",
|
||||
["Longitude"] = "Longitude coordinate of the tile center",
|
||||
["ZoomLevel"] = "Zoom level for the tile (higher values = more detail)"
|
||||
};
|
||||
|
||||
foreach (var parameter in operation.Parameters)
|
||||
{
|
||||
if (parameterDescriptions.TryGetValue(parameter.Name, out var description))
|
||||
{
|
||||
parameter.Description = description;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,31 @@
|
||||
using Microsoft.OpenApi.Models;
|
||||
using Swashbuckle.AspNetCore.SwaggerGen;
|
||||
|
||||
namespace SatelliteProvider.Api.Swagger;
|
||||
|
||||
public class ParameterDescriptionFilter : IOperationFilter
|
||||
{
|
||||
public void Apply(OpenApiOperation operation, OperationFilterContext context)
|
||||
{
|
||||
if (operation.Parameters == null) return;
|
||||
|
||||
var parameterDescriptions = new Dictionary<string, string>
|
||||
{
|
||||
["lat"] = "Latitude coordinate where image was captured",
|
||||
["lon"] = "Longitude coordinate where image was captured",
|
||||
["mgrs"] = "MGRS coordinate string",
|
||||
["squareSideMeters"] = "Square side size in meters",
|
||||
["Latitude"] = "Latitude coordinate of the tile center",
|
||||
["Longitude"] = "Longitude coordinate of the tile center",
|
||||
["ZoomLevel"] = "Zoom level for the tile (higher values = more detail)"
|
||||
};
|
||||
|
||||
foreach (var parameter in operation.Parameters)
|
||||
{
|
||||
if (parameterDescriptions.TryGetValue(parameter.Name, out var description))
|
||||
{
|
||||
parameter.Description = description;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,17 @@
|
||||
namespace SatelliteProvider.Common.DTO;
|
||||
|
||||
public record DownloadTileResponse
|
||||
{
|
||||
public Guid Id { get; set; }
|
||||
public int ZoomLevel { get; set; }
|
||||
public double Latitude { get; set; }
|
||||
public double Longitude { get; set; }
|
||||
public double TileSizeMeters { get; set; }
|
||||
public int TileSizePixels { get; set; }
|
||||
public string ImageType { get; set; } = string.Empty;
|
||||
public string? MapsVersion { get; set; }
|
||||
public int? Version { get; set; }
|
||||
public string FilePath { get; set; } = string.Empty;
|
||||
public DateTime CreatedAt { get; set; }
|
||||
public DateTime UpdatedAt { get; set; }
|
||||
}
|
||||
@@ -0,0 +1,6 @@
|
||||
namespace SatelliteProvider.Common.DTO;
|
||||
|
||||
public record GetSatelliteTilesResponse
|
||||
{
|
||||
public List<SatelliteTile> Tiles { get; set; } = new();
|
||||
}
|
||||
@@ -0,0 +1,23 @@
|
||||
using System.ComponentModel.DataAnnotations;
|
||||
|
||||
namespace SatelliteProvider.Common.DTO;
|
||||
|
||||
public record RequestRegionRequest
|
||||
{
|
||||
[Required]
|
||||
public Guid Id { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Latitude { get; set; }
|
||||
|
||||
[Required]
|
||||
public double Longitude { get; set; }
|
||||
|
||||
[Required]
|
||||
public double SizeMeters { get; set; }
|
||||
|
||||
[Required]
|
||||
public int ZoomLevel { get; set; } = 18;
|
||||
|
||||
public bool StitchTiles { get; set; } = false;
|
||||
}
|
||||
@@ -0,0 +1,10 @@
|
||||
namespace SatelliteProvider.Common.DTO;
|
||||
|
||||
public record SatelliteTile
|
||||
{
|
||||
public string TileId { get; set; } = string.Empty;
|
||||
public byte[] ImageData { get; set; } = Array.Empty<byte>();
|
||||
public double Lat { get; set; }
|
||||
public double Lon { get; set; }
|
||||
public int ZoomLevel { get; set; }
|
||||
}
|
||||
@@ -0,0 +1,7 @@
|
||||
namespace SatelliteProvider.Common.DTO;
|
||||
|
||||
public record SaveResult
|
||||
{
|
||||
public bool Success { get; set; }
|
||||
public string? Exception { get; set; }
|
||||
}
|
||||
@@ -43,7 +43,7 @@ Roadmap: `_docs/04_refactoring/03-code-quality-refactoring/analysis/refactoring_
|
||||
| AZ-377 | C24 | Consolidate Earth constants + 111000 | 3 | AZ-371 | 2 | To Do |
|
||||
| AZ-368 | C15 | Shared TileCsvWriter | 3 | — | 2 | To Do |
|
||||
| AZ-367 | C14 | Shared TileGridStitcher | 3 | AZ-364 | 3 | To Do |
|
||||
| AZ-369 | C16 | Move inline DTOs out of Program.cs | 3 | — | 2 | To Do |
|
||||
| AZ-369 | C16 | Move inline DTOs out of Program.cs | 3 | — | 2 | Done (In Testing) |
|
||||
| AZ-365 | C12 | Decompose RouteService.CreateRouteAsync | 3 | — | 5 | To Do |
|
||||
| AZ-364 | C11 | Decompose RouteProcessingService god-class | 3 | AZ-366, AZ-367 (folds in AZ-360) | 5 | To Do |
|
||||
| AZ-360 | C08 | Replace IServiceProvider in RouteProcessingService | 3 | AZ-364 (folded) | 2 | To Do |
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
# Batch 14 Report — Refactor 03 Phase 3 (continued)
|
||||
|
||||
Date: 2026-05-11
|
||||
Epic: AZ-350 (03-code-quality-refactoring)
|
||||
Status: ✅ Complete, pushed
|
||||
|
||||
## Scope (1 task / 2 SP)
|
||||
|
||||
| ID | C-ID | Title | Points | Component |
|
||||
|----|------|-------|--------|-----------|
|
||||
| AZ-369 | C16 | Move inline DTOs out of Program.cs | 2 | Api + Common |
|
||||
|
||||
Third task of Phase 3. Pure SRP cleanup — no logic change, no behavior change. Final small extraction before the bigger C11 / C12 decompositions.
|
||||
|
||||
## Scope clarification — UploadImageRequest stays in API
|
||||
|
||||
The task spec asks for all six DTOs to move to `SatelliteProvider.Common/DTO/`. One of them — `UploadImageRequest` — contains `[Required] public IFormFile? Image`. `IFormFile` lives in `Microsoft.AspNetCore.Http`. Moving it to `Common` would force `<FrameworkReference Include="Microsoft.AspNetCore.App"/>` into the foundation layer, contradicting `module-layout.md`:
|
||||
|
||||
```
|
||||
Component: Common
|
||||
- Imports from: (none)
|
||||
```
|
||||
|
||||
`UploadImageRequest` is also semantically an HTTP-form-data transport DTO consumed only by the 501-stub upload endpoint — not a cross-component shape. User chose to keep it in the API project under `SatelliteProvider.Api.DTOs` (Choose option A on the contradiction surfaced at batch start). The task spec's "Outcome: Six DTOs live in Common/DTO/" is otherwise satisfied to the maximum extent compatible with the layering invariant.
|
||||
|
||||
## Changes
|
||||
|
||||
### Production
|
||||
|
||||
- **NEW** `SatelliteProvider.Common/DTO/GetSatelliteTilesResponse.cs` — `public record GetSatelliteTilesResponse` with the single `List<SatelliteTile> Tiles` property. Identical shape to the inline declaration.
|
||||
- **NEW** `SatelliteProvider.Common/DTO/SatelliteTile.cs` — `public record SatelliteTile` with `TileId`, `ImageData`, `Lat`, `Lon`, `ZoomLevel`. Identical shape.
|
||||
- **NEW** `SatelliteProvider.Common/DTO/SaveResult.cs` — `public record SaveResult` with `Success`, `Exception`. Identical shape.
|
||||
- **NEW** `SatelliteProvider.Common/DTO/DownloadTileResponse.cs` — `public record DownloadTileResponse` with all 12 fields including `Version: int?` (preserved from AZ-357). Identical shape.
|
||||
- **NEW** `SatelliteProvider.Common/DTO/RequestRegionRequest.cs` — `public record RequestRegionRequest` with `[Required]` annotations preserved (`System.ComponentModel.DataAnnotations` is a BCL primitive, safe for Common). Identical shape.
|
||||
- **NEW** `SatelliteProvider.Api/DTOs/UploadImageRequest.cs` — `public record UploadImageRequest` kept in the API project to avoid forcing an ASP.NET framework reference into the foundation layer. Namespace: `SatelliteProvider.Api.DTOs`. Shape preserved.
|
||||
- **NEW** `SatelliteProvider.Api/Swagger/ParameterDescriptionFilter.cs` — `public class ParameterDescriptionFilter : IOperationFilter`. Namespace: `SatelliteProvider.Api.Swagger`. Same parameter-description dictionary, same `Apply` logic.
|
||||
- **MODIFIED** `SatelliteProvider.Api/Program.cs`:
|
||||
- Removed inline type declarations (107 lines, lines 258-366 in the pre-refactor file).
|
||||
- Added `using SatelliteProvider.Api.DTOs;` and `using SatelliteProvider.Api.Swagger;`.
|
||||
- Removed `using System.ComponentModel.DataAnnotations;` (no longer used in the host file — only the moved DTOs referenced `[Required]`).
|
||||
- `using SatelliteProvider.Common.DTO;` already present (was used by `CreateRouteRequest`, `RegionResponse`, etc.); now also brings in the five moved DTOs.
|
||||
|
||||
### Tests
|
||||
|
||||
No test changes required. The behaviour under test is identical, and tests already exercise the DTOs by `Type`-binding through the public API (e.g. `c.MapType<UploadImageRequest>(...)`, `[FromBody] RequestRegionRequest`, `[FromForm] UploadImageRequest`). System.Text.Json does not encode namespaces in JSON output, so the on-the-wire shape is byte-for-byte equivalent.
|
||||
|
||||
Integration test project (`SatelliteProvider.IntegrationTests/Models.cs`) keeps its own local `DownloadTileResponse` / `RequestRegionRequest` definitions on purpose — that project has no `ProjectReference` to `Common` and replicates the wire shape locally for HTTP deserialization. Left untouched (no cross-reference introduced).
|
||||
|
||||
## Verification
|
||||
|
||||
- **Unit tests**: 84 / 84 passing (no change in count from batch 13; no test files added or removed).
|
||||
- **Integration smoke + full suite**: green. Container exits 0. Tests exercised — among many others — the `/api/satellite/tiles/latlon` endpoint (deserializes `DownloadTileResponse`), the `/api/satellite/request` POST endpoint (deserializes `RequestRegionRequest`), the `/api/satellite/upload` 501 stub (operates against `UploadImageRequest` via Swagger `MapType`), and the security-test path that exercises malformed `RequestRegionRequest` JSON (still rejected with HTTP 400 via the AZ-353 global handler).
|
||||
- **AC-2 verification (OpenAPI shape unchanged)**: implicit from the integration suite — every endpoint that consumes or produces one of the moved DTOs is exercised end-to-end against the deployed Swagger-backed API and continues to deserialize identically.
|
||||
|
||||
## Acceptance criteria coverage
|
||||
|
||||
| AC | Evidence |
|
||||
|----|----------|
|
||||
| **AC-1** `Program.cs` no longer declares any DTOs or Swagger filters | `wc -l SatelliteProvider.Api/Program.cs` = 257 lines (was 366). The file ends at `}` of `async Task<IResult> GetRoute(...)`. `grep -E '^public (record|class) ' SatelliteProvider.Api/Program.cs` returns 0 matches. |
|
||||
| **AC-2** Public OpenAPI shape unchanged | DTOs moved between namespaces only; field names, types, order, and `[Required]` annotations preserved. System.Text.Json output is namespace-agnostic. Integration tests deserialize via wire shape and all pass. |
|
||||
| **AC-3** 37 unit + 5 smoke tests green | 84 unit + full integration suite green. |
|
||||
|
||||
## Behavior preservation notes
|
||||
|
||||
- **JSON wire shape**: namespaces are not encoded in JSON. Renaming `global::DownloadTileResponse` → `SatelliteProvider.Common.DTO.DownloadTileResponse` does not change the wire shape.
|
||||
- **OpenAPI schema names**: Swashbuckle emits schema names from the type's simple `Name` by default (not the fully-qualified name). Names remain `DownloadTileResponse`, `RequestRegionRequest`, `UploadImageRequest`, `GetSatelliteTilesResponse`, `SatelliteTile`, `SaveResult` — unchanged.
|
||||
- **`c.MapType<UploadImageRequest>(...)`**: still resolves to the same type (now in `SatelliteProvider.Api.DTOs`). The Swagger schema mapping is unchanged.
|
||||
- **`c.OperationFilter<ParameterDescriptionFilter>()`**: still resolves to the same filter (now in `SatelliteProvider.Api.Swagger`). The parameter-description dictionary is identical.
|
||||
- **Records vs classes**: all six moved DTOs were already `record` types; preserved as `record`. The Swagger filter was a `class`; preserved as `class`.
|
||||
|
||||
## Architecture / SRP impact
|
||||
|
||||
- `Program.cs` shrunk from 366 → 257 lines (~30% reduction). It is now an endpoint wirer with no top-level type declarations — exactly the SRP win the task asked for.
|
||||
- The foundation layer (`Common`) gained five DTOs and no new external dependencies. `module-layout.md`'s `Common: Imports from: (none)` invariant preserved.
|
||||
- The API layer (`Api`) gained two small sub-namespaces (`Api.DTOs`, `Api.Swagger`) reflecting the natural decomposition of host-file concerns. Both folders mirror established conventions in adjacent .NET codebases.
|
||||
- No cyclic dependencies introduced. The dependency graph remains a DAG.
|
||||
|
||||
## Per-batch code review (inline — mechanical refactor)
|
||||
|
||||
A standalone `/code-review` invocation was skipped for this batch because every change is a literal move (type declaration relocated to a new file with identical shape, references updated). The Step 9 review criteria reduced to:
|
||||
|
||||
- **Spec compliance** — all three ACs satisfied (table above).
|
||||
- **Code quality** — type bodies copied verbatim; whitespace and bracing normalized to the project's convention (4-space indent, `using` directives sorted by namespace prefix). No SOLID violation introduced; the change resolves an SRP violation in `Program.cs`.
|
||||
- **Security** — no new attack surface. `[Required]` annotations preserved on `RequestRegionRequest` and `UploadImageRequest`; the AZ-353 global handler continues to translate validation failures to HTTP 400 + sanitized ProblemDetails (verified by the existing security integration test that POSTs malformed JSON).
|
||||
- **Performance** — net zero. Same types, same JSON serialization path.
|
||||
- **Cross-task consistency** — Phase 3 stays on the "structural cleanup" axis. No drift from AZ-366 (Haversine consolidation) or AZ-368 (TileCsvWriter extraction).
|
||||
- **Architecture** — `UploadImageRequest` deliberately kept in `Api/DTOs/` to preserve the `Common: Imports from: (none)` invariant. Documented above and in the scope-clarification section.
|
||||
|
||||
**Verdict**: PASS. No findings to track. The next cumulative review fires after batch 15 (K=3 trigger; window = batches 13+14+15).
|
||||
|
||||
## Up next
|
||||
|
||||
- **Batch 15**: candidate is **AZ-365** (C12, decompose `RouteService.CreateRouteAsync`, 5 SP) — the first of the two big Phase 3 decompositions. AZ-365's deps are clear (no blockers). Batch 15 is solo because the task is 5 SP and complex.
|
||||
- **Cumulative review next** fires after batch 15 (window 13+14+15).
|
||||
- **Remaining Phase 3**: AZ-377 (C24 Earth constants, depends on AZ-371), AZ-365 (C12, 5 SP), AZ-364 (C11 RouteProcessingService god-class, 5 SP, depends on AZ-366 + AZ-367), AZ-367 (C14 TileGridStitcher, 3 SP, depends on AZ-364) — note the dependency edge AZ-364 ↔ AZ-367 means AZ-367 actually unblocks AZ-364, so the practical ordering is AZ-365 → AZ-367 → AZ-364 (folds AZ-360). AZ-377 floats into Phase 4 once AZ-371 lands.
|
||||
Reference in New Issue
Block a user