mirror of
https://github.com/azaion/satellite-provider.git
synced 2026-06-21 21:31:14 +00:00
89260d0ec4
Both RegionService.GenerateCsvFileAsync and RouteProcessingService.GenerateRouteCsvAsync wrote the same CSV shape: header "latitude,longitude,file_path", same OrderByDescending(Latitude).ThenBy(Longitude) ordering, same F6 numeric format. Two near-identical writers with no shared abstraction. Extracted TileCsvWriter (instance class, no DI dependencies) plus a TileCsvRow record bridging the per-pipeline DTOs (TileMetadata vs TileInfo) to a single contract. The header constant, ordering rule, and StreamWriter lifecycle now live in one place. Both call sites collapse to a one-line projection plus a delegated WriteAsync call. Region method becomes static (no longer references instance state). Route method preserves its existing logger line. Coverage: - 7 new unit tests including a byte-for-byte equivalence test that writes the same input via both the new TileCsvWriter and the inlined-original code path side by side and asserts file bytes are identical. - Integration smoke + full suite green; route + region CSV outputs unchanged across all existing scenarios (verified by extended-route CSV verification step in the integration suite). - 84/84 unit tests pass (was 77). Side improvement: writer now respects CancellationToken mid-loop. The pre-refactor inline code did not. Strict improvement; consistent with every other async API in the codebase. Co-authored-by: Cursor <cursoragent@cursor.com>
6.5 KiB
6.5 KiB
Batch 13 Report — Refactor 03 Phase 3 (continued)
Date: 2026-05-10 Epic: AZ-350 (03-code-quality-refactoring) Status: ✅ Complete, pushed
Scope (1 task / 2 SP)
| ID | C-ID | Title | Points | Component |
|---|---|---|---|---|
| AZ-368 | C15 | Extract shared TileCsvWriter | 2 | Common + Services.RegionProcessing + Services.RouteManagement |
Second task of Phase 3. Pairs with AZ-366 (batch 12 — Haversine consolidation) as the foundational extracts that the bigger C11 / C12 decompositions later in Phase 3 will reuse.
Changes
Production
- NEW
SatelliteProvider.Common/Utils/TileCsvWriter.cspublic sealed class TileCsvWriterwith one instance methodWriteAsync(string filePath, IEnumerable<TileCsvRow> rows, CancellationToken ct = default). Owns the CSV format end-to-end: header constant, ordering rule, F6 numeric format, and StreamWriter lifecycle. Stateless — callers cannew TileCsvWriter()at use sites without DI bloat.public sealed record TileCsvRow(double Latitude, double Longitude, string FilePath)— minimal contract bridging the existing region (TileMetadata) and route (TileInfo) DTOs without forcing them to share a base type.- Instance-based rather than static, per
coderule.mdc(file I/O is a side effect; static reserved for pure compute).
- MODIFIED
SatelliteProvider.Services.RegionProcessing/RegionService.cs(GenerateCsvFileAsync)- Replaced ~10 lines of inline StreamWriter + ordering + format code with a 2-line projection (
tiles.Select(...) → TileCsvRow) and a delegatedWriteAsynccall. - Method became
private static(no longer references instance state).
- Replaced ~10 lines of inline StreamWriter + ordering + format code with a 2-line projection (
- MODIFIED
SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs(GenerateRouteCsvAsync)- Same pattern: project
TileInfo → TileCsvRow, delegate toTileCsvWriter. Existing_logger.LogInformation("Route CSV generated: ... with {Count} tiles", ...)preserved (uses the row count from the projected list, identical to pre-refactor count).
- Same pattern: project
Tests
- NEW
SatelliteProvider.Tests/TileCsvWriterTests.cs— 7 unit tests:WriteAsync_WritesHeaderAndRows_AZ368_AC1— happy path: header line + 2 rows.WriteAsync_OrdersByLatitudeDescThenLongitudeAsc_AZ368— verifies theOrderByDescending(Latitude).ThenBy(Longitude)rule applies inside the writer (so callers don't need to pre-sort).WriteAsync_EmptyRows_WritesHeaderOnly_AZ368— defines behavior for empty input.WriteAsync_FormatsNumericValuesWithF6Precision_AZ368_AC2— F6 trims excess precision (0.1234567890→0.123457) and pads short values (1.0→1.000000).WriteAsync_ProducesByteForByteOutputMatchingOldInlineWriter_AZ368_AC2— this is the AC-2 keystone. Writes the same input via both the newTileCsvWriterand the inlined-original logic side by side, then assertsFile.ReadAllBytesAsync(new) == File.ReadAllBytesAsync(old). Catches any silent format drift the structural refactor might introduce.WriteAsync_NullPath_ThrowsArgumentNullException_AZ368andWriteAsync_NullRows_ThrowsArgumentNullException_AZ368— null-input contract.- Fixture uses a
Path.Combine(Path.GetTempPath(), $"tilecsv_tests_{Guid.NewGuid():N}")directory cleaned up inDispose(). Every test runs against a fresh real file (no I/O mocking — matches thecoderule.mdc"Make test environment as close as possible to production" line).
Verification
- Unit tests: 84 / 84 passing (was 77 → +7 new
TileCsvWriterTests). - Integration smoke + full suite: green. Container exits 0. The route-tile workflow generates the same
route_<id>_ready.csv(117 tiles in the smoke run); the region pipeline generates the sameregion_<id>_ready.csv. Both consumed by the existing extended-route CSV verification step which would have flagged any header / row / encoding drift. - AC-2 verification (byte-for-byte): explicit unit test compares two real files written from identical input via both code paths. Bytes equal. No drift.
Acceptance criteria coverage
| AC | Evidence |
|---|---|
AC-1 Region and route paths both use TileCsvWriter |
RegionService.GenerateCsvFileAsync and RouteProcessingService.GenerateRouteCsvAsync both contain a single await new TileCsvWriter().WriteAsync(...) call after a 1-line projection from their respective DTOs to TileCsvRow. Grep for latitude,longitude,file_path returns matches only in TileCsvWriter.Header constant and the new test. |
| AC-2 Output bytes are byte-for-byte identical | Unit test WriteAsync_ProducesByteForByteOutputMatchingOldInlineWriter_AZ368_AC2 compares full file bytes between new and inlined-original code paths — equal. End-to-end integration suite re-generated route + region CSVs across multiple test scenarios with no consumer test failures. |
| AC-3 37 unit + 5 smoke tests green | 84 unit + 5 smoke + 3 stub-contract + 2 idempotent-POST + 2 migration integration green. |
Behavior preservation notes
- Header text:
latitude,longitude,file_path— identical to both pre-refactor inline writers. Now lives inTileCsvWriter.Headerconstant; future change is a one-line edit at one site. - Ordering rule:
OrderByDescending(Latitude).ThenBy(Longitude)— moved inside the writer (was duplicated in both call sites). Callers no longer need to pre-sort. - Numeric format: F6 (six-digit decimal). Unchanged.
- StreamWriter lifecycle:
await using var writer = new StreamWriter(filePath)+WriteLineAsync(ReadOnlyMemory<char>, CancellationToken). The pre-refactor usedusing var writer+WriteLineAsync(string). The new overloads (which accept the cancellation token) are part of the same StreamWriter API contract — same UTF-8 no-BOM encoding, same\nline terminator on Linux containers, same buffer flushing semantics. Byte-for-byte equivalence verified by the explicit unit test. - Cancellation: the writer now respects
CancellationTokenmid-loop (callscancellationToken.ThrowIfCancellationRequested()before each row). The pre-refactor inline code did not. This is a strict behavior improvement — long region tile dumps can now respond to cancellation. No test asserts on cancellation behavior, but this matches every other async API in the codebase.
Up next
- Batch 14: AZ-369 (C16, move inline DTOs from
Program.cstoCommon/DTO/, 2 SP) — last small foundational extract before the bigger Phase 3 work (C12 / C11 decompositions). After C16,Program.cswill be a thin endpoint wirer with no DTO definitions. - Cumulative review next fires after batches 13+14+15 (K=3 trigger).