From 7736cfd761cbf2b13e5c3b77d92426bbef30dd62 Mon Sep 17 00:00:00 2001 From: Oleksandr Bezdieniezhnykh Date: Mon, 11 May 2026 04:11:57 +0300 Subject: [PATCH] [AZ-374] Add batch 21 report; autodev state batch 21 complete Co-authored-by: Cursor --- _docs/03_implementation/batch_21_report.md | 96 ++++++++++++++++++++++ _docs/_autodev_state.md | 2 +- 2 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 _docs/03_implementation/batch_21_report.md diff --git a/_docs/03_implementation/batch_21_report.md b/_docs/03_implementation/batch_21_report.md new file mode 100644 index 0000000..c9139fc --- /dev/null +++ b/_docs/03_implementation/batch_21_report.md @@ -0,0 +1,96 @@ +# Batch Report + +**Batch**: 21 +**Tasks**: AZ-374 (C21 — typed/named `HttpClient` for Google Maps in DI) +**Date**: 2026-05-11 +**Run**: `03-code-quality-refactoring` +**Cycle**: 1 + +## Task Results + +| Task | Status | Files Modified | Tests | AC Coverage | Issues | +|------|--------|----------------|-------|-------------|--------| +| AZ-374_refactor_typed_httpclient_googlemaps | Done | 3 src/test = 3 files | 175 unit + 5 smoke pass | 3/3 covered | None | + +## Design note: registration location + +The task spec recommended adding `AddHttpClient("GoogleMapsTiles", ...)` in `Program.cs`. I placed it inside `TileDownloaderServiceCollectionExtensions.AddTileDownloader()` instead. Rationale (per `coderule.mdc` SRP guidance): + +- The named-client name, User-Agent, and timeout are all TileDownloader-internal concerns. Their owner is the component that resolves them — `GoogleMapsDownloaderV2`. The DI extension already encapsulates everything else TileDownloader needs (memory cache, `ISatelliteDownloader`, `ITileService`). +- Placing this in `Program.cs` would have spread component-internal knowledge into the application entry-point file. The extension was created precisely to avoid that. +- The bare `builder.Services.AddHttpClient()` call in `Program.cs:36` was kept untouched — it is now structurally redundant (the named-client registration also wires up `IHttpClientFactory`), but removing it is outside this task's scope. + +The named-client name itself (`"GoogleMapsTiles"`), the `User-Agent` header value, and the 100-second default timeout are now `internal const`s on `GoogleMapsDownloaderV2` so the extension method and the downloader resolve from the same source of truth. + +## Changes + +### Modified production code + +- `SatelliteProvider.Services.TileDownloader/GoogleMapsDownloaderV2.cs` + - Promoted `private const USER_AGENT` → `internal const UserAgent` (PascalCase; visibility to allow the extension method in the same assembly to reference it). + - Added `internal const string GoogleMapsTilesClientName = "GoogleMapsTiles";`. + - Added `internal const int DefaultHttpClientTimeoutSeconds = 100;` (matches `HttpClient`'s implicit default; preserves prior behavior). + - Three `CreateClient()` call sites updated to `CreateClient(GoogleMapsTilesClientName)`: + - `GetSessionToken` (session-token POST) + - `DownloadSingleTileAsync` retry lambda (single-tile GET) + - `PerformDownloadAsync` retry lambda (batch-tile GET) + - Removed the two per-call `httpClient.DefaultRequestHeaders.UserAgent.ParseAdd(USER_AGENT);` lines — the header is now set once at registration. +- `SatelliteProvider.Services.TileDownloader/TileDownloaderServiceCollectionExtensions.cs` + - Added `services.AddHttpClient(GoogleMapsDownloaderV2.GoogleMapsTilesClientName, c => { c.DefaultRequestHeaders.UserAgent.ParseAdd(GoogleMapsDownloaderV2.UserAgent); c.Timeout = TimeSpan.FromSeconds(GoogleMapsDownloaderV2.DefaultHttpClientTimeoutSeconds); });`. + +### Unchanged (deliberate) + +- `SatelliteProvider.Api/Program.cs` — `builder.Services.AddHttpClient();` kept. See "Design note" above; cleanup deferred. +- Per-call `User-Agent` setup is removed; the named client carries the header on every `CreateClient(...)` resolution. + +### Modified tests + +- `SatelliteProvider.Tests/TileServiceTests.cs` + - Added `Microsoft.Extensions.DependencyInjection` using. + - New `TileDownloaderRegistrationTests.AddTileDownloader_RegistersNamedGoogleMapsTilesHttpClient_AZ374_AC1` — builds a `ServiceCollection`, calls `AddTileDownloader()`, resolves `IHttpClientFactory`, asks for `"GoogleMapsTiles"`, asserts the returned client carries a `Mozilla/5.0...`-style User-Agent and a 100-second `Timeout`. Covers AC-1 directly and AC-2 transitively (the only consumer of `CreateClient(GoogleMapsTilesClientName)` is `GoogleMapsDownloaderV2`). + +## AC Test Coverage + +| AC | Covered by | +|----|------------| +| AC-1 (single registration with User-Agent + Timeout) | `AddTileDownloader_RegistersNamedGoogleMapsTilesHttpClient_AZ374_AC1` | +| AC-2 (downloader uses `CreateClient("GoogleMapsTiles")` everywhere) | Source-level: the only `CreateClient(` occurrences in `GoogleMapsDownloaderV2.cs` reference `GoogleMapsTilesClientName`. Behavioral: smoke tests `GetTileByLatLon` + region/route tile downloads succeed end-to-end against the real Google Maps API, exercising all three call sites (session token, single-tile, batch-tile). | +| AC-3 (tests stay green) | `scripts/run-tests.sh --smoke` reports 175 unit + 5 smoke. | + +## Test Run + +| Suite | Result | Count | +|-------|--------|-------| +| Unit (`SatelliteProvider.Tests`) | All passed | 175 (was 174; +1 new test for the DI registration) | +| Smoke integration (Docker) | All passed | 5 scenarios | + +## Code Review Verdict: PASS + +Inline review covered: + +- **Architecture**: Named-client registration co-located with the consumer in `AddTileDownloader()`; no leakage into the WebApi entry point. SRP improvement over the task-spec recommendation, with rationale recorded above and verified against `coderule.mdc`. +- **Behavior preservation**: User-Agent value byte-identical (`Mozilla/5.0 (Windows NT 10.0; …)`); timeout preserved at `HttpClient`'s implicit default (100 s); retry/backoff unchanged; session-token / single-tile / batch-tile code paths all confirmed via the smoke tile-download scenarios. +- **AC coverage**: 3/3 with at least one running test per AC; AC-2 also relies on the structural fact that there is no other `CreateClient(` in `GoogleMapsDownloaderV2`. +- **Code quality**: SRP preserved; no narrative comments; consts named per C# conventions; A/A/A test pattern; no suppressed errors; argument validation unaffected. +- **Cross-batch consistency**: extends the AZ-371 (C18 magic-numbers-to-config) theme by adding a third configuration-style constant cluster; consistent with the AZ-364 collaborator-extraction pattern (small, well-named extension surfaces); AZ-370 enums (batch 19) and AZ-373 MapsVersion drop (batch 20) untouched. + +**Findings**: None. + +## Auto-Fix Attempts: 0 +## Stuck Agents: None + +## Cumulative review — K=3 trigger + +Batches 19, 20, 21 have completed since the last cumulative review (`cumulative_review_batches_16-18_cycle1_report.md`). The K=3 trigger now fires. Cumulative review will be run as the next step (autodev sub_step `cumulative-review`), targeting the union of files changed in batches 19–21. + +## Next Batch + +After the cumulative review passes, Phase 4 continues with the remaining 7 tasks in `todo/`: + +- `AZ-375` — C22 O(N) existing-tile lookup (2 SP, needs AZ-371 ✓) +- `AZ-376` — C23 delete unused `FindExistingTileAsync` (1 SP) +- `AZ-377` — C24 consolidate Earth-geometry constants (2 SP, needs AZ-371 ✓) +- `AZ-378` — C25 repo `_logger` fields (1 SP) +- `AZ-379` — C26 repo SELECT column-list constants (2 SP) +- `AZ-380` — C27 delete `CalculatePolygonDiagonalDistance` (1 SP) +- `AZ-372` — C19 dotnet format + analyzers + Coverlet (3 SP) diff --git a/_docs/_autodev_state.md b/_docs/_autodev_state.md index d49aaae..bbadc84 100644 --- a/_docs/_autodev_state.md +++ b/_docs/_autodev_state.md @@ -8,7 +8,7 @@ status: in_progress sub_step: phase: 4 name: execution - detail: "batch 20 (AZ-373) complete; K=3 cumulative review fires after batch 21" + detail: "batch 21 (AZ-374) complete; K=3 cumulative review pending for batches 19-21" retry_count: 0 cycle: 1 tracker: jira