mirror of https://github.com/azaion/satellite-provider.git synced 2026-06-21 10:11:13 +00:00

Files

T

Oleksandr Bezdieniezhnykh fcd494f67e [AZ-812] Region API: rename Latitude/Longitude → Lat/Lon (OSM convention)

Mirror of AZ-794 (inventory z/x/y rename). RequestRegionRequest.cs renames C#
props Latitude→Lat / Longitude→Lon and adds [JsonPropertyName("lat"/"lon")] so
the wire format is unambiguous under the AZ-795 strict-parsing stack
(UnmappedMemberHandling.Disallow → legacy {"latitude":..,"longitude":..} now
returns HTTP 400 instead of silently coercing).

Updates all in-repo consumers: API handler (Program.cs), integration tests
(Models.cs, RegionTests.cs, IdempotentPostTests.cs, SecurityTests.cs), the
performance harness (run-performance-tests.sh PT-03/04/05/07), and module
docs (common_dtos.md, api_program.md; system-flows.md F2 already used
lat/lon). New RegionFieldRenameTests.cs covers AC-4 both directions (new
format → 200, legacy format → 400). Smoke green; no regressions.

region-request.md contract doc not bumped here — AZ-808 publishes v1.0.0
directly with the post-rename names per AZ-812 coordination clause.

Batch 01 of cycle 8. PASS_WITH_WARNINGS (one Low DRY finding for follow-up
test-helper consolidation; details in
_docs/03_implementation/reviews/batch_01_cycle8_review.md).

Co-authored-by: Cursor <cursoragent@cursor.com>

2026-05-22 15:54:53 +03:00

4.4 KiB

Raw Blame History

Code Review Report

Batch: 01 (cycle 8) Tasks: AZ-812 (Region API field rename Latitude/Longitude → Lat/Lon, OSM convention) Date: 2026-05-22 Verdict: PASS_WITH_WARNINGS

Findings

#	Severity	Category	File:Line	Title
1	Low	Maintainability	`SatelliteProvider.IntegrationTests/RegionFieldRenameTests.cs:79`	`AssertErrorsContainsMention` copy-pasted from `TileInventoryValidationTests.cs`

Finding Details

F1: AssertErrorsContainsMention copy-pasted from TileInventoryValidationTests.cs (Low / Maintainability)

Location: SatelliteProvider.IntegrationTests/RegionFieldRenameTests.cs:79-110 (and the prior copy at TileInventoryValidationTests.cs:396-428)
Description: The new RegionFieldRenameTests.cs copies the private AssertErrorsContainsMention helper verbatim from TileInventoryValidationTests.cs. Both helpers walk the same errors object and assert a mention exists in either keys or messages.
Suggestion: Promote the helper to ProblemDetailsAssertions.cs (the natural home for cross-test ProblemDetails assertions) and migrate both tests in a small follow-up cleanup. NOT done in this batch — touching the cycle-7 file is out of AZ-812 scope. Tracked as future hygiene, not a regression.
Task: AZ-812

Phase Summary

Phase	Outcome
1. Context Loading	Read AZ-812 spec + `_docs/02_document/module-layout.md`. Scope is the Region POST DTO rename.
2. Spec Compliance	AC-1 ✓ (DTO has Lat/Lon + JsonPropertyName), AC-2 ✓ (wire format end-to-end with `lat`/`lon`), AC-3 ✓ (`RegionTests.cs` happy-path updated), AC-4 ✓ (`RegionFieldRenameTests` validates both directions, smoke green), AC-5 ✓ (common_dtos.md, api_program.md, system-flows.md updated/verified), AC-6 ✓ (`region-request.md` does not yet exist — AZ-808 will publish v1.0.0 directly with new names per spec coordination). No Spec-Gap.
3. Code Quality	Mechanical DTO rename, clean. One DRY violation (F1) — Low severity.
4. Security	No SQL injection, no hardcoded secrets, no sensitive data in logs. New test uses GUID + test coordinates only.
5. Performance	No perf impact (field rename). No N+1, no blocking I/O.
6. Cross-Task Consistency	Single-task batch — N/A.
7. Architecture Compliance	DTO in `SatelliteProvider.Common/DTO/` (Common layer, importable by all). Test in `SatelliteProvider.IntegrationTests/` (test layer). No layering violations, no cycles, no Public-API bypasses, no ADR violations.

Files Reviewed

SatelliteProvider.Common/DTO/RequestRegionRequest.cs (DTO rename + [JsonPropertyName] attrs)
SatelliteProvider.Api/Program.cs (handler property access updated)
SatelliteProvider.IntegrationTests/Models.cs (test-side DTO mirror updated)
SatelliteProvider.IntegrationTests/RegionTests.cs (happy-path uses new property names)
SatelliteProvider.IntegrationTests/IdempotentPostTests.cs (JSON payload lat/lon)
SatelliteProvider.IntegrationTests/SecurityTests.cs (JSON payload lat/lon)
SatelliteProvider.IntegrationTests/RegionFieldRenameTests.cs (new — AZ-812 AC-4 coverage)
SatelliteProvider.IntegrationTests/Program.cs (smoke + full suite wired to call RegionFieldRenameTests.RunAll)
scripts/run-performance-tests.sh (PT-03/04/05/07 JSON bodies updated to lat/lon)
_docs/02_document/modules/common_dtos.md (RequestRegionRequest section added; RegionRequest disambiguated as internal queue type)
_docs/02_document/modules/api_program.md (RequestRegionRequest moved from Local Records to Common/DTO section)

Test Evidence

./scripts/run-tests.sh --smoke (integration-tests container exit code 0):

Test: Region endpoint OSM field-name rename (AZ-812)
====================================================

AZ-812 AC-4 (positive): new {lat,lon} wire format → HTTP 200
  ✓ {lat,lon} body accepted with HTTP 200

AZ-812 AC-4 (negative): legacy {latitude,longitude} wire format → HTTP 400 (UnmappedMemberHandling.Disallow)
  ✓ Legacy {latitude,longitude} body rejected with HTTP 400; errors map names the unknown field
✓ Region field-rename tests: PASSED

No regressions: cycle-7 inventory validation suite, idempotent POST, security, route, tile, leaflet path, and migrations 012/013/014 all green in the same smoke run.

Verdict Logic

No Critical, no High, no Medium findings.
1 Low finding (DRY in test helpers) — does not block.
PASS_WITH_WARNINGS.

4.4 KiB Raw Blame History