diff --git a/SatelliteProvider.Common/Utils/TileCsvWriter.cs b/SatelliteProvider.Common/Utils/TileCsvWriter.cs new file mode 100644 index 0000000..bb27a20 --- /dev/null +++ b/SatelliteProvider.Common/Utils/TileCsvWriter.cs @@ -0,0 +1,34 @@ +namespace SatelliteProvider.Common.Utils; + +// AZ-368 / C15: shared CSV writer for tile rows. +// Both region and route pipelines previously emitted the same CSV (header +// "latitude,longitude,file_path", same OrderByDescending(Latitude).ThenBy(Longitude) +// ordering, same F6 numeric format). Two near-identical writers are now one. +// +// The class is instance-based (no static, per coderule.mdc — file I/O has side +// effects). It carries no dependencies, so callers can `new TileCsvWriter()` +// at use sites without DI bloat. +public sealed class TileCsvWriter +{ + public const string Header = "latitude,longitude,file_path"; + + public async Task WriteAsync(string filePath, IEnumerable rows, CancellationToken cancellationToken = default) + { + ArgumentNullException.ThrowIfNull(filePath); + ArgumentNullException.ThrowIfNull(rows); + + var ordered = rows.OrderByDescending(r => r.Latitude).ThenBy(r => r.Longitude).ToList(); + + await using var writer = new StreamWriter(filePath); + await writer.WriteLineAsync(Header.AsMemory(), cancellationToken); + + foreach (var row in ordered) + { + cancellationToken.ThrowIfCancellationRequested(); + var line = $"{row.Latitude:F6},{row.Longitude:F6},{row.FilePath}"; + await writer.WriteLineAsync(line.AsMemory(), cancellationToken); + } + } +} + +public sealed record TileCsvRow(double Latitude, double Longitude, string FilePath); diff --git a/SatelliteProvider.Services.RegionProcessing/RegionService.cs b/SatelliteProvider.Services.RegionProcessing/RegionService.cs index 54c8a58..69b4d5c 100644 --- a/SatelliteProvider.Services.RegionProcessing/RegionService.cs +++ b/SatelliteProvider.Services.RegionProcessing/RegionService.cs @@ -293,17 +293,10 @@ public class RegionService : IRegionService return outputPath; } - private async Task GenerateCsvFileAsync(string filePath, List tiles, CancellationToken cancellationToken) + private static async Task GenerateCsvFileAsync(string filePath, List tiles, CancellationToken cancellationToken) { - var orderedTiles = tiles.OrderByDescending(t => t.Latitude).ThenBy(t => t.Longitude).ToList(); - - using var writer = new StreamWriter(filePath); - await writer.WriteLineAsync("latitude,longitude,file_path"); - - foreach (var tile in orderedTiles) - { - await writer.WriteLineAsync($"{tile.Latitude:F6},{tile.Longitude:F6},{tile.FilePath}"); - } + var rows = tiles.Select(t => new TileCsvRow(t.Latitude, t.Longitude, t.FilePath)); + await new TileCsvWriter().WriteAsync(filePath, rows, cancellationToken); } private async Task GenerateSummaryFileAsync( diff --git a/SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs b/SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs index 8604f51..993312f 100644 --- a/SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs +++ b/SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs @@ -390,17 +390,9 @@ public class RouteProcessingService : BackgroundService IEnumerable tiles, CancellationToken cancellationToken) { - var orderedTiles = tiles.OrderByDescending(t => t.Latitude).ThenBy(t => t.Longitude).ToList(); - - using var writer = new StreamWriter(filePath); - await writer.WriteLineAsync("latitude,longitude,file_path"); - - foreach (var tile in orderedTiles) - { - await writer.WriteLineAsync($"{tile.Latitude:F6},{tile.Longitude:F6},{tile.FilePath}"); - } - - _logger.LogInformation("Route CSV generated: {FilePath} with {Count} tiles", filePath, orderedTiles.Count); + var rows = tiles.Select(t => new TileCsvRow(t.Latitude, t.Longitude, t.FilePath)).ToList(); + await new TileCsvWriter().WriteAsync(filePath, rows, cancellationToken); + _logger.LogInformation("Route CSV generated: {FilePath} with {Count} tiles", filePath, rows.Count); } private async Task GenerateRouteSummaryAsync( diff --git a/SatelliteProvider.Tests/TileCsvWriterTests.cs b/SatelliteProvider.Tests/TileCsvWriterTests.cs new file mode 100644 index 0000000..6d1404d --- /dev/null +++ b/SatelliteProvider.Tests/TileCsvWriterTests.cs @@ -0,0 +1,170 @@ +using FluentAssertions; +using SatelliteProvider.Common.Utils; + +namespace SatelliteProvider.Tests; + +public class TileCsvWriterTests : IDisposable +{ + private readonly string _tempDir; + + public TileCsvWriterTests() + { + _tempDir = Path.Combine(Path.GetTempPath(), $"tilecsv_tests_{Guid.NewGuid():N}"); + Directory.CreateDirectory(_tempDir); + } + + public void Dispose() + { + if (Directory.Exists(_tempDir)) + { + Directory.Delete(_tempDir, recursive: true); + } + } + + [Fact] + public async Task WriteAsync_WritesHeaderAndRows_AZ368_AC1() + { + // Arrange + var sut = new TileCsvWriter(); + var path = Path.Combine(_tempDir, "tiles.csv"); + var rows = new[] + { + new TileCsvRow(47.461747, 37.647063, "/tiles/a.jpg"), + new TileCsvRow(47.460000, 37.648000, "/tiles/b.jpg"), + }; + + // Act + await sut.WriteAsync(path, rows); + + // Assert + var lines = await File.ReadAllLinesAsync(path); + lines.Should().HaveCount(3); + lines[0].Should().Be("latitude,longitude,file_path"); + lines.Skip(1).Should().Contain("47.461747,37.647063,/tiles/a.jpg"); + lines.Skip(1).Should().Contain("47.460000,37.648000,/tiles/b.jpg"); + } + + [Fact] + public async Task WriteAsync_OrdersByLatitudeDescThenLongitudeAsc_AZ368() + { + // Arrange + var sut = new TileCsvWriter(); + var path = Path.Combine(_tempDir, "ordered.csv"); + // Intentionally unsorted; expected order is lat DESC, then lon ASC. + var rows = new[] + { + new TileCsvRow(10.0, 30.0, "/c.jpg"), + new TileCsvRow(20.0, 20.0, "/b.jpg"), + new TileCsvRow(20.0, 10.0, "/a.jpg"), + }; + + // Act + await sut.WriteAsync(path, rows); + + // Assert + var lines = await File.ReadAllLinesAsync(path); + lines[0].Should().Be("latitude,longitude,file_path"); + lines[1].Should().Be("20.000000,10.000000,/a.jpg", "lat=20 wins over lat=10; lon=10 wins over lon=20"); + lines[2].Should().Be("20.000000,20.000000,/b.jpg"); + lines[3].Should().Be("10.000000,30.000000,/c.jpg", "lat=10 comes last in desc order"); + } + + [Fact] + public async Task WriteAsync_EmptyRows_WritesHeaderOnly_AZ368() + { + // Arrange + var sut = new TileCsvWriter(); + var path = Path.Combine(_tempDir, "empty.csv"); + + // Act + await sut.WriteAsync(path, Array.Empty()); + + // Assert + var lines = await File.ReadAllLinesAsync(path); + lines.Should().ContainSingle(); + lines[0].Should().Be("latitude,longitude,file_path"); + } + + [Fact] + public async Task WriteAsync_FormatsNumericValuesWithF6Precision_AZ368_AC2() + { + // Arrange + var sut = new TileCsvWriter(); + var path = Path.Combine(_tempDir, "format.csv"); + var rows = new[] + { + new TileCsvRow(1.0, 2.0, "/x.jpg"), + new TileCsvRow(0.1234567890, 99.0000001, "/y.jpg"), + }; + + // Act + await sut.WriteAsync(path, rows); + + // Assert — F6 trims/pads to exactly 6 decimal places + var lines = await File.ReadAllLinesAsync(path); + lines.Should().Contain("1.000000,2.000000,/x.jpg"); + lines.Should().Contain("0.123457,99.000000,/y.jpg"); + } + + [Fact] + public async Task WriteAsync_ProducesByteForByteOutputMatchingOldInlineWriter_AZ368_AC2() + { + // Arrange — replicate the exact pre-refactor inline writer behavior to + // prove the extracted class doesn't drift the on-disk format. + var rows = new[] + { + new TileCsvRow(47.461747, 37.647063, "/tiles/a.jpg"), + new TileCsvRow(47.460000, 37.648000, "/tiles/b.jpg"), + new TileCsvRow(47.470000, 37.640000, "/tiles/c.jpg"), + }; + + var newPath = Path.Combine(_tempDir, "new.csv"); + var oldPath = Path.Combine(_tempDir, "old.csv"); + + // Act — new path via TileCsvWriter + await new TileCsvWriter().WriteAsync(newPath, rows); + + // Act — old path via inlined original logic + var ordered = rows.OrderByDescending(t => t.Latitude).ThenBy(t => t.Longitude).ToList(); + using (var writer = new StreamWriter(oldPath)) + { + await writer.WriteLineAsync("latitude,longitude,file_path"); + foreach (var t in ordered) + { + await writer.WriteLineAsync($"{t.Latitude:F6},{t.Longitude:F6},{t.FilePath}"); + } + } + + // Assert + var newBytes = await File.ReadAllBytesAsync(newPath); + var oldBytes = await File.ReadAllBytesAsync(oldPath); + newBytes.Should().Equal(oldBytes, "AZ-368 AC-2 requires byte-for-byte identical CSV output"); + } + + [Fact] + public async Task WriteAsync_NullPath_ThrowsArgumentNullException_AZ368() + { + // Arrange + var sut = new TileCsvWriter(); + + // Act + Func act = () => sut.WriteAsync(null!, Array.Empty()); + + // Assert + await act.Should().ThrowAsync(); + } + + [Fact] + public async Task WriteAsync_NullRows_ThrowsArgumentNullException_AZ368() + { + // Arrange + var sut = new TileCsvWriter(); + var path = Path.Combine(_tempDir, "nullrows.csv"); + + // Act + Func act = () => sut.WriteAsync(path, null!); + + // Assert + await act.Should().ThrowAsync(); + } +} diff --git a/_docs/03_implementation/batch_13_report.md b/_docs/03_implementation/batch_13_report.md new file mode 100644 index 0000000..eea5468 --- /dev/null +++ b/_docs/03_implementation/batch_13_report.md @@ -0,0 +1,63 @@ +# 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.cs` + - `public sealed class TileCsvWriter` with one instance method `WriteAsync(string filePath, IEnumerable rows, CancellationToken ct = default)`. Owns the CSV format end-to-end: header constant, ordering rule, F6 numeric format, and StreamWriter lifecycle. Stateless — callers can `new 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 delegated `WriteAsync` call. + - Method became `private static` (no longer references instance state). +- **MODIFIED** `SatelliteProvider.Services.RouteManagement/RouteProcessingService.cs` (`GenerateRouteCsvAsync`) + - Same pattern: project `TileInfo → TileCsvRow`, delegate to `TileCsvWriter`. Existing `_logger.LogInformation("Route CSV generated: ... with {Count} tiles", ...)` preserved (uses the row count from the projected list, identical to pre-refactor count). + +### Tests +- **NEW** `SatelliteProvider.Tests/TileCsvWriterTests.cs` — 7 unit tests: + - `WriteAsync_WritesHeaderAndRows_AZ368_AC1` — happy path: header line + 2 rows. + - `WriteAsync_OrdersByLatitudeDescThenLongitudeAsc_AZ368` — verifies the `OrderByDescending(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 new `TileCsvWriter` and the inlined-original logic side by side, then asserts `File.ReadAllBytesAsync(new) == File.ReadAllBytesAsync(old)`. Catches any silent format drift the structural refactor might introduce. + - `WriteAsync_NullPath_ThrowsArgumentNullException_AZ368` and `WriteAsync_NullRows_ThrowsArgumentNullException_AZ368` — null-input contract. + - Fixture uses a `Path.Combine(Path.GetTempPath(), $"tilecsv_tests_{Guid.NewGuid():N}")` directory cleaned up in `Dispose()`. Every test runs against a fresh real file (no I/O mocking — matches the `coderule.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__ready.csv` (117 tiles in the smoke run); the region pipeline generates the same `region__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 in `TileCsvWriter.Header` constant; 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, CancellationToken)`. The pre-refactor used `using 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 `\n` line terminator on Linux containers, same buffer flushing semantics. Byte-for-byte equivalence verified by the explicit unit test. +- **Cancellation**: the writer now respects `CancellationToken` mid-loop (calls `cancellationToken.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.cs` to `Common/DTO/`, 2 SP) — last small foundational extract before the bigger Phase 3 work (C12 / C11 decompositions). After C16, `Program.cs` will be a thin endpoint wirer with no DTO definitions. +- **Cumulative review** next fires after batches 13+14+15 (K=3 trigger).