diff --git a/.cursor/rules/coderule.mdc b/.cursor/rules/coderule.mdc index 4de3f25..8940f09 100644 --- a/.cursor/rules/coderule.mdc +++ b/.cursor/rules/coderule.mdc @@ -11,6 +11,7 @@ alwaysApply: true - Avoid boilerplate and unnecessary indirection, but never sacrifice readability for brevity. - Never suppress errors silently — no `2>/dev/null`, empty `catch` blocks, bare `except: pass`, or discarded error returns. These hide the information you need most when something breaks. If an error is truly safe to ignore, log it or comment why. - Do not add comments that merely narrate what the code does. Comments are appropriate for: non-obvious business rules, workarounds with references to issues/bugs, safety invariants, and public API contracts. Make comments as short and concise as possible. Exception: every test must use the Arrange / Act / Assert pattern with language-appropriate comment syntax (`# Arrange` for Python, `// Arrange` for C#/Rust/JS/TS). Omit any section that is not needed (e.g. if there is no setup, skip Arrange; if act and assert are the same line, keep only Assert) +- API consumer documentation (OpenAPI / Swagger `Description` and `Summary`, REST API reference docs, public SDK docstrings) is written for the *external API consumer*, not the implementer. Do NOT include task IDs (`AZ-NNN`, `JIRA-NNN`), contract-doc filenames (`tile-inventory.md v2.0.0`), version-bump history, or implementation milestones in these strings. Internal change tracking belongs in commit messages, contract docs, changelogs, and code comments — never in the public API description. Extend an existing pattern only if it already follows this rule; if the existing description leads with internal noise, treat that as a defect and clean it (or surface it to the user) rather than propagating it. - Do not add verbose debug/trace logs by default. Log exceptions, security events (auth failures, permission denials), and business-critical state transitions. Add debug-level logging only when asked. - Do not put code annotations unless it was asked specifically - Write code that takes into account the different environments: development, production diff --git a/AGENTS.md b/AGENTS.md index 42e54cd..488cf95 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -177,7 +177,7 @@ docker-compose -f docker-compose.yml -f docker-compose.tests.yml up --build --ab ### Configuration Values Development defaults: -- PostgreSQL: localhost:5432, user/pass: postgres/postgres +- PostgreSQL: localhost:5433 (host-side, mapped to container port 5432), user/pass: postgres/postgres - API: http://localhost:5100 - Max zoom level: 20 - Default zoom level: 18 diff --git a/README.md b/README.md index 715999e..0af7a02 100644 --- a/README.md +++ b/README.md @@ -434,7 +434,7 @@ Log level can be adjusted in `appsettings.json` under `Serilog:MinimumLevel`. ### Service won't start - Check Docker is running -- Verify ports 5100 and 5432 are available +- Verify ports 5100 and 5433 are available (Postgres host-side; the container itself listens on 5432 inside the docker network) - Check logs: `docker-compose logs api` ### Tiles not downloading diff --git a/SatelliteProvider.Api/GlobalExceptionHandler.cs b/SatelliteProvider.Api/GlobalExceptionHandler.cs index b5be05f..42e0fcd 100644 --- a/SatelliteProvider.Api/GlobalExceptionHandler.cs +++ b/SatelliteProvider.Api/GlobalExceptionHandler.cs @@ -1,3 +1,4 @@ +using System.Text.Json; using Microsoft.AspNetCore.Diagnostics; using Microsoft.AspNetCore.Mvc; @@ -60,6 +61,30 @@ public sealed class GlobalExceptionHandler : IExceptionHandler { httpContext.Response.StatusCode = badRequest.StatusCode; + // AZ-795: deserialization failures (unknown field via UnmappedMemberHandling.Disallow, + // type mismatch, malformed JSON) surface here as BadHttpRequestException with a + // System.Text.Json `JsonException` somewhere in the inner-exception chain. Convert + // them to RFC 7807 ValidationProblemDetails so wire-format errors share the same + // shape as FluentValidation business-rule errors — see + // `_docs/02_document/contracts/api/error-shape.md`. + var deserializationErrors = TryExtractDeserializationErrors(badRequest); + if (deserializationErrors is not null && badRequest.StatusCode == StatusCodes.Status400BadRequest) + { + var validation = new ValidationProblemDetails(deserializationErrors) + { + Status = badRequest.StatusCode, + Title = "One or more validation errors occurred.", + Type = "https://tools.ietf.org/html/rfc7231#section-6.5.1", + }; + + await httpContext.Response.WriteAsJsonAsync( + validation, + options: null, + contentType: "application/problem+json", + cancellationToken: cancellationToken); + return; + } + var problem = new ProblemDetails { Status = badRequest.StatusCode, @@ -73,4 +98,36 @@ public sealed class GlobalExceptionHandler : IExceptionHandler contentType: "application/problem+json", cancellationToken: cancellationToken); } + + private static IDictionary? TryExtractDeserializationErrors(BadHttpRequestException ex) + { + var current = ex.InnerException; + while (current is not null) + { + if (current is JsonException jsonEx) + { + var path = NormalizeJsonPath(jsonEx.Path); + var message = string.IsNullOrEmpty(jsonEx.Message) + ? "Invalid JSON." + : jsonEx.Message; + + return new Dictionary + { + [path] = new[] { message } + }; + } + + current = current.InnerException; + } + + return null; + } + + private static string NormalizeJsonPath(string? path) + { + if (string.IsNullOrEmpty(path)) return "$"; + return path.StartsWith("$.", StringComparison.Ordinal) + ? path.Substring(2) + : path; + } } diff --git a/SatelliteProvider.Api/Program.cs b/SatelliteProvider.Api/Program.cs index d0905ec..6d3865f 100644 --- a/SatelliteProvider.Api/Program.cs +++ b/SatelliteProvider.Api/Program.cs @@ -1,3 +1,4 @@ +using FluentValidation; using Microsoft.AspNetCore.Authorization; using Microsoft.AspNetCore.Http.Features; using Microsoft.AspNetCore.Mvc; @@ -8,6 +9,7 @@ using SatelliteProvider.Api; using SatelliteProvider.Api.Authentication; using SatelliteProvider.Api.DTOs; using SatelliteProvider.Api.Swagger; +using SatelliteProvider.Api.Validators; using SatelliteProvider.DataAccess; using SatelliteProvider.DataAccess.Repositories; using SatelliteProvider.DataAccess.TypeHandlers; @@ -98,14 +100,28 @@ builder.Services.AddCors(options => builder.Services.AddProblemDetails(); builder.Services.AddExceptionHandler(); +// AZ-795: strict JSON parsing — unknown fields are rejected at the deserializer +// level instead of being silently dropped. Pairs with the per-endpoint +// FluentValidation filter (`WithValidation()`) so the API has a single +// uniform RFC 7807 error contract for both wire-format failures and +// business-rule failures (`_docs/02_document/contracts/api/error-shape.md`). builder.Services.ConfigureHttpJsonOptions(options => { options.SerializerOptions.PropertyNamingPolicy = System.Text.Json.JsonNamingPolicy.CamelCase; options.SerializerOptions.PropertyNameCaseInsensitive = true; + options.SerializerOptions.UnmappedMemberHandling = System.Text.Json.Serialization.JsonUnmappedMemberHandling.Disallow; options.SerializerOptions.Converters.Add( new System.Text.Json.Serialization.JsonStringEnumConverter(System.Text.Json.JsonNamingPolicy.CamelCase)); }); +// AZ-795: register every IValidator in this assembly with DI so the +// generic ValidationEndpointFilter can resolve them at request time. +// GlobalValidatorConfig.ApplyOnce() centralizes process-wide FluentValidation +// configuration (camelCase property paths, etc.) so the API host and the +// unit-test fixture share one source of truth — see error-shape.md Inv-4. +builder.Services.AddValidatorsFromAssemblyContaining(); +GlobalValidatorConfig.ApplyOnce(); + builder.Services.AddEndpointsApiExplorer(); builder.Services.AddSwaggerGen(c => { @@ -199,13 +215,14 @@ app.MapGet("/api/satellite/tiles/mgrs", GetSatelliteTilesByMgrs) app.MapPost("/api/satellite/tiles/inventory", GetTilesInventory) .RequireAuthorization() + .WithValidation() .Accepts("application/json") .Produces(StatusCodes.Status200OK) .ProducesProblem(StatusCodes.Status400BadRequest) .WithOpenApi(op => new(op) { Summary = "Bulk tile inventory lookup by (z,x,y) coords or location_hash", - Description = "AZ-505 / `tile-inventory.md` v1.0.0. Body MUST populate exactly one of `tiles` (array of `{tileZoom,tileX,tileY}`) OR `locationHashes` (array of UUIDv5). Response order matches request order. Returns one entry per request item with `present: true|false`; when present, identity + recency fields are included. Hard cap: 5000 entries per call (HTTP 400 above)." + Description = "Body MUST populate exactly one of `tiles` (array of `{z, x, y}` slippy-map coordinates) OR `locationHashes` (array of UUIDv5 hashes) — sending both, or neither, is HTTP 400. Response order matches request order; each entry reports `present: true|false`, and when present includes `id`, `capturedAt`, `source`, `flightId`, `resolutionMPerPx`. Hard cap: 5000 entries per request." }); app.MapPost("/api/satellite/upload", UploadUavTileBatch) @@ -216,7 +233,7 @@ app.MapPost("/api/satellite/upload", UploadUavTileBatch) .WithOpenApi(op => new(op) { Summary = "Upload a batch of UAV-captured satellite tiles", - Description = "AZ-488 / `uav-tile-upload.md` v1.0.0. Multipart form: a JSON `metadata` field and an aligned `files` collection. Each item is graded by the 5-rule quality gate and persisted with `source='uav'` when accepted. Returns 200 with per-item results (mixed accept/reject), 400 for envelope-level errors (malformed metadata, missing files, oversized batch), 401 without a valid JWT, 403 without the `GPS` permission claim." + Description = "Multipart form: a JSON `metadata` field and an aligned `files` collection. Each item is graded by the 5-rule quality gate and persisted with `source='uav'` when accepted. Returns 200 with per-item results (mixed accept/reject), 400 for envelope-level errors (malformed metadata, missing files, oversized batch), 401 without a valid JWT, 403 without the `GPS` permission claim." }) .DisableAntiforgery(); @@ -225,7 +242,7 @@ app.MapPost("/api/satellite/request", RequestRegion) .WithOpenApi(op => new(op) { Summary = "Request tiles for a region", - Description = "Idempotent (AZ-362): POSTing the same `id` twice returns the existing region resource with HTTP 200 and does not enqueue duplicate background processing.", + Description = "Idempotent: POSTing the same `id` twice returns the existing region resource with HTTP 200 and does not enqueue duplicate background processing.", }); app.MapGet("/api/satellite/region/{id:guid}", GetRegionStatus) @@ -237,7 +254,7 @@ app.MapPost("/api/satellite/route", CreateRoute) .WithOpenApi(op => new(op) { Summary = "Create a route with intermediate points", - Description = "Idempotent (AZ-362): POSTing the same `id` twice returns the existing route resource with HTTP 200 and does not regenerate intermediate points or re-queue geofence regions.", + Description = "Idempotent: POSTing the same `id` twice returns the existing route resource with HTTP 200 and does not regenerate intermediate points or re-queue geofence regions.", }); app.MapGet("/api/satellite/route/{id:guid}", GetRoute) @@ -285,37 +302,10 @@ IResult GetSatelliteTilesByMgrs(string mgrs, double squareSideMeters) } async Task GetTilesInventory( - [FromBody] TileInventoryRequest? request, + [FromBody] TileInventoryRequest request, HttpContext httpContext, ITileService tileService) { - if (request is null) - { - return Results.Problem( - statusCode: StatusCodes.Status400BadRequest, - title: "Invalid tile inventory request", - detail: "Request body is required."); - } - - var tileCount = request.Tiles?.Count ?? 0; - var hashCount = request.LocationHashes?.Count ?? 0; - if ((tileCount == 0) == (hashCount == 0)) - { - return Results.Problem( - statusCode: StatusCodes.Status400BadRequest, - title: "Invalid tile inventory request", - detail: "Populate exactly one of `tiles` or `locationHashes`. Sending both, or neither, is not allowed."); - } - - var totalCount = Math.Max(tileCount, hashCount); - if (totalCount > TileInventoryLimits.MaxEntriesPerRequest) - { - return Results.Problem( - statusCode: StatusCodes.Status400BadRequest, - title: "Invalid tile inventory request", - detail: $"Inventory request capped at {TileInventoryLimits.MaxEntriesPerRequest} entries; got {totalCount}."); - } - var response = await tileService.GetInventoryAsync(request, httpContext.RequestAborted); return Results.Ok(response); } diff --git a/SatelliteProvider.Api/SatelliteProvider.Api.csproj b/SatelliteProvider.Api/SatelliteProvider.Api.csproj index d8240ae..f0aacbf 100644 --- a/SatelliteProvider.Api/SatelliteProvider.Api.csproj +++ b/SatelliteProvider.Api/SatelliteProvider.Api.csproj @@ -7,6 +7,8 @@ + + diff --git a/SatelliteProvider.Api/Validators/GlobalValidatorConfig.cs b/SatelliteProvider.Api/Validators/GlobalValidatorConfig.cs new file mode 100644 index 0000000..51aa01c --- /dev/null +++ b/SatelliteProvider.Api/Validators/GlobalValidatorConfig.cs @@ -0,0 +1,31 @@ +using FluentValidation; + +namespace SatelliteProvider.Api.Validators; + +// AZ-795 / AZ-796: process-wide FluentValidation configuration shared by the +// API host and unit tests. Tests must call ApplyOnce() in their fixture setup +// so the property-name casing they assert against matches what the running +// API will produce — see `_docs/02_document/contracts/api/error-shape.md` +// invariant Inv-4 (camelCase paths in `errors` map). +public static class GlobalValidatorConfig +{ + private static readonly object _gate = new(); + private static bool _applied; + + public static void ApplyOnce() + { + lock (_gate) + { + if (_applied) return; + + ValidatorOptions.Global.PropertyNameResolver = (type, member, expression) => + { + var name = member?.Name; + if (string.IsNullOrEmpty(name)) return null; + return char.ToLowerInvariant(name[0]) + name[1..]; + }; + + _applied = true; + } + } +} diff --git a/SatelliteProvider.Api/Validators/InventoryRequestValidator.cs b/SatelliteProvider.Api/Validators/InventoryRequestValidator.cs new file mode 100644 index 0000000..6ff40c1 --- /dev/null +++ b/SatelliteProvider.Api/Validators/InventoryRequestValidator.cs @@ -0,0 +1,73 @@ +using FluentValidation; +using SatelliteProvider.Common.DTO; + +namespace SatelliteProvider.Api.Validators; + +// AZ-796: FluentValidation rules for POST /api/satellite/tiles/inventory. +// Wired through ValidationEndpointFilter at endpoint +// registration time (`WithValidation()` in Program.cs). +// Failures are converted to RFC 7807 ValidationProblemDetails per +// `_docs/02_document/contracts/api/error-shape.md` v1.0.0. +// +// Required-field detection (rules 5+) is partially handled at the deserializer +// level via `[JsonRequired]` on TileCoord.Z/X/Y plus +// `JsonSerializerOptions.UnmappedMemberHandling.Disallow` (AZ-795). This +// validator covers the non-deserializer-detectable rules: XOR populated, +// per-array entry caps, and slippy-map range constraints. +public sealed class InventoryRequestValidator : AbstractValidator +{ + public InventoryRequestValidator() + { + RuleFor(req => req).Custom((req, ctx) => + { + var hasTiles = req.Tiles is { Count: > 0 }; + var hasHashes = req.LocationHashes is { Count: > 0 }; + if (hasTiles == hasHashes) + { + ctx.AddFailure( + "$", + "Populate exactly one of `tiles` or `locationHashes` (sending both, or neither, is not allowed)."); + } + }); + + RuleFor(req => req.Tiles!.Count) + .LessThanOrEqualTo(TileInventoryLimits.MaxEntriesPerRequest) + .OverridePropertyName("tiles") + .WithMessage($"`tiles` must contain at most {TileInventoryLimits.MaxEntriesPerRequest} entries.") + .When(req => req.Tiles is not null); + + RuleFor(req => req.LocationHashes!.Count) + .LessThanOrEqualTo(TileInventoryLimits.MaxEntriesPerRequest) + .OverridePropertyName("locationHashes") + .WithMessage($"`locationHashes` must contain at most {TileInventoryLimits.MaxEntriesPerRequest} entries.") + .When(req => req.LocationHashes is not null); + + RuleForEach(req => req.Tiles) + .SetValidator(new TileCoordValidator()) + .When(req => req.Tiles is not null); + } +} + +internal sealed class TileCoordValidator : AbstractValidator +{ + private const int MaxZoom = 22; + + public TileCoordValidator() + { + RuleFor(c => c.Z) + .InclusiveBetween(0, MaxZoom) + .WithMessage($"`z` must be between 0 and {MaxZoom} (slippy-map zoom range)."); + + RuleFor(c => c.X) + .GreaterThanOrEqualTo(0) + .WithMessage("`x` must be ≥ 0.") + .Must((coord, x) => coord.Z >= 0 && coord.Z <= MaxZoom && x < (1L << coord.Z)) + .WithMessage(coord => $"`x` must be < 2^z = {(coord.Z >= 0 && coord.Z <= MaxZoom ? (1L << coord.Z).ToString() : "")} for z={coord.Z}."); + + RuleFor(c => c.Y) + .GreaterThanOrEqualTo(0) + .WithMessage("`y` must be ≥ 0.") + .Must((coord, y) => coord.Z >= 0 && coord.Z <= MaxZoom && y < (1L << coord.Z)) + .WithMessage(coord => $"`y` must be < 2^z = {(coord.Z >= 0 && coord.Z <= MaxZoom ? (1L << coord.Z).ToString() : "")} for z={coord.Z}."); + } +} diff --git a/SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs b/SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs new file mode 100644 index 0000000..fff6703 --- /dev/null +++ b/SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs @@ -0,0 +1,42 @@ +using FluentValidation; + +namespace SatelliteProvider.Api.Validators; + +// AZ-795: shared validation infrastructure. A generic IEndpointFilter that +// resolves IValidator from DI for the first argument of type T in the +// invoked endpoint and returns RFC 7807 ValidationProblemDetails (HTTP 400) +// with a structured `errors` map when the validator rejects. When validation +// passes, the filter forwards to the next stage unchanged. +// +// The filter is generic per request type; per-endpoint wire-up is done via +// `RouteHandlerBuilder.WithValidation()` (see ValidationEndpointFilterExtensions). +// Per AZ-795 Outcome: callers must NOT need per-endpoint try/catch boilerplate; +// the filter provides the uniform error contract documented in +// `_docs/02_document/contracts/api/error-shape.md`. +public sealed class ValidationEndpointFilter : IEndpointFilter where T : class +{ + public async ValueTask InvokeAsync( + EndpointFilterInvocationContext context, + EndpointFilterDelegate next) + { + var argument = context.Arguments.OfType().FirstOrDefault(); + if (argument is null) + { + return await next(context); + } + + var validator = context.HttpContext.RequestServices.GetService>(); + if (validator is null) + { + return await next(context); + } + + var result = await validator.ValidateAsync(argument, context.HttpContext.RequestAborted); + if (!result.IsValid) + { + return Results.ValidationProblem(result.ToDictionary()); + } + + return await next(context); + } +} diff --git a/SatelliteProvider.Api/Validators/ValidationEndpointFilterExtensions.cs b/SatelliteProvider.Api/Validators/ValidationEndpointFilterExtensions.cs new file mode 100644 index 0000000..a5b196e --- /dev/null +++ b/SatelliteProvider.Api/Validators/ValidationEndpointFilterExtensions.cs @@ -0,0 +1,19 @@ +namespace SatelliteProvider.Api.Validators; + +// AZ-795: ergonomic extension method for opting an endpoint into +// FluentValidation. Applied at MapPost/MapGet registration time: +// +// app.MapPost("/api/satellite/tiles/inventory", GetTilesInventory) +// .WithValidation(); +// +// One line per endpoint; no per-handler try/catch boilerplate; uniform +// RFC 7807 error shape — see `_docs/02_document/contracts/api/error-shape.md`. +public static class ValidationEndpointFilterExtensions +{ + public static RouteHandlerBuilder WithValidation(this RouteHandlerBuilder builder) + where T : class + { + builder.AddEndpointFilter>(); + return builder; + } +} diff --git a/SatelliteProvider.Api/appsettings.Development.json b/SatelliteProvider.Api/appsettings.Development.json index 1267d63..31af2f7 100644 --- a/SatelliteProvider.Api/appsettings.Development.json +++ b/SatelliteProvider.Api/appsettings.Development.json @@ -8,7 +8,7 @@ } }, "ConnectionStrings": { - "DefaultConnection": "Host=localhost;Port=5432;Database=satelliteprovider;Username=postgres;Password=postgres" + "DefaultConnection": "Host=localhost;Port=5433;Database=satelliteprovider;Username=postgres;Password=postgres" }, "Jwt": { "Secret": "DEV-ONLY-DO-NOT-USE-IN-PROD-replace-with-real-secret-via-JWT_SECRET-env-var", diff --git a/SatelliteProvider.Common/DTO/TileInventory.cs b/SatelliteProvider.Common/DTO/TileInventory.cs index ea129c6..df11307 100644 --- a/SatelliteProvider.Common/DTO/TileInventory.cs +++ b/SatelliteProvider.Common/DTO/TileInventory.cs @@ -1,3 +1,5 @@ +using System.Text.Json.Serialization; + namespace SatelliteProvider.Common.DTO; // AZ-505: bulk-list / inventory request envelope. Either `Tiles` OR @@ -12,21 +14,33 @@ public sealed class TileInventoryRequest public IReadOnlyList? LocationHashes { get; set; } } -// AZ-505: Slippy-map tile coordinate triple. Field naming matches the on-wire -// snake_case used by the existing `GET /tiles/{z}/{x}/{y}` and the AZ-484/AZ-503 -// `tiles` table columns (`tile_zoom`, `tile_x`, `tile_y`). +// AZ-505: Slippy-map tile coordinate triple. AZ-794 (cycle 7) renamed the +// wire-format fields from `tileZoom/tileX/tileY` → `z/x/y` to align with the +// OSM / slippy-map convention already used by `GET /tiles/{z}/{x}/{y}` and +// to shave wire-size on inventory requests carrying thousands of entries. +// The C# property names (`Z`, `X`, `Y`) intentionally mirror the wire names +// 1:1 so consumers don't need to mentally translate at the deserialization +// boundary. The DataAccess `TileEntity.TileZoom/TileX/TileY` columns are +// unchanged — that's a database identity, not a wire format. public sealed class TileCoord { - public int TileZoom { get; set; } - public int TileX { get; set; } - public int TileY { get; set; } + [JsonRequired] + public int Z { get; set; } + + [JsonRequired] + public int X { get; set; } + + [JsonRequired] + public int Y { get; set; } } // AZ-505: Inventory response. Entries are returned in the SAME ORDER as the // matching request input (per AC-1). When Request.Tiles was populated, each -// entry's `TileZoom`/`TileX`/`TileY` echoes the request entry; when -// Request.LocationHashes was populated, the coord triple fields are 0 (the -// caller already knows the hash and can map it back themselves). +// entry's `Z`/`X`/`Y` echoes the request entry; when Request.LocationHashes +// was populated, the coord triple fields are 0 (the caller already knows +// the hash and can map it back themselves). AZ-794 (cycle 7) renamed the +// coord triple to `z/x/y` to align wire format with the URL-path +// convention. public sealed class TileInventoryResponse { public IReadOnlyList Results { get; set; } = Array.Empty(); @@ -40,11 +54,14 @@ public sealed class TileInventoryResponse // `EstimatedBytes` is intentionally absent in v1.0.0 — adding the per-row // `stat()` cost is deferred until production profiling justifies it (see // AZ-505 Outcome bullet 1 + Excluded list). +// +// AZ-794 (cycle 7): coord triple renamed `tileZoom/tileX/tileY` → `z/x/y` +// (contract bumped to v2.0.0). public sealed class TileInventoryEntry { - public int TileZoom { get; set; } - public int TileX { get; set; } - public int TileY { get; set; } + public int Z { get; set; } + public int X { get; set; } + public int Y { get; set; } public Guid LocationHash { get; set; } public bool Present { get; set; } diff --git a/SatelliteProvider.IntegrationTests/IdempotentPostTests.cs b/SatelliteProvider.IntegrationTests/IdempotentPostTests.cs index fb0790c..4acac8b 100644 --- a/SatelliteProvider.IntegrationTests/IdempotentPostTests.cs +++ b/SatelliteProvider.IntegrationTests/IdempotentPostTests.cs @@ -103,8 +103,8 @@ public static class IdempotentPostTests createTilesZip = false, points = new[] { - new { latitude = 47.4617, longitude = 37.6470 }, - new { latitude = 47.4630, longitude = 37.6485 }, + new { lat = 47.4617, lon = 37.6470 }, + new { lat = 47.4630, lon = 37.6485 }, }, }); diff --git a/SatelliteProvider.IntegrationTests/ProblemDetailsAssertions.cs b/SatelliteProvider.IntegrationTests/ProblemDetailsAssertions.cs new file mode 100644 index 0000000..977c756 --- /dev/null +++ b/SatelliteProvider.IntegrationTests/ProblemDetailsAssertions.cs @@ -0,0 +1,102 @@ +using System.Text.Json; + +namespace SatelliteProvider.IntegrationTests; + +// AZ-795: shared ProblemDetails / ValidationProblemDetails assertion helper +// for integration tests. Every endpoint that emits a 4xx error MUST produce +// a body matching the contract in +// `_docs/02_document/contracts/api/error-shape.md` (v1.0.0). Tests use this +// helper instead of re-deriving the shape per call site. +public static class ProblemDetailsAssertions +{ + private static readonly JsonSerializerOptions JsonOptions = new() + { + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + PropertyNameCaseInsensitive = true + }; + + public static async Task ReadProblemDetailsAsync(HttpResponseMessage response, string label) + { + var contentType = response.Content.Headers.ContentType?.MediaType; + if (contentType is null || !contentType.Contains("application/problem+json", StringComparison.OrdinalIgnoreCase)) + { + var body = await response.Content.ReadAsStringAsync(); + throw new Exception( + $"{label}: expected Content-Type 'application/problem+json', got '{contentType}'. Body: {body}"); + } + + var stream = await response.Content.ReadAsStreamAsync(); + using var doc = await JsonDocument.ParseAsync(stream); + return doc.RootElement.Clone(); + } + + public static void AssertValidationProblem( + JsonElement problem, + int expectedStatus, + string label, + string? expectedErrorPath = null, + string? expectedErrorContains = null) + { + if (!problem.TryGetProperty("status", out var statusEl) || statusEl.GetInt32() != expectedStatus) + { + throw new Exception( + $"{label}: expected status={expectedStatus}, got {(statusEl.ValueKind == JsonValueKind.Number ? statusEl.GetInt32().ToString() : "missing")}"); + } + + if (!problem.TryGetProperty("title", out var titleEl) || string.IsNullOrEmpty(titleEl.GetString())) + { + throw new Exception($"{label}: expected non-empty 'title', got missing/empty."); + } + + if (!problem.TryGetProperty("errors", out var errorsEl) || errorsEl.ValueKind != JsonValueKind.Object) + { + throw new Exception($"{label}: expected 'errors' object, got {errorsEl.ValueKind}."); + } + + if (expectedErrorPath is not null) + { + if (!errorsEl.TryGetProperty(expectedErrorPath, out var fieldEl) || fieldEl.ValueKind != JsonValueKind.Array) + { + throw new Exception( + $"{label}: expected errors['{expectedErrorPath}'] array, got {(errorsEl.TryGetProperty(expectedErrorPath, out var raw) ? raw.ValueKind.ToString() : "missing")}. " + + $"Available paths: {string.Join(", ", EnumeratePaths(errorsEl))}."); + } + + if (expectedErrorContains is not null) + { + var first = fieldEl.EnumerateArray().FirstOrDefault(); + var firstStr = first.ValueKind == JsonValueKind.String ? first.GetString() : null; + if (firstStr is null || !firstStr.Contains(expectedErrorContains, StringComparison.OrdinalIgnoreCase)) + { + throw new Exception( + $"{label}: expected errors['{expectedErrorPath}'][0] to contain '{expectedErrorContains}', got '{firstStr}'."); + } + } + } + } + + public static void AssertProblemDetails( + JsonElement problem, + int expectedStatus, + string label) + { + if (!problem.TryGetProperty("status", out var statusEl) || statusEl.GetInt32() != expectedStatus) + { + throw new Exception( + $"{label}: expected status={expectedStatus}, got {(statusEl.ValueKind == JsonValueKind.Number ? statusEl.GetInt32().ToString() : "missing")}"); + } + + if (!problem.TryGetProperty("title", out var titleEl) || string.IsNullOrEmpty(titleEl.GetString())) + { + throw new Exception($"{label}: expected non-empty 'title', got missing/empty."); + } + } + + private static IEnumerable EnumeratePaths(JsonElement errorsEl) + { + foreach (var prop in errorsEl.EnumerateObject()) + { + yield return prop.Name; + } + } +} diff --git a/SatelliteProvider.IntegrationTests/Program.cs b/SatelliteProvider.IntegrationTests/Program.cs index 5d82314..d3686bf 100644 --- a/SatelliteProvider.IntegrationTests/Program.cs +++ b/SatelliteProvider.IntegrationTests/Program.cs @@ -139,6 +139,7 @@ class Program await StubAndErrorContractTests.RunAll(httpClient); await IdempotentPostTests.RunAll(httpClient); await TileInventoryTests.RunAll(httpClient); + await TileInventoryValidationTests.RunAll(httpClient); await LeafletPathIndexOnlyTests.RunAll(connectionString); await MigrationTests.RunAll(); } @@ -162,6 +163,7 @@ class Program await StubAndErrorContractTests.RunAll(httpClient); await IdempotentPostTests.RunAll(httpClient); await TileInventoryTests.RunAll(httpClient); + await TileInventoryValidationTests.RunAll(httpClient); await LeafletPathIndexOnlyTests.RunAll(connectionString); await MigrationTests.RunAll(); } diff --git a/SatelliteProvider.IntegrationTests/TileInventoryTests.cs b/SatelliteProvider.IntegrationTests/TileInventoryTests.cs index 4254bb3..5e04677 100644 --- a/SatelliteProvider.IntegrationTests/TileInventoryTests.cs +++ b/SatelliteProvider.IntegrationTests/TileInventoryTests.cs @@ -60,10 +60,10 @@ public static class TileInventoryTests var seed = (int)(DateTime.UtcNow.Ticks % int.MaxValue); var random = new Random(seed); var presentCoords = Enumerable.Range(0, 12) - .Select(i => new TileCoord { TileZoom = zoom, TileX = 600_000 + (seed % 1000) * 100 + i, TileY = 700_000 + (seed % 1000) * 100 + i }) + .Select(i => new TileCoord { Z = zoom, X = 50_000 + (seed % 1000) * 100 + i, Y = 60_000 + (seed % 1000) * 100 + i }) .ToArray(); var absentCoords = Enumerable.Range(0, 13) - .Select(i => new TileCoord { TileZoom = zoom, TileX = 800_000 + (seed % 1000) * 100 + i, TileY = 900_000 + (seed % 1000) * 100 + i }) + .Select(i => new TileCoord { Z = zoom, X = 80_000 + (seed % 1000) * 100 + i, Y = 100_000 + (seed % 1000) * 100 + i }) .ToArray(); // Pre-seed the present cells. Mix sources / flights to exercise the @@ -74,7 +74,7 @@ public static class TileInventoryTests for (var i = 0; i < presentCoords.Length; i++) { var coord = presentCoords[i]; - var locationHash = Uuidv5.LocationHashForTile(coord.TileZoom, coord.TileX, coord.TileY); + var locationHash = Uuidv5.LocationHashForTile(coord.Z, coord.X, coord.Y); // Seed at least one google_maps row for every present cell. var googleId = Guid.NewGuid(); @@ -119,7 +119,7 @@ public static class TileInventoryTests } var presentHashes = presentCoords - .Select(c => Uuidv5.LocationHashForTile(c.TileZoom, c.TileX, c.TileY)) + .Select(c => Uuidv5.LocationHashForTile(c.Z, c.X, c.Y)) .ToHashSet(); for (var i = 0; i < allCoords.Length; i++) @@ -127,14 +127,14 @@ public static class TileInventoryTests var requestedCoord = allCoords[i]; var entry = body.Results[i]; - if (entry.TileZoom != requestedCoord.TileZoom || entry.TileX != requestedCoord.TileX || entry.TileY != requestedCoord.TileY) + if (entry.Z != requestedCoord.Z || entry.X != requestedCoord.X || entry.Y != requestedCoord.Y) { throw new Exception( - $"AC-1: entry {i} coords mismatch — request was ({requestedCoord.TileZoom},{requestedCoord.TileX},{requestedCoord.TileY}), " + - $"response is ({entry.TileZoom},{entry.TileX},{entry.TileY})"); + $"AC-1: entry {i} coords mismatch — request was ({requestedCoord.Z},{requestedCoord.X},{requestedCoord.Y}), " + + $"response is ({entry.Z},{entry.X},{entry.Y})"); } - var expectedHash = Uuidv5.LocationHashForTile(requestedCoord.TileZoom, requestedCoord.TileX, requestedCoord.TileY); + var expectedHash = Uuidv5.LocationHashForTile(requestedCoord.Z, requestedCoord.X, requestedCoord.Y); if (entry.LocationHash != expectedHash) { throw new Exception($"AC-1: entry {i} location_hash mismatch — expected {expectedHash}, got {entry.LocationHash}"); @@ -195,11 +195,11 @@ public static class TileInventoryTests var seed = (int)(DateTime.UtcNow.Ticks % int.MaxValue); var coord = new TileCoord { - TileZoom = zoom, - TileX = 1_200_000 + (seed % 1000), - TileY = 1_300_000 + (seed % 1000) + Z = zoom, + X = 130_000 + (seed % 1000), + Y = 150_000 + (seed % 1000) }; - var locationHash = Uuidv5.LocationHashForTile(coord.TileZoom, coord.TileX, coord.TileY); + var locationHash = Uuidv5.LocationHashForTile(coord.Z, coord.X, coord.Y); var googleId = Guid.NewGuid(); var googleCapturedAt = DateTime.UtcNow.AddHours(-2); @@ -252,7 +252,7 @@ public static class TileInventoryTests // Arrange var request = new TileInventoryRequest { - Tiles = new[] { new TileCoord { TileZoom = 18, TileX = 1, TileY = 1 } }, + Tiles = new[] { new TileCoord { Z = 18, X = 1, Y = 1 } }, LocationHashes = new[] { Guid.NewGuid() } }; @@ -309,7 +309,7 @@ public static class TileInventoryTests using var anonymous = new HttpClient { BaseAddress = baseAddress, Timeout = TimeSpan.FromSeconds(30) }; var request = new TileInventoryRequest { - Tiles = new[] { new TileCoord { TileZoom = 18, TileX = 1, TileY = 1 } } + Tiles = new[] { new TileCoord { Z = 18, X = 1, Y = 1 } } }; // Act @@ -361,7 +361,7 @@ public static class TileInventoryTests { var x = 100_000 + random.Next(0, 65_536); var y = 100_000 + random.Next(0, 65_536); - coords[i] = new TileCoord { TileZoom = zoom, TileX = x, TileY = y }; + coords[i] = new TileCoord { Z = zoom, X = x, Y = y }; var hash = Uuidv5.LocationHashForTile(zoom, x, y); idP.Value = Guid.NewGuid(); zP.Value = zoom; @@ -429,12 +429,12 @@ public static class TileInventoryTests VALUES (@id, @z, @x, @y, @lat, @lon, 200.0, 256, 'jpg', @fp, @src, @t, @t, @t, @flight, @loc) ON CONFLICT DO NOTHING;", conn); cmd.Parameters.AddWithValue("id", id); - cmd.Parameters.AddWithValue("z", coord.TileZoom); - cmd.Parameters.AddWithValue("x", coord.TileX); - cmd.Parameters.AddWithValue("y", coord.TileY); - cmd.Parameters.AddWithValue("lat", 60.0 + coord.TileX * 1e-9); - cmd.Parameters.AddWithValue("lon", 30.0 + coord.TileY * 1e-9); - cmd.Parameters.AddWithValue("fp", $"tiles/seed/{coord.TileZoom}/{coord.TileX}/{coord.TileY}.jpg"); + cmd.Parameters.AddWithValue("z", coord.Z); + cmd.Parameters.AddWithValue("x", coord.X); + cmd.Parameters.AddWithValue("y", coord.Y); + cmd.Parameters.AddWithValue("lat", 60.0 + coord.X * 1e-9); + cmd.Parameters.AddWithValue("lon", 30.0 + coord.Y * 1e-9); + cmd.Parameters.AddWithValue("fp", $"tiles/seed/{coord.Z}/{coord.X}/{coord.Y}.jpg"); cmd.Parameters.AddWithValue("src", source); // schema column is TIMESTAMP (no tz); Npgsql v6+ refuses to bind a // Kind=Utc DateTime into a plain timestamp column. Callers pass UTC diff --git a/SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs b/SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs new file mode 100644 index 0000000..da92f43 --- /dev/null +++ b/SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs @@ -0,0 +1,430 @@ +using System.Net; +using System.Net.Http.Json; +using System.Text; +using System.Text.Json; + +namespace SatelliteProvider.IntegrationTests; + +// AZ-796: end-to-end coverage for the inventory endpoint's strict input +// validation. Each test exercises one rule from the validator (FluentValidation +// for business rules, JsonSerializerOptions for wire-format rules) and asserts +// the response body conforms to the RFC 7807 ValidationProblemDetails contract +// in `_docs/02_document/contracts/api/error-shape.md` v1.0.0. +public static class TileInventoryValidationTests +{ + private const string InventoryPath = "/api/satellite/tiles/inventory"; + + public static async Task RunAll(HttpClient httpClient) + { + RouteTestHelpers.PrintTestHeader("Test: Inventory endpoint strict validation (AZ-796)"); + + await HappyPath_Returns200(httpClient); + + // Rule 1: body present + await EmptyBody_Returns400(httpClient); + // Rule 2: tiles required (one of tiles/locationHashes must be populated) + await NeitherPopulated_Returns400(httpClient); + await BothPopulated_Returns400(httpClient); + // Rule 3: tiles non-empty + await EmptyTilesArray_Returns400(httpClient); + // Rule 4: tiles max size + await TilesOverCap_Returns400(httpClient); + // Rule 5: each entry has z, x, y + await MissingZ_Returns400WithFieldPath(httpClient); + await MissingXAndY_Returns400(httpClient); + // Rule 6: non-negative integer fields + await NegativeAxis_Returns400(httpClient); + await TypeMismatch_Returns400(httpClient); + // Rule 7: z within supported zoom range + await ZoomOutOfRange_Returns400WithFieldPath(httpClient); + // Rule 8: x / y within tile-axis bounds + await XBeyondZoomBounds_Returns400(httpClient); + await YBeyondZoomBounds_Returns400(httpClient); + // Rule 9: unknown fields rejected + await UnknownRootField_Returns400(httpClient); + await UnknownNestedField_Returns400(httpClient); + await OldV1FieldName_Returns400(httpClient); + + Console.WriteLine("✓ Inventory validation tests: PASSED"); + } + + private static async Task HappyPath_Returns200(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: well-formed request with z/x/y triple → HTTP 200"); + + // Arrange + const string body = """{"tiles":[{"z":18,"x":1,"y":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + + // Assert + if (response.StatusCode != HttpStatusCode.OK) + { + var errorBody = await response.Content.ReadAsStringAsync(); + throw new Exception($"AZ-796 happy path: expected 200, got {(int)response.StatusCode}. Body: {errorBody}"); + } + + Console.WriteLine(" ✓ Valid {z, x, y} request returns HTTP 200"); + } + + private static async Task EmptyBody_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796 rule 1: empty body → HTTP 400"); + + // Arrange — POST with zero-byte body and JSON content type. The framework + // rejects the missing required body parameter before any handler/filter + // runs, so the response is basic RFC 7807 ProblemDetails with no `errors` + // map (vs. ValidationProblemDetails on field-level violations). + var content = new StringContent(string.Empty, Encoding.UTF8, "application/json"); + + // Act + var response = await httpClient.PostAsync(InventoryPath, content); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 empty body"); + + // Assert + ProblemDetailsAssertions.AssertProblemDetails(problem, expectedStatus: 400, label: "AZ-796 empty body"); + + Console.WriteLine(" ✓ Empty body rejected with HTTP 400 + ProblemDetails"); + } + + private static async Task NeitherPopulated_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796 rule 2: neither tiles nor locationHashes populated → HTTP 400"); + + // Arrange + const string body = """{}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 neither populated"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 neither populated", + expectedErrorPath: "$"); + + Console.WriteLine(" ✓ Empty object rejected with errors[\"$\"] (XOR rule)"); + } + + private static async Task BothPopulated_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796 rule 2: both tiles and locationHashes populated → HTTP 400"); + + // Arrange + const string body = """{"tiles":[{"z":18,"x":1,"y":1}],"locationHashes":["00000000-0000-0000-0000-000000000000"]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 both populated"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 both populated", + expectedErrorPath: "$"); + + Console.WriteLine(" ✓ Both populated rejected with errors[\"$\"] (XOR rule)"); + } + + private static async Task EmptyTilesArray_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796 rule 3: empty tiles array → HTTP 400"); + + // Arrange — XOR rule treats empty arrays as not-populated + const string body = """{"tiles":[]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 empty tiles array"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 empty tiles array", + expectedErrorPath: "$"); + + Console.WriteLine(" ✓ Empty tiles array rejected with HTTP 400"); + } + + private static async Task TilesOverCap_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796 rule 4: > 5000 tile entries → HTTP 400"); + + // Arrange — TileInventoryLimits.MaxEntriesPerRequest is 5000 + var sb = new StringBuilder(); + sb.Append("""{"tiles":["""); + for (var i = 0; i < 5001; i++) + { + if (i > 0) sb.Append(','); + sb.Append("""{"z":18,"x":1,"y":1}"""); + } + sb.Append("]}"); + + // Act + var response = await PostJsonAsync(httpClient, sb.ToString()); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 over cap"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 over cap", + expectedErrorPath: "tiles"); + + Console.WriteLine(" ✓ 5001-entry tiles array rejected with errors[\"tiles\"]"); + } + + private static async Task MissingZ_Returns400WithFieldPath(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: missing `z` → HTTP 400 with structured errors map"); + + // Arrange — JsonRequired on TileCoord.Z catches this at the deserializer layer. + const string body = """{"tiles":[{"x":1,"y":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 missing z"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-796 missing z"); + AssertErrorsContainsMention(problem, expectedMention: "z", label: "AZ-796 missing z"); + + Console.WriteLine(" ✓ Missing `z` rejected with errors map mentioning the field"); + } + + private static async Task MissingXAndY_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: missing `x` and `y` → HTTP 400"); + + // Arrange + const string body = """{"tiles":[{"z":18}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 missing x/y"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-796 missing x/y"); + + Console.WriteLine(" ✓ Missing `x` and `y` rejected with HTTP 400"); + } + + private static async Task ZoomOutOfRange_Returns400WithFieldPath(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: z out of slippy-map range → HTTP 400 with errors[\"tiles[0].z\"]"); + + // Arrange — z=30 is beyond the supported max of 22 + const string body = """{"tiles":[{"z":30,"x":0,"y":0}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 z=30"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 z=30", + expectedErrorPath: "tiles[0].z", + expectedErrorContains: "between 0 and 22"); + + Console.WriteLine(" ✓ z=30 rejected with errors[\"tiles[0].z\"] mentioning range"); + } + + private static async Task XBeyondZoomBounds_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: x ≥ 2^z → HTTP 400"); + + // Arrange — at z=2, valid x is 0..3; x=4 is invalid + const string body = """{"tiles":[{"z":2,"x":4,"y":0}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 x=4 z=2"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 x=4 z=2", + expectedErrorPath: "tiles[0].x"); + + Console.WriteLine(" ✓ x=4 at z=2 rejected with errors[\"tiles[0].x\"]"); + } + + private static async Task YBeyondZoomBounds_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: y ≥ 2^z → HTTP 400"); + + // Arrange — at z=0, valid y is 0..0; y=1 is invalid + const string body = """{"tiles":[{"z":0,"x":0,"y":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 y=1 z=0"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 y=1 z=0", + expectedErrorPath: "tiles[0].y"); + + Console.WriteLine(" ✓ y=1 at z=0 rejected with errors[\"tiles[0].y\"]"); + } + + private static async Task NegativeAxis_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: negative coordinate → HTTP 400"); + + // Arrange + const string body = """{"tiles":[{"z":18,"x":-1,"y":0}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 x=-1"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem( + problem, + expectedStatus: 400, + label: "AZ-796 x=-1", + expectedErrorPath: "tiles[0].x"); + + Console.WriteLine(" ✓ x=-1 rejected with errors[\"tiles[0].x\"]"); + } + + private static async Task UnknownRootField_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: unknown root field → HTTP 400 (UnmappedMemberHandling.Disallow)"); + + // Arrange + const string body = """{"unknownField":42,"tiles":[{"z":18,"x":1,"y":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 unknown root field"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-796 unknown root field"); + AssertErrorsContainsMention(problem, expectedMention: "unknownField", label: "AZ-796 unknown root field"); + + Console.WriteLine(" ✓ Unknown root field rejected; errors map names the field"); + } + + private static async Task UnknownNestedField_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: unknown nested field on tile entry → HTTP 400"); + + // Arrange + const string body = """{"tiles":[{"z":18,"x":1,"y":1,"foo":42}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 unknown nested field"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-796 unknown nested field"); + AssertErrorsContainsMention(problem, expectedMention: "foo", label: "AZ-796 unknown nested field"); + + Console.WriteLine(" ✓ Unknown nested field rejected; errors map names the field"); + } + + private static async Task OldV1FieldName_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-794 + AZ-796: legacy `tileZoom`/`tileX`/`tileY` field name → HTTP 400"); + + // Arrange — exact AZ-777 Phase 1 reproduction; v1 callers must now fail explicitly + // instead of silently coercing to (0,0,0). + const string body = """{"tiles":[{"tileZoom":18,"tileX":1,"tileY":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-794 legacy field"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-794 legacy field"); + AssertErrorsContainsMention(problem, expectedMention: "tileZoom", label: "AZ-794 legacy field"); + + Console.WriteLine(" ✓ Legacy v1.x field names rejected with explicit error (no silent coercion)"); + } + + private static async Task TypeMismatch_Returns400(HttpClient httpClient) + { + Console.WriteLine(); + Console.WriteLine("AZ-796: type mismatch (string where integer expected) → HTTP 400"); + + // Arrange + const string body = """{"tiles":[{"z":"eighteen","x":1,"y":1}]}"""; + + // Act + var response = await PostJsonAsync(httpClient, body); + var problem = await ProblemDetailsAssertions.ReadProblemDetailsAsync(response, "AZ-796 type mismatch"); + + // Assert + ProblemDetailsAssertions.AssertValidationProblem(problem, expectedStatus: 400, label: "AZ-796 type mismatch"); + + Console.WriteLine(" ✓ String-where-int rejected with HTTP 400"); + } + + private static Task PostJsonAsync(HttpClient httpClient, string body) + { + var content = new StringContent(body, Encoding.UTF8, "application/json"); + return httpClient.PostAsync(InventoryPath, content); + } + + private static void AssertErrorsContainsMention(JsonElement problem, string expectedMention, string label) + { + if (!problem.TryGetProperty("errors", out var errorsEl) || errorsEl.ValueKind != JsonValueKind.Object) + { + throw new Exception($"{label}: expected 'errors' object in ProblemDetails body."); + } + + var found = false; + foreach (var prop in errorsEl.EnumerateObject()) + { + if (prop.Name.Contains(expectedMention, StringComparison.OrdinalIgnoreCase)) + { + found = true; + break; + } + + foreach (var msg in prop.Value.EnumerateArray()) + { + if (msg.GetString()?.Contains(expectedMention, StringComparison.OrdinalIgnoreCase) == true) + { + found = true; + break; + } + } + + if (found) break; + } + + if (!found) + { + var paths = string.Join(", ", errorsEl.EnumerateObject().Select(p => p.Name)); + throw new Exception($"{label}: expected '{expectedMention}' to appear in errors keys or messages. Available paths: {paths}."); + } + } +} diff --git a/SatelliteProvider.Services.TileDownloader/TileService.cs b/SatelliteProvider.Services.TileDownloader/TileService.cs index 22c4676..551ea84 100644 --- a/SatelliteProvider.Services.TileDownloader/TileService.cs +++ b/SatelliteProvider.Services.TileDownloader/TileService.cs @@ -173,8 +173,8 @@ public class TileService : ITileService { foreach (var coord in tiles!) { - var hash = Uuidv5.LocationHashForTile(coord.TileZoom, coord.TileX, coord.TileY); - entries.Add((coord.TileZoom, coord.TileX, coord.TileY, hash)); + var hash = Uuidv5.LocationHashForTile(coord.Z, coord.X, coord.Y); + entries.Add((coord.Z, coord.X, coord.Y, hash)); } } else @@ -195,9 +195,9 @@ public class TileService : ITileService { results.Add(new TileInventoryEntry { - TileZoom = hasTiles ? zoom : tile.TileZoom, - TileX = hasTiles ? x : tile.TileX, - TileY = hasTiles ? y : tile.TileY, + Z = hasTiles ? zoom : tile.TileZoom, + X = hasTiles ? x : tile.TileX, + Y = hasTiles ? y : tile.TileY, LocationHash = hash, Present = true, Id = tile.Id, @@ -211,9 +211,9 @@ public class TileService : ITileService { results.Add(new TileInventoryEntry { - TileZoom = zoom, - TileX = x, - TileY = y, + Z = zoom, + X = x, + Y = y, LocationHash = hash, Present = false }); diff --git a/SatelliteProvider.Tests/GlobalExceptionHandlerTests.cs b/SatelliteProvider.Tests/GlobalExceptionHandlerTests.cs index b8d1f7e..ddbfd42 100644 --- a/SatelliteProvider.Tests/GlobalExceptionHandlerTests.cs +++ b/SatelliteProvider.Tests/GlobalExceptionHandlerTests.cs @@ -72,6 +72,45 @@ public class GlobalExceptionHandlerTests "BadHttpRequestException is a client error and must not be ERROR-logged as a server failure"); } + [Fact] + public async Task TryHandleAsync_DeserializationFailure_WritesValidationProblemDetailsWithJsonPath_AZ795() + { + // Arrange + var loggerMock = new Mock>(); + var handler = new GlobalExceptionHandler(loggerMock.Object); + var httpContext = new DefaultHttpContext { TraceIdentifier = "trace-AZ795" }; + httpContext.Response.Body = new MemoryStream(); + var jsonInner = new JsonException( + "The JSON property 'foo' could not be mapped to any .NET member contained in type 'TileInventoryRequest'.", + "$.tiles[0].foo", + lineNumber: null, + bytePositionInLine: null); + var bindFailure = new BadHttpRequestException( + "Failed to read parameter \"TileInventoryRequest request\" from request body.", + StatusCodes.Status400BadRequest, + jsonInner); + + // Act + var handled = await handler.TryHandleAsync(httpContext, bindFailure, CancellationToken.None); + + // Assert + handled.Should().BeTrue(); + httpContext.Response.StatusCode.Should().Be(StatusCodes.Status400BadRequest); + httpContext.Response.ContentType.Should().Contain("application/problem+json"); + + httpContext.Response.Body.Position = 0; + using var doc = JsonDocument.Parse(httpContext.Response.Body); + var root = doc.RootElement; + + root.GetProperty("status").GetInt32().Should().Be(400); + root.GetProperty("title").GetString().Should().Be("One or more validation errors occurred."); + root.GetProperty("type").GetString().Should().Be("https://tools.ietf.org/html/rfc7231#section-6.5.1"); + root.GetProperty("errors") + .GetProperty("tiles[0].foo")[0] + .GetString() + .Should().Contain("could not be mapped"); + } + [Fact] public async Task TryHandleAsync_LogsFullExceptionWithCorrelationId_AC2() { diff --git a/SatelliteProvider.Tests/TestSupport/ValidatorTestModuleInitializer.cs b/SatelliteProvider.Tests/TestSupport/ValidatorTestModuleInitializer.cs new file mode 100644 index 0000000..af833b7 --- /dev/null +++ b/SatelliteProvider.Tests/TestSupport/ValidatorTestModuleInitializer.cs @@ -0,0 +1,16 @@ +using System.Runtime.CompilerServices; +using SatelliteProvider.Api.Validators; + +namespace SatelliteProvider.Tests.TestSupport; + +internal static class ValidatorTestModuleInitializer +{ + // ModuleInitializer (.NET 5+) runs once per assembly load. We piggy-back the + // production GlobalValidatorConfig.ApplyOnce() so unit tests assert against + // the same FluentValidation property-name casing the live API produces. + [ModuleInitializer] + public static void Initialize() + { + GlobalValidatorConfig.ApplyOnce(); + } +} diff --git a/SatelliteProvider.Tests/Validators/InventoryRequestValidatorTests.cs b/SatelliteProvider.Tests/Validators/InventoryRequestValidatorTests.cs new file mode 100644 index 0000000..5a8b2ab --- /dev/null +++ b/SatelliteProvider.Tests/Validators/InventoryRequestValidatorTests.cs @@ -0,0 +1,261 @@ +using FluentAssertions; +using FluentValidation.TestHelper; +using SatelliteProvider.Api.Validators; +using SatelliteProvider.Common.DTO; + +namespace SatelliteProvider.Tests.Validators; + +public class InventoryRequestValidatorTests +{ + private readonly InventoryRequestValidator _validator = new(); + + [Fact] + public void Validate_TilesPopulated_LocationHashesNull_Passes() + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 18, X = 1, Y = 1 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldNotHaveAnyValidationErrors(); + } + + [Fact] + public void Validate_LocationHashesPopulated_TilesNull_Passes() + { + // Arrange + var request = new TileInventoryRequest + { + LocationHashes = new[] { Guid.NewGuid() } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldNotHaveAnyValidationErrors(); + } + + [Fact] + public void Validate_BothPopulated_FailsXorRule() + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 18, X = 1, Y = 1 } }, + LocationHashes = new[] { Guid.NewGuid() } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("$") + .WithErrorMessage("Populate exactly one of `tiles` or `locationHashes` (sending both, or neither, is not allowed)."); + } + + [Fact] + public void Validate_NeitherPopulated_FailsXorRule() + { + // Arrange + var request = new TileInventoryRequest(); + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("$"); + } + + [Fact] + public void Validate_BothEmpty_FailsXorRule() + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = Array.Empty(), + LocationHashes = Array.Empty() + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("$"); + } + + [Fact] + public void Validate_TilesAtCap_Passes() + { + // Arrange + var coords = Enumerable.Range(0, TileInventoryLimits.MaxEntriesPerRequest) + .Select(_ => new TileCoord { Z = 18, X = 1, Y = 1 }) + .ToArray(); + var request = new TileInventoryRequest { Tiles = coords }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldNotHaveValidationErrorFor("tiles"); + } + + [Fact] + public void Validate_TilesOverCap_FailsCapRule() + { + // Arrange + var coords = Enumerable.Range(0, TileInventoryLimits.MaxEntriesPerRequest + 1) + .Select(_ => new TileCoord { Z = 18, X = 1, Y = 1 }) + .ToArray(); + var request = new TileInventoryRequest { Tiles = coords }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles") + .WithErrorMessage($"`tiles` must contain at most {TileInventoryLimits.MaxEntriesPerRequest} entries."); + } + + [Fact] + public void Validate_LocationHashesOverCap_FailsCapRule() + { + // Arrange + var hashes = Enumerable.Range(0, TileInventoryLimits.MaxEntriesPerRequest + 1) + .Select(_ => Guid.NewGuid()) + .ToArray(); + var request = new TileInventoryRequest { LocationHashes = hashes }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("locationHashes") + .WithErrorMessage($"`locationHashes` must contain at most {TileInventoryLimits.MaxEntriesPerRequest} entries."); + } + + [Theory] + [InlineData(-1)] + [InlineData(23)] + [InlineData(100)] + public void Validate_TileZoomOutOfRange_FailsRangeRule(int z) + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = z, X = 0, Y = 0 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles[0].z"); + } + + [Theory] + [InlineData(0)] + [InlineData(18)] + [InlineData(22)] + public void Validate_TileZoomInRange_PassesRangeRule(int z) + { + // Arrange + var maxAxis = (1 << z) - 1; + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = z, X = maxAxis, Y = maxAxis } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldNotHaveValidationErrorFor("tiles[0].z"); + } + + [Fact] + public void Validate_TileXNegative_FailsRangeRule() + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 18, X = -1, Y = 0 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles[0].x"); + } + + [Fact] + public void Validate_TileXAtUpperBound_FailsRangeRule() + { + // Arrange — at z=2, valid x is 0..3, so x=4 is invalid + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 2, X = 4, Y = 0 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles[0].x") + .WithErrorMessage("`x` must be < 2^z = 4 for z=2."); + } + + [Fact] + public void Validate_TileYNegative_FailsRangeRule() + { + // Arrange + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 18, X = 0, Y = -1 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles[0].y"); + } + + [Fact] + public void Validate_TileYAtUpperBound_FailsRangeRule() + { + // Arrange — at z=0, valid y is 0..0, so y=1 is invalid + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 0, X = 0, Y = 1 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldHaveValidationErrorFor("tiles[0].y"); + } + + [Fact] + public void Validate_AxesAtMaxForZoom_Passes() + { + // Arrange — at z=18, valid x/y is 0..(2^18 - 1) = 0..262143 + var request = new TileInventoryRequest + { + Tiles = new[] { new TileCoord { Z = 18, X = 262_143, Y = 262_143 } } + }; + + // Act + var result = _validator.TestValidate(request); + + // Assert + result.ShouldNotHaveAnyValidationErrors(); + } +} diff --git a/_docs/02_document/architecture.md b/_docs/02_document/architecture.md index 8142821..0850c93 100644 --- a/_docs/02_document/architecture.md +++ b/_docs/02_document/architecture.md @@ -88,7 +88,7 @@ The N-source storage contract is authoritative in `_docs/02_document/contracts/d | Config | Development | Production | |--------|-------------|------------| -| Database | localhost:5432 (Docker) | Container network `db:5432` | +| Database | localhost:5433 (Docker) | Container network `db:5432` | | Secrets | appsettings.Development.json | Environment variables | | Logging | Console + File | File (./logs/) | | API URL | http://localhost:5100 | http://0.0.0.0:5100 | @@ -200,3 +200,28 @@ The authoritative source/flight markers are the `tiles.source` and `tiles.flight **Decision**: Use `IHostedService` implementations that consume from the in-process queue. **Consequences**: Clean separation of request handling and processing; lifecycle managed by the host. + +## 9. Input Validation (AZ-795) + +Every public HTTP endpoint MUST reject malformed or out-of-range payloads with HTTP 400 + RFC 7807 `ValidationProblemDetails`. The shared infrastructure landed in AZ-795 (cycle 7) is two collaborating layers: + +1. **Deserializer-level rejection** — `JsonSerializerOptions.UnmappedMemberHandling.Disallow` configured in `Program.cs` (`ConfigureHttpJsonOptions`) catches unknown fields, type mismatches, and malformed JSON. The framework wraps the resulting `JsonException` in `BadHttpRequestException`; `GlobalExceptionHandler` extracts the JSON path and emits a structured `ValidationProblemDetails` body. +2. **Business-rule rejection** — `FluentValidation` 12.0.0 validators registered via `AddValidatorsFromAssemblyContaining()` and wired through the generic `ValidationEndpointFilter` (`SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs`). Endpoints opt in via `RouteHandlerBuilder.WithValidation()`; the filter calls `Results.ValidationProblem(result.ToDictionary())` on failure. + +Both layers produce the wire shape documented in `_docs/02_document/contracts/api/error-shape.md` (v1.0.0). + +### Validator coverage + +| Endpoint | Request DTO | Validator | Status | Owning task | +|----------|-------------|-----------|--------|-------------| +| `POST /api/satellite/tiles/inventory` | `TileInventoryRequest` | `InventoryRequestValidator` | covered | AZ-796 (cycle 7) | +| `GET /tiles/{z}/{x}/{y}` | route params | (route-constraint only — `:int` covers types; AZ-795 deserializer guards body shape on POST endpoints only) | covered by route-constraint | AZ-487 (cycle 1, JWT gate) | +| `GET /api/satellite/tiles/latlon` | query params | (query-binding type checks via `[FromQuery]`; future AZ-795 child task to add explicit FluentValidation) | partial | future AZ-795 child | +| `POST /api/satellite/upload` | `UavTileBatchUploadRequest` (multipart) | (envelope-level validation in `UavTileUploadHandler`; future AZ-795 child to formalize as FluentValidation) | partial | future AZ-795 child | +| `POST /api/satellite/request` | `RequestRegionRequest` | (inline `SizeMeters` range check; future AZ-795 child) | partial | future AZ-795 child | +| `POST /api/satellite/route` | `CreateRouteRequest` | (typed `ArgumentException` path → 400; future AZ-795 child) | partial | future AZ-795 child | +| `GET /api/satellite/region/{id:guid}` | route param | (route-constraint `:guid`) | covered by route-constraint | — | +| `GET /api/satellite/route/{id:guid}` | route param | (route-constraint `:guid`) | covered by route-constraint | — | +| `GET /api/satellite/tiles/mgrs` | (stub) | n/a — returns 501 | n/a | AZ-356 | + +The `partial` rows are tracked under the AZ-795 epic; per-endpoint child tickets to be filed by parent-suite team after enumerating the surface from the OpenAPI spec. diff --git a/_docs/02_document/contracts/api/error-shape.md b/_docs/02_document/contracts/api/error-shape.md new file mode 100644 index 0000000..0386e70 --- /dev/null +++ b/_docs/02_document/contracts/api/error-shape.md @@ -0,0 +1,136 @@ +# Contract: error-shape + +**Component**: WebApi (`SatelliteProvider.Api`) — applies to every public HTTP endpoint +**Producer task**: AZ-795 — `_docs/02_tasks/done/AZ-795_strict_validation_epic.md` +**Consumer tasks**: every per-endpoint child of AZ-795 (first: AZ-796) plus every `gps-denied-onboard` HTTP client and every future browser/CLI consumer +**Version**: 1.0.0 +**Status**: frozen +**Last Updated**: 2026-05-22 + +## Purpose + +Defines the uniform RFC 7807 ProblemDetails / ValidationProblemDetails shape every public endpoint emits for client (4xx) errors. The contract exists so consumers can pattern-match against a single error payload regardless of which endpoint they called and regardless of whether the failure happened at the deserializer (unknown field, type mismatch) or at a FluentValidation rule (missing field, out-of-range value, business invariant). + +The contract is enforced by two collaborating pieces of shared infrastructure: + +1. **Deserializer-level rejection** — `JsonSerializerOptions.UnmappedMemberHandling.Disallow` (.NET 8+) catches unknown fields, type mismatches, and malformed JSON. The framework's `BadHttpRequestException` carries a `System.Text.Json.JsonException` as its inner exception; `GlobalExceptionHandler` (`SatelliteProvider.Api/GlobalExceptionHandler.cs`) extracts the JSON path and emits a `ValidationProblemDetails` body. +2. **Business-rule rejection** — `FluentValidation` (12.0.0) validators wired through the generic `ValidationEndpointFilter` (`SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs`). Endpoints opt in via `RouteHandlerBuilder.WithValidation()`. The filter calls `Results.ValidationProblem(result.ToDictionary())`, which produces an identically-shaped body. + +Both paths produce `Content-Type: application/problem+json`. Both populate the same `errors` map keyed by request-body field path. + +## Shape + +### Validation failures (HTTP 400) + +```jsonc +{ + "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1", + "title": "One or more validation errors occurred.", + "status": 400, + "errors": { + "tiles[0].z": ["The z field is required."], + "tiles[1]": ["The JSON property 'tileZoom' could not be mapped to any .NET member contained in type 'TileCoord'."] + } +} +``` + +Per-field rules: + +| Field | Type | Required | Description | +|-------|------|----------|-------------| +| `type` | URI string | yes | RFC 7231 §6.5.1 link for 400 (FluentValidation default) or RFC 9110 link for 5xx (server errors). | +| `title` | string | yes | Human-readable summary. Validation failures use `"One or more validation errors occurred."`; non-validation 400s use `"Bad Request"`. | +| `status` | integer | yes | Echoes the HTTP status code. | +| `errors` | object\ | required for validation failures | Keys are JSON-path-style request-body field names. Values are arrays of error messages. Empty array is not allowed for a present key. | +| `traceId` | string | optional | Correlation identifier for log lookup. Populated for 5xx; may be populated for 4xx if FluentValidation surfaces it. | + +### Field-path keys in `errors` + +Field-path keys MUST follow the same casing as the request body's JSON (camelCase root + dotted/indexed access for nested types). Examples: + +| Failure type | Example field-path key | +|--------------|------------------------| +| Missing root field | `tiles` | +| Missing nested field on tile entry | `tiles[0].z` | +| Out-of-range value | `tiles[3].z` | +| Unknown root field | `unknownField` (or `$` if path unavailable) | +| Unknown field inside nested object | `tiles[0].foo` | +| Both/neither XOR violation | `$` (request-body root) | + +The deserialization-failure path (unknown field, type mismatch) sets the key to the JSON path System.Text.Json reports. FluentValidation paths use the property names from `RuleFor(x => x.Tiles)` etc., which automatically produce camelCase paths matching the request body. + +### Generic 4xx errors (no validation context) + +Some 4xx responses (auth failures, not-found, framework binding errors that aren't JSON deserialization) emit the simpler ProblemDetails shape — no `errors` map: + +```jsonc +{ + "type": "https://datatracker.ietf.org/doc/html/rfc9110#name-401-unauthorized", + "title": "Unauthorized", + "status": 401 +} +``` + +| Status | Title | Notes | +|--------|-------|-------| +| 400 (non-validation) | `"Bad Request"` | Framework binding failures that don't carry a JsonException (rare). | +| 401 | (varies) | Emitted by the JwtBearer middleware via `WWW-Authenticate`; body content depends on framework version. | +| 403 | (varies) | Authorization failure. Body shape governed by ASP.NET Core defaults. | +| 404 | (varies) | Per-endpoint default; some endpoints emit a custom NotFound body (e.g. region/route). | +| 501 | `"Not implemented"` | Stub endpoints (e.g. `/api/satellite/tiles/mgrs`). | + +### 5xx errors + +Server errors emit the simpler ProblemDetails shape with a `correlationId` extension property pointing at the server log entry. The body NEVER contains the original exception message or stack trace (sanitization landed in AZ-353): + +```jsonc +{ + "type": "https://datatracker.ietf.org/doc/html/rfc9110#name-500-internal-server-error", + "title": "Internal Server Error", + "status": 500, + "detail": "An unexpected error occurred. Use the correlationId to look up the server log entry.", + "correlationId": "0HMBR..." +} +``` + +## Invariants + +- **Inv-1**: Every 4xx and 5xx response sets `Content-Type: application/problem+json`. +- **Inv-2**: Validation failures (HTTP 400 from FluentValidation OR from JSON deserialization with a JsonException inner exception) always include an `errors` object. +- **Inv-3**: Each `errors` entry has at least one message. Empty arrays are forbidden. +- **Inv-4**: Field-path keys in `errors` use the same casing as the request body (camelCase root, dotted/indexed access for nested types). +- **Inv-5**: 5xx responses include a `correlationId` extension property; 4xx responses do not. No 4xx response leaks server-internal state (DB connection strings, secrets, internal stack frames). +- **Inv-6**: Unknown fields at root or in any nested object are rejected with HTTP 400 — not silently dropped. The error key names the offending field path. +- **Inv-7**: Type mismatches (e.g. string where integer expected) are rejected with HTTP 400 and the error key names the offending field path. + +## Non-Goals + +- **Not covered**: i18n / translated error messages. Messages are English-only; consumers translate on their side if needed. +- **Not covered**: error codes. The `errors` map carries human-readable strings, not stable error codes. Consumers MUST NOT pattern-match on the string content; they pattern-match on field paths. +- **Not covered**: rate-limit or quota errors. Those are a separate concern with their own contract (TBD). +- **Not covered**: 1xx / 3xx responses. Those are framework-level and not shaped by this contract. + +## Versioning Rules + +- **Patch (1.0.x)**: Documentation clarifications, additional invariants that do not change wire behavior. +- **Minor (1.x.0)**: Adding an optional extension field to ProblemDetails (e.g., `correlationId` becoming standard for 4xx as well as 5xx). Adding new field-path conventions that are backward-compatible (e.g., a new `[i]` indexing rule). +- **Major (2.0.0)**: Changing `errors` map shape (e.g. swapping to error-code keys). Changing `Content-Type`. Renaming `errors` to anything else. Removing `correlationId` from 5xx. + +## Test Cases + +| Case | Input | Expected | Notes | +|------|-------|----------|-------| +| validation-missing-field | Inventory request with `tiles: [{ "z": 18 }]` (x, y missing) | HTTP 400 + `errors["tiles[0].x"]` and `errors["tiles[0].y"]` populated | Inv-2, Inv-4 | +| validation-out-of-range | Inventory request with `tiles: [{ "z": 30, "x": 1, "y": 1 }]` | HTTP 400 + `errors["tiles[0].z"]` mentioning supported zoom range | Inv-2 | +| validation-unknown-root-field | Body `{ "unknownField": 42, "tiles": [...] }` | HTTP 400 + `errors["unknownField"]` populated with "could not be mapped" | Inv-6 | +| validation-unknown-nested-field | Body `{ "tiles": [{ "z": 18, "x": 1, "y": 1, "foo": 42 }] }` | HTTP 400 + `errors["tiles[0].foo"]` populated | Inv-6 | +| validation-type-mismatch | Body `{ "tiles": [{ "z": "eighteen" }] }` | HTTP 400 + `errors["tiles[0].z"]` populated | Inv-7 | +| validation-xor-both-populated | Body with both `tiles` and `locationHashes` populated | HTTP 400 + `errors["$"]` (or root key) populated | Inv-2 | +| 5xx-includes-correlation-id | Endpoint throws unhandled exception | HTTP 500 + `correlationId` extension matching `httpContext.TraceIdentifier` | Inv-5 | +| 5xx-no-secret-leak | Exception message contains a connection string with `Password=hunter2` | HTTP 500 body contains neither the password nor the connection string | Inv-5 | + +## Change Log + +| Version | Date | Change | Author | +|---------|------|--------|--------| +| 1.0.0 | 2026-05-22 | Initial contract — uniform RFC 7807 ValidationProblemDetails shape for FluentValidation business-rule failures + JSON deserialization failures, including unknown-field rejection (`UnmappedMemberHandling.Disallow`). Sanitized ProblemDetails for 5xx (preserves AZ-353). Produced by AZ-795. | autodev (Step 10, cycle 7) | diff --git a/_docs/02_document/contracts/api/tile-inventory.md b/_docs/02_document/contracts/api/tile-inventory.md index 2442a85..836b9ea 100644 --- a/_docs/02_document/contracts/api/tile-inventory.md +++ b/_docs/02_document/contracts/api/tile-inventory.md @@ -1,11 +1,11 @@ # Contract: tile-inventory **Component**: WebApi (`SatelliteProvider.Api`) producing rows via TileDownloader (`SatelliteProvider.Services.TileDownloader`) -**Producer task**: AZ-505 — `_docs/02_tasks/todo/AZ-505_tile_inventory_http2_leaflet_index.md` +**Producer task**: AZ-505 — `_docs/02_tasks/done/AZ-505_tile_inventory_http2_leaflet_index.md` (initial); AZ-794 — `_docs/02_tasks/done/AZ-794_inventory_field_rename_osm.md` (v2.0.0 wire-format rename); AZ-796 — `_docs/02_tasks/done/AZ-796_inventory_endpoint_validation.md` (FluentValidation + ProblemDetails wiring) **Consumer tasks**: `gps-denied-onboard` AZ-316 (`c11_tile_downloader`), future mission-planner UI cache-sizing flows -**Version**: 1.0.0 +**Version**: 2.0.0 **Status**: frozen -**Last Updated**: 2026-05-12 +**Last Updated**: 2026-05-22 ## Purpose @@ -27,14 +27,14 @@ The request MUST carry a valid JWT (AZ-487). No `permissions` claim is required ### Request body -Exactly one of `tiles` OR `locationHashes` MUST be populated. Sending both, or neither, is HTTP 400. +Exactly one of `tiles` OR `locationHashes` MUST be populated. Sending both, or neither, is HTTP 400 (validation enforced by `InventoryRequestValidator` per AZ-796). ```jsonc -// Form A — coord-keyed +// Form A — coord-keyed (v2.0.0; AZ-794 renamed tileZoom/tileX/tileY → z/x/y) { "tiles": [ - { "tileZoom": 18, "tileX": 154321, "tileY": 95812 }, - { "tileZoom": 18, "tileX": 154322, "tileY": 95812 } + { "z": 18, "x": 154321, "y": 95812 }, + { "z": 18, "x": 154322, "y": 95812 } ] } @@ -51,28 +51,31 @@ Per-field constraints: | Field | Type | Required | Description | Constraints | |-------|------|----------|-------------|-------------| -| `tiles` | `TileCoord[]` | yes (XOR `locationHashes`) | Slippy-map tile coords | Up to 5000 entries per request. Each entry MUST have all three of `tileZoom`, `tileX`, `tileY`. | +| `tiles` | `TileCoord[]` | yes (XOR `locationHashes`) | Slippy-map tile coords | Up to 5000 entries per request. Each entry MUST have all three of `z`, `x`, `y`. | | `locationHashes` | `UUID[]` | yes (XOR `tiles`) | Pre-computed UUIDv5 `location_hash` values | Up to 5000 entries per request. Each entry MUST be RFC 4122 UUID. | Hard cap: **5000 entries per request** (`SatelliteProvider.Common.DTO.TileInventoryLimits.MaxEntriesPerRequest`). Anything larger → HTTP 400. The cap is 2× the AC-4 perf gate (2500 tiles). +Strict parsing: unknown fields at root or nested under any tile entry are rejected with HTTP 400 by `JsonSerializerOptions.UnmappedMemberHandling.Disallow` (AZ-795). The error body conforms to `error-shape.md` v1.0.0. + ### `TileCoord` (per entry under `tiles`) -| Field | Type | Required | Description | -|-------|------|----------|-------------| -| `tileZoom` | integer | yes | Slippy-map zoom level | -| `tileX` | integer | yes | Slippy-map tile column | -| `tileY` | integer | yes | Slippy-map tile row | +| Field | Type | Required | Description | Range | +|-------|------|----------|-------------|-------| +| `z` | integer | yes | Slippy-map zoom level | 0–22 (matches `tile_zoom` schema constraint) | +| `x` | integer | yes | Slippy-map tile column | 0 ≤ x < 2^z | +| `y` | integer | yes | Slippy-map tile row | 0 ≤ y < 2^z | ### Response body ```jsonc +// v2.0.0 — coord triple uses z/x/y (AZ-794) { "results": [ { - "tileZoom": 18, - "tileX": 154321, - "tileY": 95812, + "z": 18, + "x": 154321, + "y": 95812, "locationHash": "ad8c1c4c-2b27-5af4-902f-9c8baeed1e84", "present": true, "id": "5d83…", @@ -82,9 +85,9 @@ Hard cap: **5000 entries per request** (`SatelliteProvider.Common.DTO.TileInvent "resolutionMPerPx": 0.78125 }, { - "tileZoom": 18, - "tileX": 154322, - "tileY": 95812, + "z": 18, + "x": 154322, + "y": 95812, "locationHash": "5b8d0c2e-7f1a-5d3b-9c5e-1f3a8e7d2b6c", "present": false, "id": null, @@ -101,10 +104,10 @@ Per-entry fields: | Field | Type | Present when... | Description | |-------|------|-----------------|-------------| -| `tileZoom` | integer | always (Form A); zeroed (Form B) | Echoes the request entry's `tileZoom` when input was `tiles`; `0` when input was `locationHashes` (caller already knows the cell). | -| `tileX` | integer | always (Form A); zeroed (Form B) | Same as `tileZoom`. | -| `tileY` | integer | always (Form A); zeroed (Form B) | Same as `tileZoom`. | -| `locationHash` | UUIDv5 | always | `UUIDv5(TileNamespace, "{tileZoom}/{tileX}/{tileY}")`. Populated even when `present=false` so callers can persist the deterministic hash. | +| `z` | integer | always (Form A); zeroed (Form B) | Echoes the request entry's `z` when input was `tiles`; `0` when input was `locationHashes` (caller already knows the cell). | +| `x` | integer | always (Form A); zeroed (Form B) | Same as `z`. | +| `y` | integer | always (Form A); zeroed (Form B) | Same as `z`. | +| `locationHash` | UUIDv5 | always | `UUIDv5(TileNamespace, "{z}/{x}/{y}")`. Populated even when `present=false` so callers can persist the deterministic hash. | | `present` | bool | always | `true` iff a row exists in `tiles` with this `location_hash`. | | `id` | UUID | present=true | Most-recent row's `tiles.id`. Deterministic UUIDv5 for AZ-503+ rows; random for legacy rows. | | `capturedAt` | ISO-8601 UTC | present=true | `tiles.captured_at`. | @@ -120,15 +123,35 @@ Order invariant: `results[i]` corresponds to `request.tiles[i]` (or `request.loc |--------|------|--------------|----------|--------------| | `POST` | `/api/satellite/tiles/inventory` | `TileInventoryRequest` | `TileInventoryResponse` | 200, 400, 401 | +## Error shape + +All `400` responses conform to `_docs/02_document/contracts/api/error-shape.md` v1.0.0. Both wire-format failures (unknown fields, type mismatches; via the JSON deserializer) and business-rule failures (XOR violation, missing `z`/`x`/`y`, out-of-range zoom; via `InventoryRequestValidator`) emit a `ValidationProblemDetails` body with an `errors` map keyed by JSON-path-style field names. Example: + +```jsonc +{ + "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1", + "title": "One or more validation errors occurred.", + "status": 400, + "errors": { + "tiles[0].z": ["The z field is required."], + "tiles[1]": ["The JSON property 'tileZoom' could not be mapped to any .NET member contained in type 'TileCoord'."] + } +} +``` + +The first key shows a FluentValidation rule miss; the second shows a deserializer rejection of the *old* v1.x field name (callers still on the old shape get an explicit failure, not a silent 200 with zeroed coordinates — exactly the behaviour AZ-794 + AZ-796 were filed to enforce after the AZ-777 Phase 1 Jetson probe). + ## Invariants - **Inv-1**: Exactly one of `request.tiles` and `request.locationHashes` is populated and non-empty. Both-populated → 400; both-empty → 400. - **Inv-2**: `len(response.results) == len(request.tiles)` OR `len(request.locationHashes)` — never less, never more. -- **Inv-3**: `response.results[i].locationHash` is deterministic from `request.tiles[i]` (UUIDv5 over `"{zoom}/{x}/{y}"` with `Uuidv5.TileNamespace`) when Form A is used, or equals `request.locationHashes[i]` when Form B is used. +- **Inv-3**: `response.results[i].locationHash` is deterministic from `request.tiles[i]` (UUIDv5 over `"{z}/{x}/{y}"` with `Uuidv5.TileNamespace`) when Form A is used, or equals `request.locationHashes[i]` when Form B is used. - **Inv-4**: `response.results[i].present == true` iff a row exists in `tiles` with `location_hash = response.results[i].locationHash`. - **Inv-5**: When `present=true`, the returned row is the most-recent across sources/flights ordered by `(captured_at DESC, updated_at DESC, id DESC)` — same rule as `ITileRepository.GetByTileCoordinatesAsync` per `tile-storage` v2.0.0. - **Inv-6**: When `present=false`, `id` / `capturedAt` / `source` / `flightId` / `resolutionMPerPx` are all `null`. - **Inv-7**: `request.tiles.length` and `request.locationHashes.length` MUST be ≤ `TileInventoryLimits.MaxEntriesPerRequest` (5000); over the cap → 400. +- **Inv-8** (AZ-795 / AZ-796): Each `tiles[i].z` MUST satisfy `0 ≤ z ≤ 22`. Each `tiles[i].x` and `tiles[i].y` MUST satisfy `0 ≤ value < 2^z`. Out-of-range → 400 with `errors["tiles[i].z|x|y"]` populated. +- **Inv-9** (AZ-795): Unknown fields at root or in any nested object are rejected with HTTP 400; the error key names the offending JSON path. ## Non-Goals @@ -139,12 +162,13 @@ Order invariant: `results[i]` corresponds to `request.tiles[i]` (or `request.loc - **Not covered**: production deployment topology. Dev Kestrel runs `Http1AndHttp2` directly over TLS on port 8080 with a self-signed cert (`./certs/api.pfx`, generated by `scripts/run-tests.sh`) so ALPN can advertise `h2` — browsers and programmatic clients (httpx `http2=True`, .NET `HttpClient` with `HttpVersionPolicy.RequestVersionExact`) both multiplex over a single TLS connection. In production, TLS is expected to terminate at the ingress (Envoy / nginx / ALB) and Kestrel runs HTTP/2 cleartext behind it; AZ-505 verifies the protocol multiplexing semantics here, not the production termination layer. - **Not covered**: PMTiles or tar/multipart bundle endpoints. Rejected by AZ-503 parent rationale (HTTP/2 multistream is sufficient). - **Not covered**: write operations. Inventory is read-only; UAV writes go through `POST /api/satellite/upload` (`uav-tile-upload.md` v1.1.0). +- **Not covered**: backward-compatibility shim for v1.0.0 (`tileZoom/tileX/tileY`) field names. Per AZ-794 (Option 1 hard switch — single known consumer), v2.0.0 is the only accepted body shape; v1.x consumers receive HTTP 400 with `errors[*]: ["could not be mapped"]`. There is no transitional accept-both period. ## Versioning Rules -- **Patch (1.0.x)**: Documentation clarifications, additional invariants that do not change wire behavior. -- **Minor (1.x.0)**: Adding an optional response field that consumers may safely ignore (e.g., the future `estimatedBytes`); raising the entry cap; adding a third request form alongside the current two. -- **Major (2.0.0)**: Changing the response ordering rule; removing `present`; lowering the entry cap; making `flightId` required; adding voting / trust filtering to the read path. +- **Patch (2.0.x)**: Documentation clarifications, additional invariants that do not change wire behavior. +- **Minor (2.x.0)**: Adding an optional response field that consumers may safely ignore (e.g., the future `estimatedBytes`); raising the entry cap; adding a third request form alongside the current two. +- **Major (3.0.0)**: Changing the response ordering rule; removing `present`; lowering the entry cap; making `flightId` required; adding voting / trust filtering to the read path; renaming the `z`/`x`/`y` triple again. ## Test Cases @@ -152,9 +176,15 @@ Order invariant: `results[i]` corresponds to `request.tiles[i]` (or `request.loc |------|-------|----------|-------| | ordering-mixed-present-absent | 25 coords, 12 seeded + 13 absent, interleaved | 25 entries in request order; 12 present (id/capturedAt/source populated), 13 absent (only locationHash populated) | AC-1 | | most-recent-across-sources | Cell with `google_maps captured_at=T1` and `uav captured_at=T2 > T1`; coord request | `present=true`, `source='uav'`, `id` = UAV row's id | Inv-5 | -| validation-both-populated | Body with both `tiles` and `locationHashes` | HTTP 400 | Inv-1 | -| validation-neither-populated | Empty body or body with both fields empty | HTTP 400 | Inv-1 | -| validation-over-cap | 5001 entries | HTTP 400 | Inv-7 | +| validation-both-populated | Body with both `tiles` and `locationHashes` | HTTP 400 + ValidationProblemDetails | Inv-1 + AZ-796 | +| validation-neither-populated | Empty body or body with both fields empty | HTTP 400 + ValidationProblemDetails | Inv-1 + AZ-796 | +| validation-over-cap | 5001 entries | HTTP 400 + ValidationProblemDetails | Inv-7 + AZ-796 | +| validation-missing-z | `tiles: [{ x: 1, y: 1 }]` | HTTP 400 + `errors["tiles[0].z"]` populated | Inv-8 + AZ-796 | +| validation-out-of-range-z | `tiles: [{ z: 30, x: 1, y: 1 }]` | HTTP 400 + `errors["tiles[0].z"]` mentioning range | Inv-8 + AZ-796 | +| validation-out-of-range-x | `tiles: [{ z: 0, x: 5, y: 0 }]` (2^0 = 1) | HTTP 400 + `errors["tiles[0].x"]` populated | Inv-8 + AZ-796 | +| validation-unknown-root-field | Body with `unknownField: 42` plus `tiles: [...]` | HTTP 400 + `errors["unknownField"]` | Inv-9 + AZ-795 | +| validation-unknown-nested-field | `tiles: [{ z: 18, x: 1, y: 1, foo: 42 }]` | HTTP 400 + `errors["tiles[0].foo"]` | Inv-9 + AZ-795 | +| validation-old-field-name-tileZoom | `tiles: [{ tileZoom: 18, tileX: 1, tileY: 1 }]` (v1.x shape) | HTTP 400 + `errors["tiles[0].tileZoom"]` ("could not be mapped") | AZ-794 + Inv-9 | | auth-anonymous | No Bearer token | HTTP 401 | Standard `.RequireAuthorization()` baseline | | perf-2500-tiles | 2500-entry request against populated DB | p95 ≤ 1000 ms over 20 calls | AC-4 | | http2-multiplexing | 20 concurrent `GET /tiles/{z}/{x}/{y}` over a single H2 connection | All 20 responses `HttpResponseMessage.Version == 2.0`; ETag + Cache-Control preserved | AC-5; cross-references `tile-inventory.md` because Kestrel H2 is configured in the same PBI | @@ -163,4 +193,5 @@ Order invariant: `results[i]` corresponds to `request.tiles[i]` (or `request.loc | Version | Date | Change | Author | |---------|------|--------|--------| +| 2.0.0 | 2026-05-22 | **BREAKING**: per-entry coord triple renamed `tileZoom/tileX/tileY` → `z/x/y` (request `tiles[i]` and response `results[i]`) to align with the URL slippy-map convention already used by `GET /tiles/{z}/{x}/{y}`. AZ-794 ships as Option 1 hard switch; v1.x clients receive HTTP 400 with explicit "could not be mapped" errors. Adds Inv-8 (range constraints on z/x/y) + Inv-9 (unknown-field rejection); references `error-shape.md` v1.0.0 for the uniform 400 body shape. AZ-796 wires `InventoryRequestValidator` for Inv-1 / Inv-7 / Inv-8 enforcement. Cycle 7 — autodev Step 10. | autodev (Step 10, cycle 7) | | 1.0.0 | 2026-05-12 | Initial contract — `POST /api/satellite/tiles/inventory` with Form A (coords) / Form B (hashes) XOR validation, 5000-entry cap, most-recent-across-sources selection rule, ordering invariant. Produced by AZ-505. | autodev (Step 10, cycle 6) | diff --git a/_docs/02_document/deployment/containerization.md b/_docs/02_document/deployment/containerization.md index 51afdb3..3d30c05 100644 --- a/_docs/02_document/deployment/containerization.md +++ b/_docs/02_document/deployment/containerization.md @@ -11,7 +11,7 @@ | Service | Image | Ports (host:container) | Purpose | |---------|-------|------------------------|---------| -| postgres | postgres:16 | 5432:5432 | Database | +| postgres | postgres:16 | 5433:5432 | Database (host port 5433 chosen to avoid conflicts with sibling-project Postgres instances on dev laptops) | | api | Custom (Dockerfile) | 18980:8080, 18981:8081 | Application | ## Volumes diff --git a/_docs/02_document/module-layout.md b/_docs/02_document/module-layout.md index cd1a199..de04b9f 100644 --- a/_docs/02_document/module-layout.md +++ b/_docs/02_document/module-layout.md @@ -126,9 +126,12 @@ The cycle-1 (AZ-487) and cycle-2 (AZ-488) code reviews each surfaced an F1 (Low - `SatelliteProvider.Api/Authentication/AuthenticationServiceCollectionExtensions.cs` (added by AZ-487; `AddSatelliteJwt(IConfiguration)` registers `JwtBearer` with the suite-wide HS256 contract from `suite/_docs/10_auth.md`; validates `JWT_SECRET` ≥ 32 bytes at startup) - `SatelliteProvider.Api/Authentication/PermissionsRequirement.cs` + `PermissionsAuthorizationHandler` + `SatellitePermissions` (added by AZ-488; custom requirement that accepts a `permissions` claim shaped as either a single string or a JSON array; powers the `UavUploadPolicy` requiring the `GPS` permission) - `SatelliteProvider.Api/DTOs/UavTileBatchUploadRequest.cs` (added by AZ-488; multipart form binding envelope — kept in WebApi because it depends on `IFormFileCollection` + `[FromForm]`, both API-layer types) + - `SatelliteProvider.Api/Validators/ValidationEndpointFilter.cs` + `ValidationEndpointFilterExtensions.cs` (added by AZ-795; generic `IEndpointFilter` that runs the registered `IValidator` and returns `Results.ValidationProblem` on failure; opt-in via `RouteHandlerBuilder.WithValidation()`) + - `SatelliteProvider.Api/Validators/InventoryRequestValidator.cs` + `TileCoordValidator` (added by AZ-796; FluentValidation rules for `POST /api/satellite/tiles/inventory` — XOR `tiles`/`locationHashes`, per-array cap, slippy-map range checks) + - `SatelliteProvider.Api/Validators/GlobalValidatorConfig.cs` (added by AZ-795/AZ-796; idempotent `ApplyOnce()` configures `ValidatorOptions.Global.PropertyNameResolver` so `errors`-map keys are camelCase per `error-shape.md` Inv-4; called from `Program.cs` and from the test assembly's `ModuleInitializer`) - **Internal**: (none) - **Owns**: `SatelliteProvider.Api/**` -- **PackageReferences (added by AZ-487, bumped by AZ-496, then by AZ-500)**: `Microsoft.AspNetCore.Authentication.JwtBearer` 10.0.7 (pinned to the same minor patch as `Microsoft.AspNetCore.OpenApi` 10.0.7; AZ-496 bumped both packages from 8.0.21 → 8.0.25 in cycle 3 to close cycle-1 D1 + cycle-2 D3 supply-chain findings, then AZ-500 bumped both 8.0.25 → 10.0.7 in cycle 4 as part of the .NET 8 → .NET 10 migration; AZ-500 also bumped `Swashbuckle.AspNetCore` 6.6.2 → 10.1.7 here to land Microsoft.OpenApi 2.x compat required by ASP.NET Core 10). +- **PackageReferences (added by AZ-487, bumped by AZ-496, then by AZ-500; AZ-795 added FluentValidation)**: `Microsoft.AspNetCore.Authentication.JwtBearer` 10.0.7 (pinned to the same minor patch as `Microsoft.AspNetCore.OpenApi` 10.0.7; AZ-496 bumped both packages from 8.0.21 → 8.0.25 in cycle 3 to close cycle-1 D1 + cycle-2 D3 supply-chain findings, then AZ-500 bumped both 8.0.25 → 10.0.7 in cycle 4 as part of the .NET 8 → .NET 10 migration; AZ-500 also bumped `Swashbuckle.AspNetCore` 6.6.2 → 10.1.7 here to land Microsoft.OpenApi 2.x compat required by ASP.NET Core 10). `FluentValidation` + `FluentValidation.DependencyInjectionExtensions` 12.0.0 added by AZ-795 to back the strict-input-validation epic. - **Imports from**: Common (incl. AZ-488 UAV DTOs + `UavQualityConfig`), DataAccess, TileDownloader (incl. AZ-488 `IUavTileUploadHandler`), RegionProcessing, RouteManagement - **Consumed by**: (none — top-level entry point) diff --git a/_docs/02_tasks/_dependencies_table.md b/_docs/02_tasks/_dependencies_table.md index 84af760..1c10390 100644 --- a/_docs/02_tasks/_dependencies_table.md +++ b/_docs/02_tasks/_dependencies_table.md @@ -123,9 +123,9 @@ Adopted into satellite-provider cycle 7 with the recommended ordering: shared va | Task | Title | Depends On | Points | Status | |------|-------|-----------|--------|--------| -| AZ-794 | Inventory body fields: rename `tileZoom/tileX/tileY` → `z/x/y` (OSM convention) | — (coordinate release with AZ-795 / AZ-796) | 3 | To Do (cycle 7) | -| AZ-795 | Strict input validation across all public endpoints (FluentValidation + ProblemDetails) — **Epic with shared-infra ship** | — (children gated on shared infra landing first) | — (epic; shared-infra estimate 5–8 pts; per-endpoint children ~3 pts each) | To Do (cycle 7) | -| AZ-796 | Strict validation for inventory endpoint (POST /api/satellite/tiles/inventory) | AZ-795 (HARD — shared infra); coordinate with AZ-794 | 3 | To Do (cycle 7) | +| AZ-794 | Inventory body fields: rename `tileZoom/tileX/tileY` → `z/x/y` (OSM convention) | — (coordinate release with AZ-795 / AZ-796) | 3 | Done (cycle 7) | +| AZ-795 | Strict input validation across all public endpoints (FluentValidation + ProblemDetails) — **Epic with shared-infra ship** | — (children gated on shared infra landing first) | — (epic; shared-infra estimate 5–8 pts; per-endpoint children ~3 pts each) | Done — shared infra shipped (cycle 7); future per-endpoint child tasks open | +| AZ-796 | Strict validation for inventory endpoint (POST /api/satellite/tiles/inventory) | AZ-795 (HARD — shared infra); coordinate with AZ-794 | 3 | Done (cycle 7) | ## Execution Order diff --git a/_docs/02_tasks/done/AZ-794_inventory_field_rename_osm.md b/_docs/02_tasks/done/AZ-794_inventory_field_rename_osm.md new file mode 100644 index 0000000..3cdeff7 --- /dev/null +++ b/_docs/02_tasks/done/AZ-794_inventory_field_rename_osm.md @@ -0,0 +1,94 @@ +# Inventory API: rename body fields to OSM-style z/x/y + +**Task**: AZ-794_inventory_field_rename_osm +**Name**: Rename inventory body fields tileZoom/tileX/tileY → z/x/y (OSM convention) +**Description**: Align the `POST /api/satellite/tiles/inventory` body shape with the URL-path slippy-map convention (`{z}/{x}/{y}`). Same coordinate concept is named two different ways inside the same API today; this task makes the body match the URL. +**Complexity**: 3 points (recommended; final call by parent-suite team) +**Dependencies**: — (coordinate release window with AZ-795 / AZ-796 to bundle the breaking changes) +**Component**: SatelliteProvider.Api + SatelliteProvider.Common (DTOs) +**Tracker**: AZ-794 (https://denyspopov.atlassian.net/browse/AZ-794) +**Epic**: — (related to AZ-795 input-validation epic via Jira "Relates" link) +**Originating ticket**: gps-denied-onboard AZ-777 Phase 1 (Jetson probe, 2026-05-22) + +## Origin + +Surfaced during gps-denied-onboard AZ-777 Phase 1 Jetson probing of the parent-suite `satellite-provider` service. The reviewing engineer noted that the inventory endpoint uses two different naming conventions for the same coordinate concept: + +- URL: `GET /tiles/{z}/{x}/{y}` — OSM/XYZ-standard short names. +- Body: `{"tiles": [{"tileZoom":12,"tileX":2424,"tileY":1424}]}` — verbose .NET-style names. + +Conformance to the URL convention end-to-end (both URL and body) aligns with 20 years of slippy-map ecosystem norms (OSM, Google, Mapbox, Bing, Leaflet, MapLibre) and removes the need for consumers to translate at the boundary. Jira AZ-794 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Problem + +Same coordinate concept; two names; same API: +- URL path uses `z`, `x`, `y`. +- Request and response bodies use `tileZoom`, `tileX`, `tileY`. + +Every consumer that thinks in slippy-map vocabulary has to translate at the boundary. Wire size is also ~3× larger than necessary on the field names (a 900-tile inventory request carries ~5 KB of `tileZoom`/`tileX`/`tileY` vs ~1.7 KB of `z`/`x`/`y`). + +## Outcome + +- `POST /api/satellite/tiles/inventory` request body uses `{"tiles": [{"z":...,"x":...,"y":...}]}` per tile entry. +- Response body uses `{"results": [{"z":...,"x":...,"y":...,"locationHash":...,"present":...,...}]}` per entry. All non-coord fields (`locationHash`, `present`, `id`, `capturedAt`, `source`, `flightId`, `resolutionMPerPx`) unchanged. +- OpenAPI / Swagger spec updated to match. +- Schema doc `_docs/02_document/contracts/api/tile-inventory.md` bumped (v1.x.0 → next major) with a Migration / Coexistence section. +- Integration tests updated. +- Release notes / migration guide entry naming AZ-794 as the breaking-rename owner. + +## Scope + +### Included + +- DTOs for the inventory request + response — exact file path to be confirmed by the implementer (likely `SatelliteProvider.Common/DTO/TileInventory.cs` per AZ-505 spec). +- The MapPost handler / minimal-api endpoint registration in `SatelliteProvider.Api/Program.cs` (or wherever the inventory endpoint is wired today). +- `SatelliteProvider.IntegrationTests/TileInventoryTests.cs` — update payload builders + response assertions. +- `_docs/02_document/contracts/api/tile-inventory.md` — major version bump with Change Log entry naming this task. +- Release notes / migration guide. + +### Excluded + +- The `GET /tiles/{z}/{x}/{y}` endpoint — already uses the correct names; no change. +- Strict input validation — owned by AZ-796 (sibling under epic AZ-795). +- Other endpoint renames. +- Internal storage / query / repo-method names (`tile_zoom`, `tile_x`, `tile_y` DB columns) — wire-format change only. + +## Acceptance Criteria + +**AC-1: Request body uses short names** +Given a POST body `{"tiles":[{"z":12,"x":2424,"y":1424}]}` with valid JWT +When `POST /api/satellite/tiles/inventory` is called +Then HTTP 200 with `results[0].z == 12`, `results[0].x == 2424`, `results[0].y == 1424` and a deterministic non-zero `locationHash`. + +**AC-2: Response body uses short names** +Given a successful inventory call +When the response is parsed +Then every `results[i]` object contains `z`, `x`, `y` keys (not `tileZoom`, `tileX`, `tileY`). All other fields (`locationHash`, `present`, `id`, `capturedAt`, `source`, `flightId`, `resolutionMPerPx`) are unchanged byte-for-byte from the pre-rename contract. + +**AC-3: OpenAPI spec accuracy** +Given `/swagger/v1/swagger.json` (or equivalent) +When the InventoryRequest + InventoryEntry schemas are inspected +Then they declare `z`, `x`, `y` (not the old names) as the required coordinate properties. + +**AC-4: Migration guidance documented** +Given the rename ships +Then `_docs/02_document/contracts/api/tile-inventory.md` is bumped to a new major version, the Change Log entry names AZ-794 and the breaking-rename, and a Migration / Coexistence section either (a) names the hard-switch release with the consumer-side bump coordinated, or (b) documents the accept-both transition window. + +## Rollout — pick one + +- **Option 1 — hard switch** (recommended while consumer count is small): rename atomically, bump API version, ship coordinated consumer update in the same release. +- **Option 2 — accept-both transition**: server accepts both `z` and `tileZoom` on input for one release; response always uses short names; deprecation notice in release notes; remove long names in next release. + +## Constraints + +- **Breaking change** — coordinate with all known consumers before shipping. Known consumer at filing time: `gps-denied-onboard` `HttpTileDownloader` in `src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py`. Parent-suite team to enumerate the full consumer set before deciding rollout cadence. +- **No internal storage rename** — DB columns (`tile_zoom`, `tile_x`, `tile_y`) stay as named. Wire-format change only; internal Postgres schema is out of scope. +- **Coordinate with AZ-795 / AZ-796** — if validation strictness ships in the same release as the rename, the validators must use the new short names from day one. Recommended ordering: ship AZ-794 first, then AZ-795 shared infra, then AZ-796. + +## References + +- Jira AZ-794: https://denyspopov.atlassian.net/browse/AZ-794 +- Related: AZ-795 (validation epic), AZ-796 (inventory validation child) +- Originating discovery: gps-denied-onboard AZ-777 (Phase 1 Jetson probe, 2026-05-22) +- Current contract doc: `_docs/02_document/contracts/api/tile-inventory.md` v1.0.0 +- Known consumer side: `gps-denied-onboard/src/gps_denied_onboard/components/c11_tile_manager/tile_downloader.py` diff --git a/_docs/02_tasks/done/AZ-795_strict_validation_epic.md b/_docs/02_tasks/done/AZ-795_strict_validation_epic.md new file mode 100644 index 0000000..340dd82 --- /dev/null +++ b/_docs/02_tasks/done/AZ-795_strict_validation_epic.md @@ -0,0 +1,153 @@ +# Strict input validation across all public endpoints (FluentValidation + ProblemDetails) + +**Task**: AZ-795_strict_validation_epic +**Name**: Strict input validation across all public endpoints +**Type**: Epic +**Description**: Every public HTTP endpoint must reject malformed input with structured 4xx errors instead of silently coercing missing fields to zero / ignoring unknown fields. Recommended approach: FluentValidation + global ProblemDetails filter + `JsonSerializerOptions.UnmappedMemberHandling.Disallow`. +**Complexity**: — (epic; rolls up children. Estimate: 5–8 pts shared infra + ~3 pts per per-endpoint child) +**Dependencies**: — (per-endpoint children depend on shared infra landing first) +**Component**: SatelliteProvider.Api (DI wiring + global filter + DTOs + validators) +**Tracker**: AZ-795 (https://denyspopov.atlassian.net/browse/AZ-795) +**Children**: AZ-796 (inventory endpoint — first concrete child); sibling per-endpoint tasks to be added by parent-suite team +**Originating ticket**: gps-denied-onboard AZ-777 Phase 1 (Jetson probe, 2026-05-22) + +## Origin + +Discovered during gps-denied-onboard AZ-777 Phase 1 Jetson probing on 2026-05-22. A hand-typed inventory request with the wrong field names (`{"z","x","y"}` instead of the current `{"tileZoom","tileX","tileY"}`) returned **HTTP 200** with `(0,0,0)` coordinates and an identical `locationHash` for every entry. Real client bugs masquerade as valid results because the deserializer silently treats unknown fields as missing and missing fields as `default(int) = 0`. + +For a service that's the single source of truth about which satellite tiles exist, permissive parsing is actively dangerous: corruption downstream, confident wrong answers, hours of debugging on the consumer side. + +Jira AZ-795 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Problem + +Every public-facing JSON endpoint on satellite-provider inherits the same Postel-permissive parsing default: +- Missing required fields → silently `default(T)` (e.g. `0` for `int`). +- Unknown fields → silently dropped (no `[JsonExtensionData]` capture, no log entry). +- Wrong types → silently coerced where possible, silently dropped where not. + +No structured error response. The only contract-level signal a misbehaving client gets today is downstream weirdness (wrong `locationHash`, repeated identical results, etc.) — many hops away from the actual cause. + +## Outcome + +- Every public-facing JSON endpoint rejects malformed input with **HTTP 400 + RFC 7807 ProblemDetails** body naming the offending field(s). +- Validators are testable in isolation (unit tests per `RuleFor`) and enforced by the HTTP layer without per-controller try/catch boilerplate. +- Unknown-field rejection is wired at the deserializer level so typos can't reach a validator. +- Uniform error response shape across all endpoints. +- New `_docs/02_document/contracts/api/error-shape.md` v1.0.0 documenting the ProblemDetails contract every endpoint conforms to. + +## Recommended approach + +1. **FluentValidation** for input DTOs (declarative, composable, validators are testable units). Final stack choice belongs to the parent-suite team; if FluentValidation is ruled out by existing constraints, alternatives are stock DataAnnotations + custom model binders or hand-written `IValidator`. +2. **Global error filter / ASP.NET model-state behavior** that emits RFC 7807 ProblemDetails for every validation failure. No per-endpoint try/catch boilerplate. +3. **Unknown-field rejection** at the deserializer: `JsonSerializerOptions.UnmappedMemberHandling.Disallow` (.NET 8+) or `Newtonsoft.Json` `MissingMemberHandling.Error`. Catches typos like `{"Z":12}` (uppercase) that no validator can catch after deserialization. + +## Error response contract (uniform across all endpoints) + +```json +{ + "type": "https://tools.ietf.org/html/rfc7231#section-6.5.1", + "title": "One or more validation errors occurred.", + "status": 400, + "errors": { + "tiles[0].z": ["The z field is required."], + "tiles[1]": ["Unexpected field: 'tileZoom'."] + } +} +``` + +Stable enough for consumers to pattern-match. Field names in `errors` paths must use the same casing as the request body (post-AZ-794 short names for the inventory endpoint). + +## Scope + +### Included — Shared infrastructure (this epic owns) + +- DI wiring for FluentValidation (or chosen alternative) in `SatelliteProvider.Api/Program.cs` (or appropriate composition root). +- Global error filter / `Configure(...)` for ProblemDetails formatting. +- `JsonSerializerOptions` configuration for unknown-field rejection. +- A validator-coverage table in `_docs/02_document/architecture.md` (or equivalent) listing each public endpoint and its validator class. +- Shared test fixtures for ProblemDetails assertions in `SatelliteProvider.IntegrationTests`. +- New contract artifact: `_docs/02_document/contracts/api/error-shape.md` v1.0.0 (the ProblemDetails shape every endpoint conforms to). + +### Included — Per-endpoint child tasks + +- One child Task per public-facing endpoint that has a JSON body. Each consumes the shared infra. +- Each child uses the AC template below. +- Parent-suite team enumerates the full endpoint surface from `/swagger/v1/swagger.json` / route map and creates the children. +- First child (concrete reference implementation): **AZ-796** — inventory endpoint. + +### Excluded + +- Authentication / authorization changes (JWT contract owned by AZ-494). +- Endpoint renaming (**AZ-794** owns the inventory body-field rename). +- Rate-limiting / quota (separate concern). +- Internal-only admin endpoints, health probes, metrics scrapers (parent-suite team owns in/out decision per endpoint). + +## Acceptance Criteria template (every child task must satisfy) + +**AC-1: Missing required field → 400** +Given a POST body that omits a required field +When the endpoint is called +Then HTTP 400 with `errors.` listing the missing field. + +**AC-2: Unknown field → 400** +Given a POST body with an unrecognized field at root or in nested objects +When the endpoint is called +Then HTTP 400 with `errors[].` naming the unexpected field. + +**AC-3: Wrong type → 400** +Given a POST body with a field of unexpected JSON type (e.g. string where integer expected) +When the endpoint is called +Then HTTP 400 with `errors.` describing the type mismatch. + +**AC-4: Out-of-range value → 400** +Given a POST body with a value outside its supported range +When the endpoint is called +Then HTTP 400 with `errors.` describing the valid range. + +**AC-5: Empty array where non-empty required → 400** +Given a POST body where a required non-empty collection is empty +When the endpoint is called +Then HTTP 400 with `errors.` describing the constraint. + +**AC-6: Validator class is its own file + unit-tested** +A `IValidator` (or equivalent) class exists in its own file under `SatelliteProvider.Api/Validators/` (or per-suite convention), with a unit test per `RuleFor(...)`. + +**AC-7: Integration tests cover one happy + one failure path per AC** +`SatelliteProvider.IntegrationTests` adds a fixture that POSTs each bad-payload variant and asserts `status == 400` + ProblemDetails shape + specific `errors[].` path. + +**AC-8: OpenAPI / Swagger spec accuracy** +`/swagger/v1/swagger.json` marks required fields, declares ranges, and documents the new 400 response shape. + +## Test requirements + +- **Unit**: one xUnit class per validator. Tests cover each `RuleFor(...)` / equivalent. +- **Integration**: `SatelliteProvider.IntegrationTests` adds one fixture per endpoint covering all AC variants (~7–10 new tests per endpoint). +- **Contract**: OpenAPI spec snapshot test confirms the published schema rejects what the validator rejects. +- **Cross-cutting**: shared `ProblemDetailsAssertions` helper in test infra so every endpoint's failure tests use the same assertion shape. + +## Migration / breaking-change strategy + +Tightening validation is a **breaking behavior change**: clients that today get 200 OK with nonsense will start getting 400. Three approaches the parent-suite team can pick from: + +1. **Hard switch** — ship in one release with a clear "Breaking" note. Cleanest for low-consumer-count (currently 1). +2. **Soft warning then enforce** — log warnings for one release when malformed input arrives; enforce in the next. +3. **API versioning** — keep `/v1` permissive, add `/v2` strict, migrate consumers, remove `/v1`. + +Recommendation: **#1** while the consumer set is small (currently 1 known: `gps-denied-onboard`). + +## Constraints + +- Shared infra MUST land before any per-endpoint child task — children are gated on it. +- Coordinate with **AZ-794** (inventory rename) — recommended ordering ships AZ-794 first so this epic's validators use the final names from day one. +- Parent-suite team enumerates the full consumer set before deciding rollout cadence (not just `gps-denied-onboard`). +- Per-endpoint child tasks added by parent-suite team after enumerating endpoint surface from OpenAPI / route map. Do NOT create all children up-front — let them be added as the team decomposes. + +## References + +- Jira AZ-795: https://denyspopov.atlassian.net/browse/AZ-795 +- First child: AZ-796 (inventory endpoint) +- Related: AZ-794 (inventory rename), AZ-777 (originating discovery in gps-denied-onboard) +- Originating discovery: gps-denied-onboard AZ-777 Phase 1 Jetson probe (2026-05-22) +- ASP.NET ProblemDetails reference: https://learn.microsoft.com/en-us/aspnet/core/web-api/handle-errors +- FluentValidation reference: https://docs.fluentvalidation.net/ diff --git a/_docs/02_tasks/done/AZ-796_inventory_endpoint_validation.md b/_docs/02_tasks/done/AZ-796_inventory_endpoint_validation.md new file mode 100644 index 0000000..ef716a8 --- /dev/null +++ b/_docs/02_tasks/done/AZ-796_inventory_endpoint_validation.md @@ -0,0 +1,124 @@ +# Strict validation for inventory endpoint (POST /api/satellite/tiles/inventory) + +**Task**: AZ-796_inventory_endpoint_validation +**Name**: Strict validation for inventory endpoint +**Description**: Add FluentValidation-backed strict input validation to `POST /api/satellite/tiles/inventory`. Reject malformed payloads with RFC 7807 ProblemDetails (HTTP 400). First concrete child of the validation-hardening epic (AZ-795); serves as reference implementation pattern for sibling per-endpoint tasks. +**Complexity**: 3 points (recommended) +**Dependencies**: AZ-795 (HARD — shared FluentValidation + ProblemDetails + unknown-field-rejection infra must land first); coordinate with AZ-794 (rename) +**Component**: SatelliteProvider.Api/Validators + SatelliteProvider.Common (DTOs) +**Tracker**: AZ-796 (https://denyspopov.atlassian.net/browse/AZ-796) +**Epic**: AZ-795 — Strict input validation across all public endpoints +**Originating ticket**: gps-denied-onboard AZ-777 Phase 1 (Jetson probe, 2026-05-22) + +## Origin + +Discovered during gps-denied-onboard AZ-777 Phase 1 Jetson probing on 2026-05-22 — see parent epic AZ-795 for full context. This ticket scopes the strict-validation work to the **inventory endpoint** as the first concrete reference implementation; sibling per-endpoint child tasks will follow the same pattern. + +Jira AZ-796 is the authoritative spec; this file mirrors the in-workspace-only sections that the satellite-provider implementer will need. + +## Problem + +`POST /api/satellite/tiles/inventory` today accepts malformed payloads silently: +- Missing required fields (`z`, `x`, `y`) → silently coerced to `0`, producing `locationHash` collisions and `(0,0,0)` echoed back as if the client had asked for tile (0,0,0). +- Unknown fields (typos like `{"Z":12}` uppercase) → silently dropped, then required field appears missing → silently 0. +- Wrong types → silently coerced where possible. +- No structured 4xx response. Real client bugs surface downstream as "all my inventory results have the same locationHash" — many hops from the actual cause. + +Concrete reproducer (from the originating probe): + +```bash +curl -sk -H "Authorization: Bearer $JWT" -H "Content-Type: application/json" \ + -d '{"tiles":[{"z":12,"x":2424,"y":1424},{"z":12,"x":2425,"y":1425}]}' \ + https://satellite-provider:8080/api/satellite/tiles/inventory +``` + +Returns HTTP 200 with both `results` entries carrying `tileZoom:0, tileX:0, tileY:0` and identical `locationHash`. Expected: HTTP 400 naming `z`, `x`, `y` as unexpected fields (pre-AZ-794) or 200 with correct echo (post-AZ-794) — but never silently-wrong 200. + +## Outcome + +- `POST /api/satellite/tiles/inventory` rejects malformed payloads with HTTP 400 + RFC 7807 ProblemDetails matching the shape defined by the parent epic AZ-795. +- An `IValidator` (or equivalent) covers all 9 validation rules listed below. +- Integration tests cover one happy path + one failure path per validation rule. +- OpenAPI spec marks required fields, declares ranges, and documents the new 400 response. +- Schema doc `_docs/02_document/contracts/api/tile-inventory.md` updated to reference the validation rules + error contract. + +## Scope + +### Included + +- `InventoryRequestValidator` (or equivalent class) in `SatelliteProvider.Api/Validators/` — full coverage of the 9 validation rules below. +- Wiring of the validator into the MapPost / minimal-api endpoint registration in `SatelliteProvider.Api/Program.cs` (or wherever the inventory endpoint is wired today). Wiring leverages the shared infra from AZ-795. +- Unit tests for the validator (`SatelliteProvider.UnitTests` or appropriate — one test method per `RuleFor(...)`). +- Integration tests in `SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs` (new file) — happy + failure case per AC. +- Update to `_docs/02_document/contracts/api/tile-inventory.md` documenting the validation rules + error shape. +- Update to `/swagger/v1/swagger.json` (via XML doc comments / Swashbuckle annotations) marking required fields + ranges + 400 response. + +### Excluded + +- Shared infra wiring (parent epic AZ-795 owns this). +- Validation for other endpoints (sibling child tasks under AZ-795 will be added by parent-suite team). +- The field rename itself (AZ-794). +- Auth / JWT changes. +- Performance considerations (existing AZ-505 perf gates remain in effect; validation overhead expected to be negligible vs DB round-trip). + +## Required validations (9 rules) + +Naming below assumes AZ-794 (rename) has shipped. If validators land BEFORE AZ-794, swap `z/x/y` for `tileZoom/tileX/tileY` and re-rename when AZ-794 lands. + +1. **Body present** — null/empty body → 400. +2. **`tiles` field required** — missing → 400 with `errors.tiles: ["required"]`. +3. **`tiles` non-empty** — empty array → 400 with `errors.tiles: ["must contain at least 1 entry"]`. +4. **`tiles` max size** — to be confirmed with parent-suite (existing AZ-505 spec uses 5000 for the EITHER/OR body shape; reaffirm here or align). Over the cap → 400. +5. **Each entry has `z`, `x`, `y`** — any missing → 400 with `errors.tiles[i].: ["required"]`. +6. **Each field is non-negative integer** — wrong type or negative → 400. +7. **`z` within supported zoom range** — out of range → 400 with `errors.tiles[i].z: ["must be between {min} and {max}"]`. Range to be confirmed with parent-suite (existing AZ-484 / AZ-503 schemas suggest 0–22; reaffirm here). +8. **`x` / `y` within tile-axis bounds for given `z`** — i.e. `0 <= x,y < 2^z` — out of range → 400 with `errors.tiles[i].x` or `.y`. +9. **Unknown fields at root or in tile entries** — 400 with `errors[].: ["unexpected field: ''"]`. Requires `JsonSerializerOptions.UnmappedMemberHandling.Disallow` (.NET 8+) at the deserializer level — this is part of AZ-795 shared infra and must be wired first. + +## Acceptance Criteria + +**AC-1: Each of the 9 validations rejects with HTTP 400 + ProblemDetails** +Given a POST body that violates exactly ONE validation rule (one failure case per rule) +When `POST /api/satellite/tiles/inventory` is called with valid JWT +Then HTTP 400; response body matches the parent epic's ProblemDetails shape; `errors[].` names the specific failing field; `errors[]` array does NOT include unrelated rules (single-rule precision). + +**AC-2: Happy path unchanged** +Given a POST body that satisfies all 9 validations +When `POST /api/satellite/tiles/inventory` is called with valid JWT +Then HTTP 200 with the existing result shape (one entry per requested tile, same ordering, fields preserved from the pre-validation contract). No regression in existing `TileInventoryTests.cs` happy-path assertions. + +**AC-3: Validator class is its own file + unit-tested** +A `InventoryRequestValidator` (or equivalent) class exists in its own file under `SatelliteProvider.Api/Validators/` (or per-suite convention). xUnit test class has one test method per `RuleFor(...)` — i.e. ≥ 9 unit-test methods. + +**AC-4: Integration tests cover happy + failure per rule** +`SatelliteProvider.IntegrationTests/TileInventoryValidationTests.cs` (new file) has ≥ 10 test methods: 1 happy path + 9 failure cases. Each failure case POSTs the malformed payload, asserts `status == 400`, asserts ProblemDetails shape, asserts the specific `errors[].` matches the rule. + +**AC-5: OpenAPI spec accuracy** +Given `/swagger/v1/swagger.json` (or equivalent) +When the InventoryRequest schema + endpoint operation are inspected +Then required fields are marked `required: true`, integer types are declared with `minimum`/`maximum` per the validation rules, the endpoint declares a 400 response with the ProblemDetails schema. + +**AC-6: Schema doc updated** +`_docs/02_document/contracts/api/tile-inventory.md` is updated (Change Log entry naming AZ-796) to document the validation rules + error contract. No version bump required (additive — error shape is a previously-undefined contract; clients that send valid payloads see no change). + +**AC-7: Manual probe captures each failure mode end-to-end** +A `scripts/probe_inventory_validation.sh` (or Postman / Bruno collection) is committed that exercises each failure mode via real `curl` with a JWT, capturing the actual response body for documentation/regression. + +## Coordination with sibling tickets + +- **Parent (AZ-795)**: shared FluentValidation + ProblemDetails + unknown-field-rejection infra must land first. +- **AZ-794 (inventory rename)**: if it ships first, validators use `z/x/y` from day one. If it ships in the same release, coordinate field names so this ticket lands once with the final names. If it ships later, validators initially use `tileZoom/tileX/tileY` and get renamed at AZ-794 ship time — less ideal but acceptable. + +## Constraints + +- **Breaking behavior change** — clients that today get 200 with nonsense will start getting 400. Coordinate rollout with all known consumers per AZ-795's migration strategy section. +- **No regression in existing `TileInventoryTests.cs`** happy-path assertions (AZ-505 AC coverage). +- **No change to internal repository / DB query path** — validation lives at the API layer only. + +## References + +- Jira AZ-796: https://denyspopov.atlassian.net/browse/AZ-796 +- Parent epic: AZ-795 (shared infra; error-shape contract) +- Related: AZ-794 (rename), AZ-505 (existing inventory endpoint spec) +- Originating discovery: gps-denied-onboard AZ-777 Phase 1 Jetson probe (2026-05-22) +- Current contract doc: `_docs/02_document/contracts/api/tile-inventory.md` v1.0.0 diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index 0591181..f95b53b 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -6,9 +6,9 @@ step: 10 name: Implement status: in_progress sub_step: - phase: 0 - name: awaiting-invocation - detail: "" + phase: 12 + name: tracker-in-testing + detail: "batch 1 of 1; AZ-794 + AZ-795 + AZ-796 implementation complete; full Docker Compose suite green (311 unit tests + integration tests including 16 new inventory-validation cases); task files archived todo/ -> done/; ready to commit + push and transition Jira tickets to In Testing" retry_count: 0 cycle: 7 tracker: jira diff --git a/docker-compose.yml b/docker-compose.yml index 610e6c3..bc1284c 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -7,7 +7,7 @@ services: POSTGRES_PASSWORD: postgres POSTGRES_DB: satelliteprovider ports: - - "5432:5432" + - "5433:5432" volumes: - postgres_data:/var/lib/postgresql/data healthcheck: diff --git a/scripts/probe_inventory_validation.sh b/scripts/probe_inventory_validation.sh new file mode 100755 index 0000000..d894c8a --- /dev/null +++ b/scripts/probe_inventory_validation.sh @@ -0,0 +1,58 @@ +#!/usr/bin/env bash +set -euo pipefail + +# Manual end-to-end probe for the inventory endpoint's strict validation gate. +# Each call below should return HTTP 400 with an `application/problem+json` +# body whose `errors` map names the offending field path. Exactly one call +# (the happy path) should return HTTP 200. +# +# Usage: +# API_URL=https://localhost:8080 JWT="" ./scripts/probe_inventory_validation.sh +# (defaults to https://localhost:8080 with a JWT minted via PerfBootstrap --mint-only) + +API_URL="${API_URL:-https://localhost:8080}" +JWT="${JWT:-}" +PATH_INV="${API_URL%/}/api/satellite/tiles/inventory" + +if [[ -z "${JWT}" ]]; then + echo "ERROR: set JWT env var to a bearer token. Mint one via:" + echo " dotnet run --project SatelliteProvider.IntegrationTests -- --mint-only" + exit 2 +fi + +curl_args=(-sS -k -H "Authorization: Bearer ${JWT}" -H "Content-Type: application/json" -X POST "${PATH_INV}") + +probe() { + local label="$1" + local body="$2" + local expected_status="$3" + + echo "----- ${label} (expecting HTTP ${expected_status}) -----" + local response + response=$(curl "${curl_args[@]}" -d "${body}" -w "\nHTTP_STATUS=%{http_code}\n") + echo "${response}" + local actual_status + actual_status=$(echo "${response}" | tail -n 1 | sed 's/HTTP_STATUS=//') + if [[ "${actual_status}" != "${expected_status}" ]]; then + echo "FAIL: expected HTTP ${expected_status}, got ${actual_status}" + return 1 + fi + echo "OK: HTTP ${expected_status}" + echo +} + +probe "happy-path" '{"tiles":[{"z":18,"x":1,"y":1}]}' 200 +probe "missing-z" '{"tiles":[{"x":1,"y":1}]}' 400 +probe "missing-x-and-y" '{"tiles":[{"z":18}]}' 400 +probe "zoom-out-of-range" '{"tiles":[{"z":30,"x":0,"y":0}]}' 400 +probe "x-beyond-zoom-bounds" '{"tiles":[{"z":2,"x":4,"y":0}]}' 400 +probe "y-beyond-zoom-bounds" '{"tiles":[{"z":0,"x":0,"y":1}]}' 400 +probe "negative-x" '{"tiles":[{"z":18,"x":-1,"y":0}]}' 400 +probe "unknown-root-field" '{"unknownField":42,"tiles":[{"z":18,"x":1,"y":1}]}' 400 +probe "unknown-nested-field" '{"tiles":[{"z":18,"x":1,"y":1,"foo":42}]}' 400 +probe "legacy-v1-field-name" '{"tiles":[{"tileZoom":18,"tileX":1,"tileY":1}]}' 400 +probe "type-mismatch-string" '{"tiles":[{"z":"eighteen","x":1,"y":1}]}' 400 +probe "both-populated-xor" '{"tiles":[{"z":18,"x":1,"y":1}],"locationHashes":["00000000-0000-0000-0000-000000000000"]}' 400 +probe "neither-populated-xor" '{}' 400 + +echo "All probes passed."