From 67ecd2a22f0a74c9201d6bf045cadd19477d9332 Mon Sep 17 00:00:00 2001 From: shimoncohen Date: Thu, 6 Aug 2026 11:18:31 +0300 Subject: [PATCH 1/3] perf(fs): probe tile extensions with File.Exists instead of directory glob (MAPCO-11364 P14) FileClient.GetTilePath enumerated the directory with a `{z}/{x}/{y}.*` glob per GetTile/TileExists. Replace with direct File.Exists probes of the known jpeg/png extensions (jpeg first, matching the S3 client). File.Exists returns false for a missing tile or missing directory, so the DirectoryNotFound catch is dropped. Tests updated to mock Path.Combine/File.Exists; the now-unreachable DirectoryNotFound test is removed. Co-Authored-By: Claude Opus 4.8 (1M context) --- MergerLogic/Clients/FileClient.cs | 21 ++--- .../Clients/FileClientTest.cs | 76 +++++++------------ 2 files changed, 40 insertions(+), 57 deletions(-) diff --git a/MergerLogic/Clients/FileClient.cs b/MergerLogic/Clients/FileClient.cs index 25544910..d5a2a776 100644 --- a/MergerLogic/Clients/FileClient.cs +++ b/MergerLogic/Clients/FileClient.cs @@ -1,4 +1,5 @@ using MergerLogic.Batching; +using MergerLogic.ImageProcessing; using System.IO.Abstractions; using MergerLogic.Utils; @@ -36,16 +37,18 @@ public override bool TileExists(int z, int x, int y) private string? GetTilePath(int z, int x, int y) { - var tilePath = this._fileSystem.Path.Join(z.ToString(), x.ToString(), y.ToString()); - try + // Probe the known extensions directly instead of globbing the directory: File.Exists returns + // false for a missing tile or missing directory, so no DirectoryNotFound handling is needed. + foreach (TileFormat format in new[] { TileFormat.Jpeg, TileFormat.Png }) { - //this may or may not be faster then checking specific files of every supported type depending on the used file system - return this._fileSystem.Directory - .EnumerateFiles(this.path, $"{tilePath}.*", SearchOption.TopDirectoryOnly).FirstOrDefault(); - } - catch (DirectoryNotFoundException) - { - return null; + string candidate = this._fileSystem.Path.Combine( + this.path, z.ToString(), x.ToString(), $"{y}.{format.ToString().ToLower()}"); + if (this._fileSystem.File.Exists(candidate)) + { + return candidate; + } } + + return null; } } diff --git a/MergerLogicUnitTests/Clients/FileClientTest.cs b/MergerLogicUnitTests/Clients/FileClientTest.cs index 1b9cd607..f991be52 100644 --- a/MergerLogicUnitTests/Clients/FileClientTest.cs +++ b/MergerLogicUnitTests/Clients/FileClientTest.cs @@ -68,25 +68,12 @@ public void GetTile(bool useCoords, bool returnsNull, TileFormat targetFormat) Coord cords = new Coord(1, 2, 3); byte[] data = targetFormat == TileFormat.Jpeg ? this._jpegImageData : this._pngImageData; - var seq = new MockSequence(); - this._pathMock - .InSequence(seq) - .Setup(util => util.Join(cords.Z.ToString(), cords.X.ToString(), cords.Y.ToString())) - .Returns("testTilePath"); - this._directoryMock - .InSequence(seq) - .Setup(dir => dir.EnumerateFiles("testFilePath", "testTilePath.*", SearchOption.TopDirectoryOnly)) - .Returns(returnsNull ? Array.Empty() : new string[] { "testTilePath" }); + SetupTilePathProbe(cords, returnsNull, targetFormat, out string? foundPath); if (!returnsNull) { this._fileMock - .InSequence(seq) - .Setup(util => util.ReadAllBytes("testTilePath")) + .Setup(util => util.ReadAllBytes(foundPath)) .Returns(data); - this._imageFormatterMock - .InSequence(seq) - .Setup(formatter => formatter.GetTileFormat(data)) - .Returns(targetFormat); } var fileClient = new FileClient("testFilePath", this._geoUtilsMock.Object, this._fsMock.Object); @@ -107,6 +94,31 @@ public void GetTile(bool useCoords, bool returnsNull, TileFormat targetFormat) this._repository.VerifyAll(); } + // GetTilePath probes jpeg then png via File.Exists. Sets up only the calls the probe actually + // makes: png is not probed once jpeg is found, so its setups are omitted (strict mocks). + private void SetupTilePathProbe(Coord cords, bool missing, TileFormat targetFormat, out string? foundPath) + { + string jpegPath = "1/2/3.jpeg"; + string pngPath = "1/2/3.png"; + bool jpegExists = !missing && targetFormat == TileFormat.Jpeg; + bool pngExists = !missing && targetFormat == TileFormat.Png; + + this._pathMock + .Setup(p => p.Combine("testFilePath", cords.Z.ToString(), cords.X.ToString(), $"{cords.Y}.jpeg")) + .Returns(jpegPath); + this._fileMock.Setup(f => f.Exists(jpegPath)).Returns(jpegExists); + + if (!jpegExists) + { + this._pathMock + .Setup(p => p.Combine("testFilePath", cords.Z.ToString(), cords.X.ToString(), $"{cords.Y}.png")) + .Returns(pngPath); + this._fileMock.Setup(f => f.Exists(pngPath)).Returns(pngExists); + } + + foundPath = jpegExists ? jpegPath : (pngExists ? pngPath : null); + } + #endregion #region TileExists @@ -117,17 +129,8 @@ public void GetTile(bool useCoords, bool returnsNull, TileFormat targetFormat) public void TileExists(bool exist) { Coord cords = new Coord(1, 2, 3); - byte[] data = this._jpegImageData; - var seq = new MockSequence(); - this._pathMock - .InSequence(seq) - .Setup(util => util.Join(cords.Z.ToString(), cords.X.ToString(), cords.Y.ToString())) - .Returns("testTilePath"); - this._directoryMock - .InSequence(seq) - .Setup(dir => dir.EnumerateFiles("testFilePath", "testTilePath.*", SearchOption.TopDirectoryOnly)) - .Returns(exist ? new string[] { "testFile" } : Array.Empty()); + SetupTilePathProbe(cords, !exist, TileFormat.Jpeg, out _); var fileClient = new FileClient("testFilePath", this._geoUtilsMock.Object, this._fsMock.Object); @@ -137,29 +140,6 @@ public void TileExists(bool exist) this._repository.VerifyAll(); } - [TestMethod] - public void TileExistsReturnFalseWhenDirectoryDontExist() - { - Coord cords = new Coord(1, 2, 3); - - var seq = new MockSequence(); - this._pathMock - .InSequence(seq) - .Setup(util => util.Join(cords.Z.ToString(), cords.X.ToString(), cords.Y.ToString())) - .Returns("testTilePath"); - this._directoryMock - .InSequence(seq) - .Setup(dir => dir.EnumerateFiles("testFilePath", "testTilePath.*", SearchOption.TopDirectoryOnly)) - .Throws(); - - var fileClient = new FileClient("testFilePath", this._geoUtilsMock.Object, this._fsMock.Object); - - var res = fileClient.TileExists(cords.Z, cords.X, cords.Y); - - Assert.AreEqual(false, res); - this._repository.VerifyAll(); - } - #endregion } From b39a236ebe81ac2e236f957cb104d6a2210c76c0 Mon Sep 17 00:00:00 2001 From: shimoncohen Date: Thu, 6 Aug 2026 11:52:55 +0300 Subject: [PATCH 2/3] refactor(fs): derive probe set from TileFormat enum; declare Jpeg first FileClient now iterates Enum.GetValues() instead of a hardcoded {Jpeg, Png} array, so new tile formats are covered automatically. The TileFormat enum is reordered to Jpeg, Png so declaration order (= probe order) keeps jpeg first, matching the S3 client's historical probe order. No int-value coupling on TileFormat exists (no casts, no default(TileFormat) reliance), so the reorder is behavior-preserving. Co-Authored-By: Claude Opus 4.8 (1M context) --- MergerLogic/Clients/FileClient.cs | 4 +++- MergerLogic/ImageProcessing/ImageFormatter.cs | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/MergerLogic/Clients/FileClient.cs b/MergerLogic/Clients/FileClient.cs index d5a2a776..42b9d4d4 100644 --- a/MergerLogic/Clients/FileClient.cs +++ b/MergerLogic/Clients/FileClient.cs @@ -1,5 +1,6 @@ using MergerLogic.Batching; using MergerLogic.ImageProcessing; +using System; using System.IO.Abstractions; using MergerLogic.Utils; @@ -39,7 +40,8 @@ public override bool TileExists(int z, int x, int y) { // Probe the known extensions directly instead of globbing the directory: File.Exists returns // false for a missing tile or missing directory, so no DirectoryNotFound handling is needed. - foreach (TileFormat format in new[] { TileFormat.Jpeg, TileFormat.Png }) + // TileFormat is the source of truth for the supported extensions (declaration order = probe order). + foreach (TileFormat format in Enum.GetValues()) { string candidate = this._fileSystem.Path.Combine( this.path, z.ToString(), x.ToString(), $"{y}.{format.ToString().ToLower()}"); diff --git a/MergerLogic/ImageProcessing/ImageFormatter.cs b/MergerLogic/ImageProcessing/ImageFormatter.cs index 50c75d95..8048d027 100644 --- a/MergerLogic/ImageProcessing/ImageFormatter.cs +++ b/MergerLogic/ImageProcessing/ImageFormatter.cs @@ -5,8 +5,8 @@ namespace MergerLogic.ImageProcessing { public enum TileFormat { - [EnumMember(Value = "png")] Png, [EnumMember(Value = "jpeg")] Jpeg, + [EnumMember(Value = "png")] Png, } public class TileFormatStrategy { From a87ad252a2c960165652dadcd6e5c20b1b17084f Mon Sep 17 00:00:00 2001 From: shimoncohen Date: Thu, 6 Aug 2026 12:01:36 +0300 Subject: [PATCH 3/3] docs: trim GetTilePath comment --- MergerLogic/Clients/FileClient.cs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/MergerLogic/Clients/FileClient.cs b/MergerLogic/Clients/FileClient.cs index 42b9d4d4..858e4f6a 100644 --- a/MergerLogic/Clients/FileClient.cs +++ b/MergerLogic/Clients/FileClient.cs @@ -38,9 +38,7 @@ public override bool TileExists(int z, int x, int y) private string? GetTilePath(int z, int x, int y) { - // Probe the known extensions directly instead of globbing the directory: File.Exists returns - // false for a missing tile or missing directory, so no DirectoryNotFound handling is needed. - // TileFormat is the source of truth for the supported extensions (declaration order = probe order). + // Probe each supported extension via File.Exists (false for a missing dir too, so no glob/catch). foreach (TileFormat format in Enum.GetValues()) { string candidate = this._fileSystem.Path.Combine(