mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-22 17:21:15 +00:00
Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 7736cfd761 | |||
| 89b4bfd245 |
@@ -14,7 +14,9 @@ namespace SatelliteProvider.Services.TileDownloader;
|
||||
public class GoogleMapsDownloaderV2 : ISatelliteDownloader
|
||||
{
|
||||
private const string TILE_URL_TEMPLATE = "https://mt{0}.google.com/vt/lyrs=s&x={1}&y={2}&z={3}&token={4}";
|
||||
private const string USER_AGENT = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.0.0 Safari/537.36";
|
||||
internal const string UserAgent = "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.0.0 Safari/537.36";
|
||||
internal const string GoogleMapsTilesClientName = "GoogleMapsTiles";
|
||||
internal const int DefaultHttpClientTimeoutSeconds = 100;
|
||||
|
||||
private readonly ILogger<GoogleMapsDownloaderV2> _logger;
|
||||
private readonly string _apiKey;
|
||||
@@ -46,7 +48,7 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
|
||||
private async Task<string?> GetSessionToken()
|
||||
{
|
||||
var url = $"https://tile.googleapis.com/v1/createSession?key={_apiKey}";
|
||||
using var httpClient = _httpClientFactory.CreateClient();
|
||||
using var httpClient = _httpClientFactory.CreateClient(GoogleMapsTilesClientName);
|
||||
try
|
||||
{
|
||||
var str = JsonConvert.SerializeObject(new { mapType = "satellite" });
|
||||
@@ -102,8 +104,7 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
|
||||
|
||||
var imageBytes = await ExecuteWithRetryAsync(async () =>
|
||||
{
|
||||
using var httpClient = _httpClientFactory.CreateClient();
|
||||
httpClient.DefaultRequestHeaders.UserAgent.ParseAdd(USER_AGENT);
|
||||
using var httpClient = _httpClientFactory.CreateClient(GoogleMapsTilesClientName);
|
||||
|
||||
var response = await httpClient.GetAsync(url, token);
|
||||
|
||||
@@ -366,8 +367,7 @@ public class GoogleMapsDownloaderV2 : ISatelliteDownloader
|
||||
|
||||
var imageBytes = await ExecuteWithRetryAsync(async () =>
|
||||
{
|
||||
using var httpClient = _httpClientFactory.CreateClient();
|
||||
httpClient.DefaultRequestHeaders.UserAgent.ParseAdd(USER_AGENT);
|
||||
using var httpClient = _httpClientFactory.CreateClient(GoogleMapsTilesClientName);
|
||||
|
||||
var response = await httpClient.GetAsync(url, token);
|
||||
|
||||
|
||||
@@ -8,6 +8,11 @@ public static class TileDownloaderServiceCollectionExtensions
|
||||
public static IServiceCollection AddTileDownloader(this IServiceCollection services)
|
||||
{
|
||||
services.AddMemoryCache();
|
||||
services.AddHttpClient(GoogleMapsDownloaderV2.GoogleMapsTilesClientName, c =>
|
||||
{
|
||||
c.DefaultRequestHeaders.UserAgent.ParseAdd(GoogleMapsDownloaderV2.UserAgent);
|
||||
c.Timeout = TimeSpan.FromSeconds(GoogleMapsDownloaderV2.DefaultHttpClientTimeoutSeconds);
|
||||
});
|
||||
services.AddSingleton<ISatelliteDownloader, GoogleMapsDownloaderV2>();
|
||||
services.AddSingleton<ITileService, TileService>();
|
||||
return services;
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
using FluentAssertions;
|
||||
using Microsoft.Extensions.Caching.Memory;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Logging.Abstractions;
|
||||
using Microsoft.Extensions.Options;
|
||||
using Moq;
|
||||
@@ -464,6 +465,30 @@ public class TileServiceTests
|
||||
}
|
||||
}
|
||||
|
||||
public class TileDownloaderRegistrationTests
|
||||
{
|
||||
[Fact]
|
||||
public void AddTileDownloader_RegistersNamedGoogleMapsTilesHttpClient_AZ374_AC1()
|
||||
{
|
||||
// Arrange
|
||||
var services = new ServiceCollection();
|
||||
|
||||
// Act
|
||||
services.AddTileDownloader();
|
||||
using var provider = services.BuildServiceProvider();
|
||||
var factory = provider.GetRequiredService<IHttpClientFactory>();
|
||||
using var client = factory.CreateClient("GoogleMapsTiles");
|
||||
|
||||
// Assert
|
||||
client.DefaultRequestHeaders.UserAgent.Should().NotBeEmpty(
|
||||
"AZ-374 AC-1: the named GoogleMapsTiles client must carry the User-Agent set once at registration time");
|
||||
client.DefaultRequestHeaders.UserAgent.ToString()
|
||||
.Should().Contain("Mozilla/5.0", "preserves the existing outbound User-Agent for Google Maps");
|
||||
client.Timeout.Should().Be(TimeSpan.FromSeconds(100),
|
||||
"preserves the HttpClient default (100s) until C18 wires this to MapConfig");
|
||||
}
|
||||
}
|
||||
|
||||
public class GoogleMapsDownloaderZoomValidationTests
|
||||
{
|
||||
private static GoogleMapsDownloaderV2 BuildDownloader()
|
||||
|
||||
@@ -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)
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user