satellite-provider/_docs/03_implementation/reviews/batch_05_review.md

Code Review Report

Batch: 5 (AZ-312, AZ-313, AZ-314 — coupling refactor: project split + DI extension methods) Date: 2026-05-10 Verdict: PASS

Scope

Structural refactoring (architecture baseline finding F4): split monolithic SatelliteProvider.Services into three per-component csprojs, rewire consumers, and extract DI registrations into per-component extension methods.

Tasks reviewed:

• AZ-312 — split Services into TileDownloader + RegionProcessing + RouteManagement csprojs
• AZ-313 — update consumer csprojs (Api, Tests, IntegrationTests) + using directives
• AZ-314 — DI extension methods per csproj + Program.cs cleanup

Changed files

• New csprojs:
  • SatelliteProvider.Services.TileDownloader/SatelliteProvider.Services.TileDownloader.csproj
  • SatelliteProvider.Services.RegionProcessing/SatelliteProvider.Services.RegionProcessing.csproj
  • SatelliteProvider.Services.RouteManagement/SatelliteProvider.Services.RouteManagement.csproj
• New DI extension files:
  • SatelliteProvider.Services.TileDownloader/TileDownloaderServiceCollectionExtensions.cs
  • SatelliteProvider.Services.RegionProcessing/RegionProcessingServiceCollectionExtensions.cs
  • SatelliteProvider.Services.RouteManagement/RouteManagementServiceCollectionExtensions.cs
• Moved (with namespace updates): TileService.cs, GoogleMapsDownloaderV2.cs, RegionService.cs, RegionProcessingService.cs, RegionRequestQueue.cs, RouteService.cs, RouteProcessingService.cs
• New common exception: SatelliteProvider.Common/Exceptions/RateLimitException.cs
• Modified: SatelliteProvider.Api/Program.cs, SatelliteProvider.Api/SatelliteProvider.Api.csproj, SatelliteProvider.Tests/SatelliteProvider.Tests.csproj, all 5 *Tests.cs files (using updates), SatelliteProvider.sln, both Dockerfiles
• Deleted: SatelliteProvider.Services/ directory and SatelliteProvider.Services.csproj

Findings

#	Severity	Category	File:Line	Title

No findings.

Phase results

Phase 2 — Spec compliance

AZ-312 (project split)

• AC-1 ✓ — Three new csprojs exist with the seven moved source files distributed correctly (verified by ls).
• AC-2 ✓ — Old SatelliteProvider.Services/ directory and csproj deleted; SatelliteProvider.sln updated.
• AC-3 ✓ — dotnet build SatelliteProvider.sln succeeds with 0 warnings, 0 errors.
• AC-4 ✓ — None of the three new csprojs reference each other. All three reference only SatelliteProvider.Common + SatelliteProvider.DataAccess.

AZ-313 (consumer rewire)

• AC-1 ✓ — Solution builds clean (47.98s, 0 warnings).
• AC-2 ✓ — All 40 unit tests pass (1.93s).
• AC-3 ✓ — Grep for using SatelliteProvider.Services; (no .<Component> suffix) returns zero source-file matches.

AZ-314 (DI extension methods)

• AC-1 ✓ — Each *ServiceCollectionExtensions.cs registers exactly the services owned by its csproj:
  • AddTileDownloader: IMemoryCache, ISatelliteDownloader, ITileService
  • AddRegionProcessing: IRegionRequestQueue (factory), IRegionService, RegionProcessingService (hosted)
  • AddRouteManagement: IRouteService, RouteProcessingService (hosted)
• AC-2 ✓ — Program.cs:33-35 calls the three extension methods; previously inlined registrations are gone.
• AC-3 ✓ — Smoke integration suite passes end-to-end (region processing, route processing, tile ZIP, security tests). Every required service resolves at runtime.
• AC-4 ✓ — No old service-registration code remains in Program.cs.

Phase 3 — Code quality

• SOLID (SRP/DIP) improved — components now have a compiler-enforced boundary; cross-component coupling can no longer be introduced silently.
• Lifetimes preserved — Singleton stayed Singleton, hosted services stayed hosted services.
• Hosted-service registration order preserved (RegionProcessingService before RouteProcessingService, matching pre-refactor order in Program.cs).
• No new dead code; no scope creep.

Phase 4 — Security quick-scan

Refactor is structural; no new input handling, query construction, or deserialization paths. Smoke security tests (SEC-01 SQL injection, SEC-02 path traversal, SEC-03 oversized region, SEC-04 malformed JSON) all pass post-refactor.

Phase 5 — Performance scan

No hot-path changes. No new allocations in the moved code (only namespace and project boundary changes).

Phase 6 — Cross-task consistency

• All three new extension methods follow the same Add<Component>(this IServiceCollection) naming convention.
• All three new csprojs share identical TFM (net8.0), ImplicitUsings, and Nullable settings.
• Package versions consistent (9.0.10 for Microsoft.Extensions.*, 13.0.4 for Newtonsoft.Json, 3.1.11 for SixLabors.ImageSharp).

Phase 7 — Architecture compliance

• Layer direction ✓ — TileDownloader (Layer 2) imports only Common + DataAccess (Layer 1). RegionProcessing and RouteManagement (Layer 3) import only Common + DataAccess (no cross-Layer-3 references after the refactor — even better than the pre-refactor baseline allowed).
• Public API respect ✓ — All cross-component imports use interfaces from SatelliteProvider.Common.Interfaces, never concrete types from sibling components.
• No new cyclic dependencies ✓ — Graph is a clean DAG: Common ← {DataAccess} ← {TileDownloader, RegionProcessing, RouteManagement} ← WebApi.
• Architecture baseline F4 — RESOLVED. The single SatelliteProvider.Services.csproj packing three components is replaced by three csprojs with compiler-enforced boundaries.
• Architecture baseline F3 — already resolved in Batch 4 (AZ-310 + AZ-311); this batch did not regress it (Program.cs tile endpoints still route through ITileService).
• RateLimitException placement — moved to SatelliteProvider.Common/Exceptions/ rather than allowing a RegionProcessing → TileDownloader ProjectReference. This is the cleanest of the three options considered (move to Common / add reference / catch general Exception) and preserves layer 3 components as siblings rather than coupling them.

Baseline Delta

Status	Finding	Severity	Notes
Resolved	F4 — SatelliteProvider.Services.csproj packs three logical components	Medium	Split into three csprojs
Carried over	(none for this batch)	—	—
Newly introduced	(none)	—	—

module-layout.md and architecture.md still describe the pre-split layout (single SatelliteProvider.Services/ project). This is expected — documentation sync is task AZ-315 (Batch 3 of this refactor run). Not a finding.

Verdict

PASS — all 12 ACs across 3 tasks satisfied. Build clean. Unit tests 40/40. Smoke integration green. Architecture baseline F4 resolved with no new findings.