From 75190b8a2bf45bc2bf910512aec7d7b0670ec41d Mon Sep 17 00:00:00 2001 From: Stef Kariotidis Date: Thu, 26 Mar 2026 14:06:20 +0200 Subject: [PATCH 1/2] Fix cold-cache tile loading: return 503 for budget exhaustion, add client-side retry After clearing the tile cache, map views showed persistent gray areas because budget-exhausted tile fetches returned 404 (permanent) instead of signaling a transient failure. Leaflet never retried, so tiles stayed gray until manual refresh. Changes: - Extract DbMetadataZoomThreshold constant (replaces magic number 9 in 9 locations) - Guard metadata insert with tileData != null to prevent ghost DB rows on budget exhaustion - Add RequestIdLoggingMiddleware + Serilog .Enrich.FromLogContext() for per-request log correlation - Introduce TileRetrievalResult to distinguish success/not-found/throttled states - Return HTTP 503 + Retry-After header when outbound budget is exhausted - Add RetryTileLayer (fetch-based L.TileLayer subclass) that retries on 503 with backoff - Centralize tile layer creation via createTileLayer() factory, replacing duplicated boilerplate across 13 JS files - Add inline tileerror retry fallback for 2 cshtml views with inline scripts Closes #206 --- Areas/Public/Controllers/TilesController.cs | 19 ++- Areas/User/Views/HiddenAreas/Create.cshtml | 23 ++- Areas/User/Views/HiddenAreas/Edit.cshtml | 23 ++- CHANGELOG.md | 20 +++ Middleware/PerformanceMonitoringMiddleware.cs | 3 +- Middleware/RequestIdLoggingMiddleware.cs | 33 ++++ Program.cs | 13 +- Services/TileCacheService.cs | 96 ++++++++--- Services/TileRetrievalResult.cs | 23 +++ .../Services/TileCacheServiceTests.cs | 55 +++++- wwwroot/js/Areas/Manager/Groups/Map.js | 8 +- .../js/Areas/Public/UsersTimeline/Embed.js | 10 +- .../js/Areas/Public/UsersTimeline/Timeline.js | 10 +- wwwroot/js/Areas/User/Groups/Map.js | 8 +- wwwroot/js/Areas/User/Location/Create.js | 9 +- wwwroot/js/Areas/User/Location/Edit.js | 13 +- wwwroot/js/Areas/User/Location/Index.js | 10 +- .../js/Areas/User/Timeline/Chronological.js | 10 +- wwwroot/js/Areas/User/Timeline/Index.js | 9 +- wwwroot/js/Areas/User/Trip/Index.js | 11 +- wwwroot/js/Areas/User/Trip/mapManager.js | 14 +- wwwroot/js/Areas/User/Visit/Edit.js | 11 +- wwwroot/js/Trip/tripViewerHelpers.js | 10 +- wwwroot/js/retryTileLayer.js | 158 ++++++++++++++++++ 24 files changed, 451 insertions(+), 148 deletions(-) create mode 100644 Middleware/RequestIdLoggingMiddleware.cs create mode 100644 Services/TileRetrievalResult.cs create mode 100644 wwwroot/js/retryTileLayer.js diff --git a/Areas/Public/Controllers/TilesController.cs b/Areas/Public/Controllers/TilesController.cs index a8c93901..ff4044f2 100644 --- a/Areas/Public/Controllers/TilesController.cs +++ b/Areas/Public/Controllers/TilesController.cs @@ -149,11 +149,20 @@ public async Task GetTile(int z, int x, int y) } // Call the tile cache service to retrieve the tile. - // The service will either return the cached tile data or (if missing) download, cache, and then return it. - var tileData = await _tileCacheService.RetrieveTileAsync(z.ToString(), x.ToString(), y.ToString(), tileUrl, HttpContext.RequestAborted); - if (tileData == null) + // The service will either return the cached tile data, signal budget exhaustion (503), + // or indicate the tile was not found (404). + var result = await _tileCacheService.RetrieveTileAsync(z.ToString(), x.ToString(), y.ToString(), tileUrl, HttpContext.RequestAborted); + + if (result.BudgetExhausted) + { + _logger.LogWarning("Tile budget exhausted for {Z}/{X}/{Y}", z, x, y); + Response.Headers["Retry-After"] = "5"; + return StatusCode(503, "Tile server busy. Please retry shortly."); + } + + if (result.TileData == null) { - _logger.LogError("Tile data not found for {z}/{x}/{y}", z, x, y); + _logger.LogError("Tile data not found for {Z}/{X}/{Y}", z, x, y); return NotFound("Tile not found."); } @@ -164,7 +173,7 @@ public async Task GetTile(int z, int x, int y) Response.Headers["X-Content-Type-Options"] = "nosniff"; // Return the tile data with the appropriate content type. - return File(tileData, "image/png"); + return File(result.TileData, "image/png"); } /// diff --git a/Areas/User/Views/HiddenAreas/Create.cshtml b/Areas/User/Views/HiddenAreas/Create.cshtml index 6419e3c2..cefded5e 100644 --- a/Areas/User/Views/HiddenAreas/Create.cshtml +++ b/Areas/User/Views/HiddenAreas/Create.cshtml @@ -46,11 +46,32 @@ ViewData["LoadLeaflet"] = true; // Make sure your _Layout includes leaflet and const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; // Add tile layer from the cache proxy. - L.tileLayer(tilesUrl, { + const tileLayer = L.tileLayer(tilesUrl, { maxZoom: 19, attribution: tilesAttribution }).addTo(map); + // Retry failed tiles with backoff (max 5 attempts, matching retryTileLayer.js). + // Inline fallback for views that cannot use ES module imports. + // Note: unlike retryTileLayer.js (which uses fetch() to detect 503 vs 404), + // this img-based approach retries ALL errors indiscriminately. Acceptable for + // these admin-only views with low traffic — worst case is 5 wasted retries + // per genuinely missing tile. + (function () { + var retryCounts = {}; + tileLayer.on('tileerror', function (e) { + var src = e.tile.src; + if (!src) return; + var key = src.split('?')[0]; + retryCounts[key] = (retryCounts[key] || 0) + 1; + if (retryCounts[key] <= 5) { + setTimeout(function () { + e.tile.src = src; + }, 1000 * Math.pow(2, retryCounts[key] - 1)); + } + }); + })(); + map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); // Layer to hold drawn polygons diff --git a/Areas/User/Views/HiddenAreas/Edit.cshtml b/Areas/User/Views/HiddenAreas/Edit.cshtml index 069dcd77..ced5a376 100644 --- a/Areas/User/Views/HiddenAreas/Edit.cshtml +++ b/Areas/User/Views/HiddenAreas/Edit.cshtml @@ -44,11 +44,32 @@ const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; - L.tileLayer(tilesUrl, { + const tileLayer = L.tileLayer(tilesUrl, { maxZoom: 19, attribution: tilesAttribution }).addTo(map); + // Retry failed tiles with backoff (max 5 attempts, matching retryTileLayer.js). + // Inline fallback for views that cannot use ES module imports. + // Note: unlike retryTileLayer.js (which uses fetch() to detect 503 vs 404), + // this img-based approach retries ALL errors indiscriminately. Acceptable for + // these admin-only views with low traffic — worst case is 5 wasted retries + // per genuinely missing tile. + (function () { + var retryCounts = {}; + tileLayer.on('tileerror', function (e) { + var src = e.tile.src; + if (!src) return; + var key = src.split('?')[0]; + retryCounts[key] = (retryCounts[key] || 0) + 1; + if (retryCounts[key] <= 5) { + setTimeout(function () { + e.tile.src = src; + }, 1000 * Math.pow(2, retryCounts[key] - 1)); + } + }); + })(); + map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); const drawnItems = new L.FeatureGroup(); diff --git a/CHANGELOG.md b/CHANGELOG.md index 8f0db9dd..b2672388 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,25 @@ # CHANGELOG +## [1.2.21] - 2026-03-26 + +### Added +- `RetryTileLayer` — custom Leaflet TileLayer subclass using `fetch()` for HTTP status code access; retries on 503 with exponential backoff and `Retry-After` header support (#206) +- `createTileLayer()` factory in `retryTileLayer.js` — centralizes tile layer creation, replacing duplicated boilerplate across 13 JS files (#206) +- `TileRetrievalResult` — typed result class distinguishing tile success, not-found, and budget-throttled states (#206) +- `RequestIdLoggingMiddleware` — pushes `HttpContext.TraceIdentifier` into Serilog `LogContext` so every log entry includes `RequestId` automatically (#206) +- Serilog `.Enrich.FromLogContext()` and `{Properties:j}` output templates for console and file sinks (#206) +- `DbMetadataZoomThreshold` constant replacing magic number `9` across `TileCacheService` (#206) +- Inline `tileerror` retry fallback for HiddenAreas Create/Edit views (cshtml inline scripts) (#206) + +### Fixed +- **HIGH:** Cold-cache tile loading returned 404 for budget-exhausted tiles — Leaflet treated as permanent failure, showing persistent gray areas. Now returns 503 + `Retry-After` header; client retries automatically (#206) +- **MEDIUM:** Ghost metadata rows stored in DB when tile fetch was aborted by budget exhaustion — rows had `Size=0`, null `ETag`/`ExpiresAtUtc`, pointing to non-existent files (#206) + +### Changed +- `CacheTileAsync` now returns `bool` (`false` = budget exhaustion) instead of `void` (#206) +- `RetrieveTileAsync` now returns `TileRetrievalResult` instead of `byte[]?` (#206) +- `PerformanceMonitoringMiddleware` log line now includes explicit `RequestId` parameter (#206) + ## [1.2.20] - 2026-03-22 ### Added diff --git a/Middleware/PerformanceMonitoringMiddleware.cs b/Middleware/PerformanceMonitoringMiddleware.cs index 9e5d582b..ce55adb7 100644 --- a/Middleware/PerformanceMonitoringMiddleware.cs +++ b/Middleware/PerformanceMonitoringMiddleware.cs @@ -21,7 +21,8 @@ public async Task InvokeAsync(HttpContext context) stopwatch.Stop(); // Stop measuring time - // Log the duration of the request + // Log the duration of the request. RequestId is automatically included via + // {Properties:j} from LogContext (pushed by RequestIdLoggingMiddleware). _logger.LogInformation("Request [{Method}] {Path} took {ElapsedMilliseconds} ms", context.Request.Method, context.Request.Path, diff --git a/Middleware/RequestIdLoggingMiddleware.cs b/Middleware/RequestIdLoggingMiddleware.cs new file mode 100644 index 00000000..79f90cc5 --- /dev/null +++ b/Middleware/RequestIdLoggingMiddleware.cs @@ -0,0 +1,33 @@ +using Serilog.Context; + +namespace Wayfarer.Middleware; + +/// +/// Pushes HttpContext.TraceIdentifier into Serilog's LogContext so every log entry +/// within the request pipeline includes the RequestId property automatically. +/// +public class RequestIdLoggingMiddleware +{ + private readonly RequestDelegate _next; + + /// + /// Initializes a new instance of . + /// + public RequestIdLoggingMiddleware(RequestDelegate next) + { + _next = next; + } + + /// + /// Pushes the request's TraceIdentifier as a "RequestId" property into Serilog's + /// LogContext, then invokes the next middleware. The property is automatically + /// removed when the request completes. + /// + public async Task InvokeAsync(HttpContext context) + { + using (LogContext.PushProperty("RequestId", context.TraceIdentifier)) + { + await _next(context); + } + } +} diff --git a/Program.cs b/Program.cs index 2f361b09..aebb7893 100644 --- a/Program.cs +++ b/Program.cs @@ -263,10 +263,16 @@ static void ConfigureLogging(WebApplicationBuilder builder) throw new InvalidOperationException( "Log file path is not configured. Please check your appsettings.json or appsettings.Development.json."); - // Configure Serilog for logging to console, file, and PostgreSQL + // Configure Serilog for logging to console, file, and PostgreSQL. + // .Enrich.FromLogContext() enables LogContext properties (e.g., RequestId pushed by + // RequestIdLoggingMiddleware) to flow into all sinks automatically. + // {Properties:j} in output templates renders pushed properties as JSON. Log.Logger = new LoggerConfiguration() - .WriteTo.Console() // Logs to the console - .WriteTo.File(logFilePath, rollingInterval: RollingInterval.Day) // Logs to a file with daily rotation + .Enrich.FromLogContext() + .WriteTo.Console(outputTemplate: + "[{Timestamp:HH:mm:ss} {Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}") + .WriteTo.File(logFilePath, rollingInterval: RollingInterval.Day, outputTemplate: + "{Timestamp:yyyy-MM-dd HH:mm:ss.fff zzz} [{Level:u3}] {Message:lj} {Properties:j}{NewLine}{Exception}") .WriteTo.PostgreSQL(builder.Configuration.GetConnectionString("DefaultConnection"), "AuditLogs", // Table for storing logs needAutoCreateTable: true) // Auto-creates the table if it doesn't exist @@ -694,6 +700,7 @@ static async Task ConfigureMiddleware(WebApplication app) // CRITICAL: Add this as the FIRST middleware to process forwarded headers from nginx app.UseForwardedHeaders(); + app.UseMiddleware(); // Enriches Serilog LogContext with HttpContext.TraceIdentifier app.UseMiddleware(); // Custom middleware for monitoring performance // Use specific middlewares based on the environment diff --git a/Services/TileCacheService.cs b/Services/TileCacheService.cs index 8430982d..99c2df2d 100644 --- a/Services/TileCacheService.cs +++ b/Services/TileCacheService.cs @@ -44,6 +44,12 @@ public class TileCacheService /// private const int LRU_TO_EVICT = 50; + /// + /// Zoom levels at or above this threshold use database-backed metadata. + /// Zoom levels below this use JSON sidecar files on disk (fewer tiles, simpler management). + /// + private const int DbMetadataZoomThreshold = 9; + /// /// Minimum staleness before updating LastAccessed in the database. /// Reduces DB writes by ~99% for popular tiles while maintaining adequate LRU precision. @@ -276,6 +282,23 @@ internal static void ResetForTesting() // Already at capacity after drain — safe to ignore. } } + + /// + /// Drains all outbound budget tokens and stops replenishment for testing. + /// After this call, the next will return false (budget exhausted). + /// Also pre-cancels the fresh CTS so the replenisher exits immediately when + /// lazily starts it — prevents token refill during the test. + /// + internal static void DrainForTesting() + { + StopReplenisher(); + while (_tokens.CurrentCount > 0) + { + _tokens.Wait(0); + } + // Pre-cancel the fresh CTS so the replenisher cannot refill tokens. + _replenisherCts.Cancel(); + } } /// @@ -703,10 +726,11 @@ private void WriteSidecarMetadata(string tileFilePath, TileSidecarMetadata metad /// /// Downloads a tile from the given URL and caches it on the file system. /// Stores ETag, Last-Modified, and computed expiry from upstream response headers. - /// For zoom levels >= 9, metadata is stored (or updated) in the database. - /// For zoom levels 0-8, metadata is stored as a JSON sidecar file. + /// For zoom levels >= DbMetadataZoomThreshold, metadata is stored (or updated) in the database. + /// For zoom levels below that, metadata is stored as a JSON sidecar file. + /// Returns false if the outbound budget was exhausted (tile not downloaded), true otherwise. /// - public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoordinate, string yCoordinate, + public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoordinate, string yCoordinate, CancellationToken cancellationToken = default) { try @@ -759,9 +783,9 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord await File.WriteAllBytesAsync(tileFilePath, tileData); } - // For zoom < 9, write sidecar in the same lock acquisition as the tile file. + // For zoom < DbMetadataZoomThreshold, write sidecar in the same lock acquisition as the tile file. // This eliminates TOCTOU where a concurrent reader sees the tile but no metadata. - if (zoom < 9) + if (zoom < DbMetadataZoomThreshold) { WriteSidecarMetadata(tileFilePath, new TileSidecarMetadata { @@ -774,7 +798,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord catch (IOException ioEx) { _logger.LogError(ioEx, "Failed to write tile data to file: {TileFilePath}", tileFilePath); - return; + return true; } finally { @@ -790,16 +814,26 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord retryCount--; if (retryCount == 0) { + // Returns true (not budget-exhausted) so the controller sends 404 rather than 503. + // Upstream HTTP failures (500/502/504) are non-retryable at the client level — + // retrying would not help if OSM is down and would only pile up stale requests. _logger.LogError("Failed to download tile after multiple attempts: {TileUrl}", TileProviderCatalog.RedactApiKey(tileUrl)); - return; + return true; } // Optional: Delay between retries to avoid rate limiting await Task.Delay(500); // 500ms delay between retries } - // For zoom levels >= 9, store or update metadata in the database. - if (zoom >= 9) + // Budget was never acquired — signal throttling to caller. + if (tileData == null && !budgetAcquired) + return false; + + // For zoom levels >= DbMetadataZoomThreshold, store or update metadata in the database. + // Only store metadata when tile data was actually downloaded — budget exhaustion + // or HTTP failures leave tileData null, and writing a row with Size=0 and no file + // would create ghost metadata that pollutes LRU eviction. + if (tileData != null && zoom >= DbMetadataZoomThreshold) { var existingMetadata = await _dbContext.TileCacheMetadata .FirstOrDefaultAsync(t => t.Zoom == zoom && t.X == x && t.Y == y); @@ -892,7 +926,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord if (databaseValues == null) { _logger.LogError("Tile metadata was deleted by another process."); - return; + return true; } await entry.ReloadAsync(); @@ -909,17 +943,20 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord { _logger.LogError( "Failed to update tile metadata after multiple attempts due to concurrency conflicts."); - return; + return true; } // Adjust the in-memory cache size using the previously saved value. Interlocked.Add(ref _currentCacheSize, (tileData?.Length ?? 0) - oldSize); } } + + return true; } catch (Exception ex) { _logger.LogError(ex, "Error caching tile from {TileUrl}.", TileProviderCatalog.RedactApiKey(tileUrl)); + return true; } } @@ -927,8 +964,9 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord /// Retrieves a tile from the cache. If the tile exists on disk, checks whether it is /// expired and re-validates with the upstream server using conditional requests. /// If the file is missing, downloads and caches the tile. + /// Returns a distinguishing success, not-found, and throttled states. /// - public async Task RetrieveTileAsync(string zoomLevel, string xCoordinate, string yCoordinate, + public async Task RetrieveTileAsync(string zoomLevel, string xCoordinate, string yCoordinate, string? tileUrl = null, CancellationToken cancellationToken = default) { try @@ -946,7 +984,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord { _logger.LogWarning("Invalid tile coordinates: z={Zoom} x={X} y={Y}", zoomLevel, xCoordinate, yCoordinate); - return null; + return TileRetrievalResult.NotFound(); } var tileKey = $"{zoomLevel}_{xCoordinate}_{yCoordinate}"; @@ -963,7 +1001,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord string? etag = null; DateTime? lastModified = null; - if (zoomLvl >= 9) + if (zoomLvl >= DbMetadataZoomThreshold) { // Single DB round-trip: load metadata + conditionally update LastAccessed var meta = await LoadAndTouchMetadataAsync(zoomLvl, xVal, yVal); @@ -1048,7 +1086,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord // File deleted by concurrent eviction/purge — treat as cache miss. } - if (cachedTileData != null) return cachedTileData; + if (cachedTileData != null) return TileRetrievalResult.Success(cachedTileData); } // Tile is expired — re-validate with upstream (if we have a URL) @@ -1066,8 +1104,8 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord xVal, yVal, etag, lastModified, clientIp, CancellationToken.None))); try { - var result = await flight.Value; - if (result != null) return result; + var revalidationResult = await flight.Value; + if (revalidationResult != null) return TileRetrievalResult.Success(revalidationResult); } catch (Exception ex) { @@ -1095,18 +1133,18 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord // File deleted by concurrent eviction/purge — treat as cache miss. } - if (staleTileData != null) return staleTileData; + if (staleTileData != null) return TileRetrievalResult.Success(staleTileData); } // 2. If the tile is not on disk, but we have a URL, attempt to fetch it. if (string.IsNullOrEmpty(tileUrl)) { _logger.LogWarning("Tile not found and no URL provided: {TileFilePath}", tileFilePath); - return null; + return TileRetrievalResult.NotFound(); } _logger.LogDebug("Tile not in cache. Fetching from: {TileUrl}", TileProviderCatalog.RedactApiKey(tileUrl)); - await CacheTileAsync(tileUrl, zoomLevel, xCoordinate, yCoordinate, cancellationToken); + var cached = await CacheTileAsync(tileUrl, zoomLevel, xCoordinate, yCoordinate, cancellationToken); // After fetching, read the file. No lock needed for reads (see fast-path comment above). byte[]? fetchedTileData = null; @@ -1122,12 +1160,16 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord // File deleted by concurrent eviction/purge — treat as cache miss. } - return fetchedTileData; + if (fetchedTileData != null) + return TileRetrievalResult.Success(fetchedTileData); + + // File doesn't exist after CacheTileAsync — distinguish budget exhaustion from other failures. + return cached ? TileRetrievalResult.NotFound() : TileRetrievalResult.Throttled(); } catch (Exception ex) { _logger.LogError(ex, "Error retrieving tile from cache."); - return null; + return TileRetrievalResult.NotFound(); } } @@ -1159,7 +1201,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord var newExpiry = ParseCacheExpiry(response); var newEtag = response.Headers.ETag?.Tag ?? etag; - if (zoom >= 9) + if (zoom >= DbMetadataZoomThreshold) { // Use own scope to avoid disposed DbContext from the originating request. using var scope = _serviceScopeFactory.CreateScope(); @@ -1170,7 +1212,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord _logger.LogDebug("Tile {TileKey} re-validated (304 Not Modified)", tileKey); byte[]? data = null; - if (zoom < 9) + if (zoom < DbMetadataZoomThreshold) { // Sidecar write + file read under the same lock to prevent a concurrent // purge from deleting the sidecar between write and read (TOCTOU). @@ -1227,7 +1269,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord { await File.WriteAllBytesAsync(tileFilePath, tileData); - if (zoom < 9) + if (zoom < DbMetadataZoomThreshold) { WriteSidecarMetadata(tileFilePath, new TileSidecarMetadata { @@ -1242,7 +1284,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string xCoord _cacheLock.Release(); } - if (zoom >= 9) + if (zoom >= DbMetadataZoomThreshold) { // Use own scope to avoid disposed DbContext from the originating request. using var scope = _serviceScopeFactory.CreateScope(); @@ -1754,7 +1796,7 @@ public async Task PurgeLRUCacheAsync() // Project only the fields needed — AsNoTracking avoids change tracker overhead. var lruCache = await dbContext.TileCacheMetadata .AsNoTracking() - .Where(file => file.Zoom >= 9) + .Where(file => file.Zoom >= DbMetadataZoomThreshold) .Select(t => new { t.Id, t.TileFilePath, t.Size }) .ToListAsync(); diff --git a/Services/TileRetrievalResult.cs b/Services/TileRetrievalResult.cs new file mode 100644 index 00000000..3a3d2e6e --- /dev/null +++ b/Services/TileRetrievalResult.cs @@ -0,0 +1,23 @@ +namespace Wayfarer.Services; + +/// +/// Result of a tile retrieval attempt, distinguishing successful data, +/// genuine absence, and transient throttling so the controller can +/// return the appropriate HTTP status code. +/// +/// Tile image bytes when retrieval succeeded, null otherwise. +/// +/// True when the tile could not be fetched because the outbound request budget +/// was exhausted. The controller should return 503 with Retry-After. +/// +public sealed record TileRetrievalResult(byte[]? TileData, bool BudgetExhausted) +{ + /// Tile data retrieved successfully. + public static TileRetrievalResult Success(byte[] data) => new(data, false); + + /// Tile not found (genuine absence or unrecoverable error). + public static TileRetrievalResult NotFound() => new(null, false); + + /// Upstream budget exhausted — transient, client should retry. + public static TileRetrievalResult Throttled() => new(null, true); +} diff --git a/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs b/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs index ecbca317..44b2dcc4 100644 --- a/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs +++ b/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs @@ -49,7 +49,8 @@ public async Task RetrieveTileAsync_UpdatesLastAccessed() meta.LastAccessed = old; db.SaveChanges(); - var bytes = await service.RetrieveTileAsync("9", "3", "4"); + var result = await service.RetrieveTileAsync("9", "3", "4"); + var bytes = result.TileData; Assert.NotNull(bytes); Assert.True(db.TileCacheMetadata.Single().LastAccessed > old); @@ -294,7 +295,8 @@ public async Task RetrieveTileAsync_ServesFromCache_WhenNotExpired() var callCount = handler.CallCount; // Retrieve should serve from cache without HTTP call - var bytes = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var result = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var bytes = result.TileData; Assert.NotNull(bytes); Assert.Equal(callCount, handler.CallCount); // No additional HTTP calls @@ -318,7 +320,8 @@ public async Task RetrieveTileAsync_SendsConditionalRequest_WhenExpired() await db.SaveChangesAsync(); // Retrieve should send conditional request because tile is expired - var bytes = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var result = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var bytes = result.TileData; Assert.NotNull(bytes); Assert.True(handler.CallCount > callCountAfterCache, "Expected conditional HTTP request"); @@ -342,7 +345,8 @@ public async Task RetrieveTileAsync_HandlesNotModified304_WithoutRedownload() await db.SaveChangesAsync(); // Retrieve: tile is expired, handler returns 304 when If-None-Match matches - var bytes = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var result = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var bytes = result.TileData; Assert.NotNull(bytes); Assert.Equal(originalFile, bytes); // Same data, not re-downloaded @@ -373,7 +377,8 @@ public async Task RetrieveTileAsync_ReplacesFile_On200AfterExpiry() // Force the handler to return 200 on revalidation (different etag = new content) handler.ForceRevalidation200 = true; - var bytes = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var result = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var bytes = result.TileData; Assert.NotNull(bytes); // DB metadata should now have the new etag @@ -401,7 +406,8 @@ public async Task RetrieveTileAsync_ServesStaleCache_WhenRevalidationFails() handler.FailNextRequest = true; // Retrieve should serve stale cached file despite re-validation failure - var bytes = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var result = await service.RetrieveTileAsync("9", "1", "2", "http://tiles/9/1/2.png"); + var bytes = result.TileData; Assert.NotNull(bytes); } @@ -473,7 +479,7 @@ public async Task RetrieveTileAsync_CoalescesConcurrentRevalidations() var results = await Task.WhenAll(tasks); // All should return data - Assert.All(results, r => Assert.NotNull(r)); + Assert.All(results, r => Assert.NotNull(r.TileData)); // Only 1 additional HTTP request should have been made (coalesced) var additionalCalls = handler.CallCount - callCountAfterCache; @@ -499,6 +505,41 @@ public async Task CacheTileAsync_StoresSidecarWithLastModified_ForLowZoom() Assert.NotNull(sidecar!.LastModifiedUpstream); } + [Fact] + public async Task CacheTileAsync_ReturnsFalse_WhenBudgetExhausted() + { + using var dir = new TempDir(); + var db = CreateDbContext(); + var service = CreateService(db, dir.Path); + + // Drain all tokens and stop replenishment — next AcquireAsync returns false. + TileCacheService.OutboundBudget.DrainForTesting(); + + // CacheTileAsync should return false (budget exhausted) and not write any file or metadata. + var result = await service.CacheTileAsync("http://tiles/10/1/1.png", "10", "1", "1"); + + Assert.False(result); + Assert.False(File.Exists(Path.Combine(dir.Path, "10_1_1.png"))); + Assert.Empty(db.TileCacheMetadata.ToList()); + } + + [Fact] + public async Task RetrieveTileAsync_ReturnsThrottled_WhenBudgetExhausted() + { + using var dir = new TempDir(); + var db = CreateDbContext(); + var service = CreateService(db, dir.Path); + + // Drain all tokens and stop replenishment — next AcquireAsync returns false. + TileCacheService.OutboundBudget.DrainForTesting(); + + // RetrieveTileAsync should signal budget exhaustion (BudgetExhausted = true). + var result = await service.RetrieveTileAsync("10", "1", "1", "http://tiles/10/1/1.png"); + + Assert.True(result.BudgetExhausted); + Assert.Null(result.TileData); + } + /// /// Creates a TileCacheService with a properly configured HttpClient. /// Mirrors the User-Agent, Timeout, and TryParseAdd fallback logic from the diff --git a/wwwroot/js/Areas/Manager/Groups/Map.js b/wwwroot/js/Areas/Manager/Groups/Map.js index cdd78633..48858f1e 100644 --- a/wwwroot/js/Areas/Manager/Groups/Map.js +++ b/wwwroot/js/Areas/Manager/Groups/Map.js @@ -1,4 +1,5 @@ import { addZoomLevelControl } from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, currentDateInputValue, @@ -8,11 +9,6 @@ import { getViewerTimeZone, } from '../../../util/datetime.js'; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; - (() => { const viewerTimeZone = getViewerTimeZone(); const getLocationSourceTimeZone = location => location?.timezone || location?.timeZoneId || location?.timeZone || null; @@ -38,7 +34,7 @@ const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contri if (!mapEl || !groupId) return; const map = L.map('groupMap').setView([0, 0], 2); - L.tileLayer(tilesUrl, { maxZoom: 19, attribution: tilesAttribution }).addTo(map); + createTileLayer().addTo(map); if (map.attributionControl && typeof map.attributionControl.setPrefix === 'function') { map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); } diff --git a/wwwroot/js/Areas/Public/UsersTimeline/Embed.js b/wwwroot/js/Areas/Public/UsersTimeline/Embed.js index 19e1520e..890fe96b 100644 --- a/wwwroot/js/Areas/Public/UsersTimeline/Embed.js +++ b/wwwroot/js/Areas/Public/UsersTimeline/Embed.js @@ -5,10 +5,6 @@ let mapBounds = null; let markerClusterGroup = null; let username = null; let markerLayer, clusterLayer, highlightLayer; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; let timelineLive; let stream; let markerTransitionTimer = null; // Timer for live-to-latest marker transition @@ -35,6 +31,7 @@ const getLocationTimestampInfo = location => formatViewerAndSourceTimes({ }); import {addZoomLevelControl, latestLocationMarker, liveMarker} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, formatDate, @@ -121,10 +118,7 @@ const initializeMap = () => { scrollWheelZoom: false, zoomAnimation: true }).setView(initialCenter, zoomLevel); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); diff --git a/wwwroot/js/Areas/Public/UsersTimeline/Timeline.js b/wwwroot/js/Areas/Public/UsersTimeline/Timeline.js index 30221d83..d0532bbe 100644 --- a/wwwroot/js/Areas/Public/UsersTimeline/Timeline.js +++ b/wwwroot/js/Areas/Public/UsersTimeline/Timeline.js @@ -3,10 +3,6 @@ let mapContainer = null; let zoomLevel = 3; let mapBounds = null; let username = null; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; let timelineLive; let markerLayer, clusterLayer, highlightLayer; let stream; @@ -33,6 +29,7 @@ const getLocationTimestampInfo = location => formatViewerAndSourceTimes({ }); import {addZoomLevelControl, latestLocationMarker, liveMarker} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, formatDate, @@ -115,10 +112,7 @@ const initializeMap = () => { mapContainer = L.map('mapContainer', { zoomAnimation: true }).setView(initialCenter, zoomLevel); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); diff --git a/wwwroot/js/Areas/User/Groups/Map.js b/wwwroot/js/Areas/User/Groups/Map.js index bd4d7aea..feefb84a 100644 --- a/wwwroot/js/Areas/User/Groups/Map.js +++ b/wwwroot/js/Areas/User/Groups/Map.js @@ -1,4 +1,5 @@ import { addZoomLevelControl } from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, currentDateInputValue, @@ -8,11 +9,6 @@ import { getViewerTimeZone, } from '../../../util/datetime.js'; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; - (() => { const viewerTimeZone = getViewerTimeZone(); const getLocationSourceTimeZone = location => location?.timezone || location?.timeZoneId || location?.timeZone || null; @@ -38,7 +34,7 @@ const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contri if (!mapEl || !groupId) return; const map = L.map('groupMap').setView([0, 0], 2); - L.tileLayer(tilesUrl, { maxZoom: 19, attribution: tilesAttribution }).addTo(map); + createTileLayer().addTo(map); if (map.attributionControl && typeof map.attributionControl.setPrefix === 'function') { map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); } diff --git a/wwwroot/js/Areas/User/Location/Create.js b/wwwroot/js/Areas/User/Location/Create.js index ee39e19d..fc5a4cbc 100644 --- a/wwwroot/js/Areas/User/Location/Create.js +++ b/wwwroot/js/Areas/User/Location/Create.js @@ -1,7 +1,4 @@ -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; +import { createTileLayer } from '../../../retryTileLayer.js'; document.addEventListener("DOMContentLoaded", function () { @@ -12,9 +9,7 @@ document.addEventListener("DOMContentLoaded", function () { }).setView([51.505, -0.09], 2); // Default view (Global zoom) // Add the tile layer from the cache proxy. - L.tileLayer(tilesUrl, { - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); // Marker variable to store the placed marker var marker; diff --git a/wwwroot/js/Areas/User/Location/Edit.js b/wwwroot/js/Areas/User/Location/Edit.js index acd28584..8edf9efa 100644 --- a/wwwroot/js/Areas/User/Location/Edit.js +++ b/wwwroot/js/Areas/User/Location/Edit.js @@ -1,9 +1,7 @@ -let mapContainer = null; +import { createTileLayer } from '../../../retryTileLayer.js'; + +let mapContainer = null; let data = null; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; document.addEventListener("DOMContentLoaded", function () { @@ -17,10 +15,7 @@ document.addEventListener("DOMContentLoaded", function () { const mapContainer = L.map('mapContainer').setView([latitude, longitude], 13); // Center map at the location // Add the tile layer from the cache proxy. - L.tileLayer(tilesUrl, { - attribution: tilesAttribution, - zoomAnimation: true - }).addTo(mapContainer); + createTileLayer({ zoomAnimation: true }).addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); // Marker variable to store the placed marker var marker; diff --git a/wwwroot/js/Areas/User/Location/Index.js b/wwwroot/js/Areas/User/Location/Index.js index 4ff7ffab..1e70aa54 100644 --- a/wwwroot/js/Areas/User/Location/Index.js +++ b/wwwroot/js/Areas/User/Location/Index.js @@ -6,11 +6,8 @@ let markerClusterGroup = null; let isSearchPanelOpen = false; let markerLayer, clusterLayer, highlightLayer; let markerTransitionTimer = null; // Timer for live-to-latest marker transition -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; import {addZoomLevelControl, latestLocationMarker, liveMarker} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, formatDate, @@ -230,10 +227,7 @@ const initializeMap = () => { mapContainer = L.map('mapContainer', { zoomAnimation: true }).setView(initialCenter, zoomLevel); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); diff --git a/wwwroot/js/Areas/User/Timeline/Chronological.js b/wwwroot/js/Areas/User/Timeline/Chronological.js index 6bae6d9c..3f8d79b1 100644 --- a/wwwroot/js/Areas/User/Timeline/Chronological.js +++ b/wwwroot/js/Areas/User/Timeline/Chronological.js @@ -4,11 +4,8 @@ let mapContainer = null; let markerLayer, clusterLayer, highlightLayer; let markerTransitionTimer = null; // Timer for live-to-latest marker transition let stream = null; // SSE stream for live updates -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; import {addZoomLevelControl, latestLocationMarker, liveMarker} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, formatDate, @@ -735,10 +732,7 @@ const initializeMap = () => { zoomAnimation: true }).setView([20, 0], 3); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); addZoomLevelControl(mapContainer); diff --git a/wwwroot/js/Areas/User/Timeline/Index.js b/wwwroot/js/Areas/User/Timeline/Index.js index bc078f1f..d16daa75 100644 --- a/wwwroot/js/Areas/User/Timeline/Index.js +++ b/wwwroot/js/Areas/User/Timeline/Index.js @@ -5,11 +5,8 @@ let mapBounds = null; let markerLayer, clusterLayer, highlightLayer; let stream; let markerTransitionTimer = null; // Timer for live-to-latest marker transition -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; import {addZoomLevelControl, latestLocationMarker, liveMarker} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import { formatViewerAndSourceTimes, formatDate, @@ -165,9 +162,7 @@ const initializeMap = () => { mapContainer = L.map('mapContainer', { zoomAnimation: true }).setView(initialCenter, zoomLevel); - L.tileLayer(tilesUrl, { - maxZoom: 19, attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); diff --git a/wwwroot/js/Areas/User/Trip/Index.js b/wwwroot/js/Areas/User/Trip/Index.js index 48c27c9d..396227a5 100644 --- a/wwwroot/js/Areas/User/Trip/Index.js +++ b/wwwroot/js/Areas/User/Trip/Index.js @@ -4,6 +4,7 @@ import { generateWikipediaLinkHtml, initWikipediaPopovers, } from '../../../util/wikipedia-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; (() => { /* ------------------------------------------------ Pagination state */ @@ -1206,11 +1207,6 @@ import { let contextMarkersGroup = null; let mapResizeObserver = null; - // Map tiles config (proxy URL + attribution) injected by layout - const tilesConfig = window.wayfarerTileConfig || {}; - const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; - const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; - // Wayfarer icon dimensions const WF_WIDTH = 28; const WF_HEIGHT = 45; @@ -1256,10 +1252,7 @@ import { contextMap = L.map(container, { zoomAnimation: true }).setView([0, 0], 13); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(contextMap); + createTileLayer().addTo(contextMap); contextMap.attributionControl.setPrefix( '© Wayfarer | ' + diff --git a/wwwroot/js/Areas/User/Trip/mapManager.js b/wwwroot/js/Areas/User/Trip/mapManager.js index e8048836..4c71ce4d 100644 --- a/wwwroot/js/Areas/User/Trip/mapManager.js +++ b/wwwroot/js/Areas/User/Trip/mapManager.js @@ -1,15 +1,12 @@ // mapManager.js – refactored to use store import {addZoomLevelControl} from '../../../map-utils.js'; +import { createTileLayer } from '../../../retryTileLayer.js'; import {store} from './storeInstance.js'; import {buildPlacePopup, buildAreaPopup} from '../../../Trip/tripPopupBuilder.js'; /* ------------------------------------------------------------------ * * Private state * ------------------------------------------------------------------ */ -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; let mapContainer = null; let drawControl = null; let drawnLayerGroup = null; @@ -171,9 +168,7 @@ export const initAreaMap = (areaId, geometry, fillColor) => { // 2️⃣ Create the Leaflet map const map = L.map(container, {zoomAnimation: true}).setView([0, 0], 2); - L.tileLayer(tilesUrl, { - attribution: tilesAttribution - }).addTo(map); + createTileLayer().addTo(map); map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); @@ -429,10 +424,7 @@ export const initializeMap = (center = [20, 0], zoom = 3) => { editable: true }).setView(center, zoom); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(mapContainer); + createTileLayer().addTo(mapContainer); mapContainer.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); addZoomLevelControl(mapContainer); diff --git a/wwwroot/js/Areas/User/Visit/Edit.js b/wwwroot/js/Areas/User/Visit/Edit.js index 9ff5df31..53be552b 100644 --- a/wwwroot/js/Areas/User/Visit/Edit.js +++ b/wwwroot/js/Areas/User/Visit/Edit.js @@ -3,14 +3,12 @@ * Allows editing timestamps, location (via map click), appearance, and notes. */ +import { createTileLayer } from '../../../retryTileLayer.js'; + // Wayfarer marker icon dimensions (matches Trip mapManager) const WF_WIDTH = 28; const WF_HEIGHT = 45; const WF_ANCHOR = [WF_WIDTH / 2, WF_HEIGHT]; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; /** * Build PNG icon URL for wayfarer markers @@ -47,10 +45,7 @@ const initMap = () => { map = L.map('mapContainer').setView([lat, lon], 15); - L.tileLayer(tilesUrl, { - maxZoom: 19, - attribution: tilesAttribution - }).addTo(map); + createTileLayer().addTo(map); // Set standard attribution prefix (matches Timeline) map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); diff --git a/wwwroot/js/Trip/tripViewerHelpers.js b/wwwroot/js/Trip/tripViewerHelpers.js index 7d826791..b5285c7a 100644 --- a/wwwroot/js/Trip/tripViewerHelpers.js +++ b/wwwroot/js/Trip/tripViewerHelpers.js @@ -7,17 +7,13 @@ */ import {addZoomLevelControl} from '../map-utils.js'; +import {createTileLayer} from '../retryTileLayer.js'; import { buildPlacePopup, buildSegmentPopup, buildAreaPopup } from './tripPopupBuilder.js'; -// Map tiles config (proxy URL + attribution) injected by layout. -const tilesConfig = window.wayfarerTileConfig || {}; -const tilesUrl = tilesConfig.tilesUrl || `${window.location.origin}/Public/tiles/{z}/{x}/{y}.png`; -const tilesAttribution = tilesConfig.attribution || '© OpenStreetMap contributors'; - /* ---------- Wayfarer PNG marker URL ---------- */ const png = (icon, bg) => `/icons/wayfarer-map-icons/dist/png/marker/${bg}/${icon}.png`; @@ -48,9 +44,7 @@ export const initLeaflet = (center = [20, 0], zoom = 3) => { }).setView(center, zoom); /* keep a handle to the tile layer so we can attach events */ - const tiles = L.tileLayer(tilesUrl, { - maxZoom: 19, attribution: tilesAttribution - }).addTo(map); + const tiles = createTileLayer().addTo(map); map.attributionControl.setPrefix('© Wayfarer | Stef K | © Leaflet'); L.control.zoom({position: 'bottomright'}).addTo(map); addZoomLevelControl(map); /* ← your existing util */ diff --git a/wwwroot/js/retryTileLayer.js b/wwwroot/js/retryTileLayer.js new file mode 100644 index 00000000..b79cdd40 --- /dev/null +++ b/wwwroot/js/retryTileLayer.js @@ -0,0 +1,158 @@ +/** + * Custom Leaflet TileLayer that uses fetch() instead of .src for tile loading. + * This enables HTTP status code inspection so we can retry on 503 (budget exhaustion) + * while treating 404 as permanent failure. + * + * Retry strategy: + * - Only retries on HTTP 503 or network errors + * - Reads Retry-After header from server (falls back to exponential backoff) + * - Max 5 retries per tile, delay capped at 10 seconds + * - 404 and other status codes are NOT retried + * + * Design note: upstream HTTP 500/502/504 errors are treated as permanent failures + * (not retried). The 503 retry strategy specifically targets outbound budget exhaustion + * on our proxy. If upstream OSM is down, retrying would not help and would only pile + * up stale retry timers. Users will see gray tiles until upstream recovers. + */ +const RetryTileLayer = L.TileLayer.extend({ + options: { + maxRetries: 5, + retryDelayMs: 1000, + }, + + /** + * Override createTile to use fetch() with AbortController for HTTP status code access. + * An AbortController is stored on the tile element so _removeTile can cancel in-flight + * fetches and pending retry timers when tiles are panned/zoomed out of view. + * @param {Object} coords - Tile coordinates {x, y, z}. + * @param {Function} done - Callback to signal tile load complete. + * @returns {HTMLImageElement} The tile image element. + */ + createTile: function (coords, done) { + const tile = document.createElement('img'); + tile.alt = ''; + tile.setAttribute('role', 'presentation'); + + if (this.options.crossOrigin || this.options.crossOrigin === '') { + tile.crossOrigin = this.options.crossOrigin === true + ? '' : this.options.crossOrigin; + } + if (typeof this.options.referrerPolicy === 'string') { + tile.referrerPolicy = this.options.referrerPolicy; + } + + // AbortController lets _removeTile cancel in-flight fetch and pending retries. + const controller = new AbortController(); + tile._abortController = controller; + + const url = this.getTileUrl(coords); + this._fetchWithRetry(url, tile, done, 0, controller.signal); + return tile; + }, + + /** + * Override _removeTile to abort in-flight fetches and revoke blob URLs. + * Leaflet calls this when tiles are panned/zoomed out of view. Without this, + * fetch() continues running and retry timers keep firing for removed tiles. + * @param {string} key - Leaflet internal tile key. + */ + _removeTile: function (key) { + const tile = this._tiles[key]; + if (tile && tile.el) { + // Abort any in-flight fetch or pending retry for this tile. + if (tile.el._abortController) { + tile.el._abortController.abort(); + tile.el._abortController = null; + } + // Revoke blob URL to prevent memory leaks. Leaflet replaces onload/onerror + // with falseFn before removal, so our revocation callbacks would never fire. + if (tile.el.src && tile.el.src.startsWith('blob:')) { + URL.revokeObjectURL(tile.el.src); + } + } + L.TileLayer.prototype._removeTile.call(this, key); + }, + + /** + * Fetches a tile via fetch(), retries on 503 or network error with backoff. + * Respects the AbortSignal so removed tiles stop retrying immediately. + * @param {string} url - The tile URL. + * @param {HTMLImageElement} tile - The tile image element. + * @param {Function} done - Leaflet callback to signal completion. + * @param {number} attempt - Current retry attempt (0-based). + * @param {AbortSignal} signal - Abort signal from the tile's AbortController. + */ + _fetchWithRetry: function (url, tile, done, attempt, signal) { + const layer = this; + const maxRetries = this.options.maxRetries; + const baseDelay = this.options.retryDelayMs; + + fetch(url, { signal: signal }).then(function (response) { + if (response.ok) { + return response.blob().then(function (blob) { + tile.onload = function () { + URL.revokeObjectURL(tile.src); + done(null, tile); + }; + tile.onerror = function (e) { + URL.revokeObjectURL(tile.src); + done(e, tile); + }; + tile.src = URL.createObjectURL(blob); + }); + } + + // 503 = budget exhausted, transient — retry with Retry-After or backoff + if (response.status === 503 && attempt < maxRetries) { + const retryAfter = response.headers.get('Retry-After'); + const parsed = retryAfter ? parseInt(retryAfter, 10) : NaN; + let delayMs = !isNaN(parsed) + ? parsed * 1000 + : baseDelay * Math.pow(2, attempt); + delayMs = Math.min(delayMs, 10000); + + setTimeout(function () { + // Check if tile was removed while waiting — abort signal is set. + if (!signal.aborted) { + layer._fetchWithRetry(url, tile, done, attempt + 1, signal); + } + }, delayMs); + return; + } + + // Non-retryable (404, 400, 500, etc.) + done(new Error('Tile fetch failed: ' + response.status), tile); + }).catch(function (err) { + // Tile was removed (panned/zoomed away) — silently stop. + if (err.name === 'AbortError') return; + + // Network error — retry if attempts remain + if (attempt < maxRetries) { + const delayMs = Math.min(baseDelay * Math.pow(2, attempt), 10000); + setTimeout(function () { + if (!signal.aborted) { + layer._fetchWithRetry(url, tile, done, attempt + 1, signal); + } + }, delayMs); + return; + } + done(err, tile); + }); + } +}); + +/** + * Creates a tile layer with retry support using the app's tile proxy config. + * Reads URL and attribution from window.wayfarerTileConfig (injected by _Layout.cshtml). + * @param {Object} [opts] - Additional L.TileLayer options to merge (e.g., {zoomAnimation: true}). + * @returns {L.TileLayer} The tile layer instance (call .addTo(map) on the result). + */ +export const createTileLayer = (opts) => { + const config = window.wayfarerTileConfig || {}; + const url = config.tilesUrl || (window.location.origin + '/Public/tiles/{z}/{x}/{y}.png'); + const attribution = config.attribution || '\u00a9 OpenStreetMap contributors'; + return new RetryTileLayer(url, Object.assign({ + maxZoom: 19, + attribution: attribution, + }, opts || {})); +}; From 02611ae47b87b57bafd5f48c73c6c0df355ef69c Mon Sep 17 00:00:00 2001 From: Stef Kariotidis Date: Thu, 26 Mar 2026 19:04:02 +0200 Subject: [PATCH 2/2] Fix review findings: jitter, clamp, early return, concurrency guards MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add ±25% jitter to client-side retry backoff to prevent thundering herd - Clamp Retry-After floor to baseDelay, reject zero/negative values - Add explicit early return in CacheTileAsync for upstream HTTP failures - Pass cancellationToken to ReadAsByteArrayAsync and Task.Delay - Extract BudgetRetryAfterSeconds constant with doc linking to budget config - Replace tileData?.Length with tileData.Length (guaranteed non-null after guards) - Guard blob URL creation with signal.aborted check to prevent memory leak - Add [Collection("OutboundBudget")] to prevent parallel test interference - Add controller test for 503 + Retry-After on budget exhaustion --- Areas/Public/Controllers/TilesController.cs | 9 ++++- Services/TileCacheService.cs | 28 +++++++------ .../Controllers/TilesControllerTests.cs | 39 +++++++++++++++++++ .../Services/TileCacheServiceTests.cs | 6 +++ wwwroot/js/retryTileLayer.js | 24 +++++++++--- 5 files changed, 87 insertions(+), 19 deletions(-) diff --git a/Areas/Public/Controllers/TilesController.cs b/Areas/Public/Controllers/TilesController.cs index ff4044f2..4b6d09aa 100644 --- a/Areas/Public/Controllers/TilesController.cs +++ b/Areas/Public/Controllers/TilesController.cs @@ -27,6 +27,13 @@ public class TilesController : Controller /// private const int MaxZoomLevel = 22; + /// + /// Retry-After header value (in seconds) sent with HTTP 503 when the outbound budget is exhausted. + /// Set to 5s to align with : at 2 tokens/sec + /// (ReplenishIntervalMs=500) with BurstCapacity=10, a full burst refills in ~5 seconds. + /// + private const string BudgetRetryAfterSeconds = "5"; + /// /// Thread-safe dictionary for rate limiting anonymous tile requests by IP address. /// Uses atomic operations via to prevent race conditions. @@ -156,7 +163,7 @@ public async Task GetTile(int z, int x, int y) if (result.BudgetExhausted) { _logger.LogWarning("Tile budget exhausted for {Z}/{X}/{Y}", z, x, y); - Response.Headers["Retry-After"] = "5"; + Response.Headers["Retry-After"] = BudgetRetryAfterSeconds; return StatusCode(503, "Tile server busy. Please retry shortly."); } diff --git a/Services/TileCacheService.cs b/Services/TileCacheService.cs index 99c2df2d..f8d45238 100644 --- a/Services/TileCacheService.cs +++ b/Services/TileCacheService.cs @@ -768,7 +768,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string if (response.IsSuccessStatusCode) { - tileData = await response.Content.ReadAsByteArrayAsync(); + tileData = await response.Content.ReadAsByteArrayAsync(cancellationToken); // Extract cache headers from upstream response for conditional request support. etag = response.Headers.ETag?.Tag; @@ -822,18 +822,22 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string } // Optional: Delay between retries to avoid rate limiting - await Task.Delay(500); // 500ms delay between retries + await Task.Delay(500, cancellationToken); // 500ms delay between retries } // Budget was never acquired — signal throttling to caller. if (tileData == null && !budgetAcquired) return false; + // Upstream HTTP failure (all retries failed, but budget was acquired) — not budget-related, + // so return true to let the controller send 404 rather than 503. + if (tileData == null) + return true; + // For zoom levels >= DbMetadataZoomThreshold, store or update metadata in the database. - // Only store metadata when tile data was actually downloaded — budget exhaustion - // or HTTP failures leave tileData null, and writing a row with Size=0 and no file - // would create ghost metadata that pollutes LRU eviction. - if (tileData != null && zoom >= DbMetadataZoomThreshold) + // tileData is guaranteed non-null here — the null cases (budget exhaustion, HTTP failure) + // return early above. + if (zoom >= DbMetadataZoomThreshold) { var existingMetadata = await _dbContext.TileCacheMetadata .FirstOrDefaultAsync(t => t.Zoom == zoom && t.X == x && t.Y == y); @@ -841,7 +845,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string { // If adding a new tile would exceed the cache limit, evict tiles. // Coalesce: only one eviction runs at a time; concurrent callers skip. - if ((Interlocked.Read(ref _currentCacheSize) + (tileData?.Length ?? 0)) > (_maxCacheSizeInMB * 1024L * 1024L)) + if ((Interlocked.Read(ref _currentCacheSize) + tileData.Length) > (_maxCacheSizeInMB * 1024L * 1024L)) { if (Interlocked.CompareExchange(ref _evictionInProgress, 1, 0) == 0) { @@ -863,7 +867,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string Y = y, // Storing the coordinates as a point (update as needed). TileLocation = new Point(x, y), - Size = tileData?.Length ?? 0, + Size = tileData.Length, TileFilePath = tileFilePath, LastAccessed = DateTime.UtcNow, ETag = etag, @@ -876,7 +880,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string { _dbContext.TileCacheMetadata.Add(tileMetadata); await _dbContext.SaveChangesAsync(); - Interlocked.Add(ref _currentCacheSize, tileData?.Length ?? 0); + Interlocked.Add(ref _currentCacheSize, tileData.Length); _logger.LogInformation("Tile metadata stored in database."); } catch (DbUpdateException) @@ -894,7 +898,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string // Save the old size for cache size adjustment var oldSize = existingMetadata.Size; // Prepare new values - existingMetadata.Size = tileData?.Length ?? 0; + existingMetadata.Size = tileData.Length; existingMetadata.LastAccessed = DateTime.UtcNow; existingMetadata.ETag = etag; existingMetadata.LastModifiedUpstream = lastModifiedUpstream; @@ -931,7 +935,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string await entry.ReloadAsync(); existingMetadata = (TileCacheMetadata)entry.Entity; - existingMetadata.Size = tileData?.Length ?? 0; + existingMetadata.Size = tileData.Length; existingMetadata.LastAccessed = DateTime.UtcNow; existingMetadata.ETag = etag; existingMetadata.LastModifiedUpstream = lastModifiedUpstream; @@ -947,7 +951,7 @@ public async Task CacheTileAsync(string tileUrl, string zoomLevel, string } // Adjust the in-memory cache size using the previously saved value. - Interlocked.Add(ref _currentCacheSize, (tileData?.Length ?? 0) - oldSize); + Interlocked.Add(ref _currentCacheSize, tileData.Length - oldSize); } } diff --git a/tests/Wayfarer.Tests/Controllers/TilesControllerTests.cs b/tests/Wayfarer.Tests/Controllers/TilesControllerTests.cs index c9ffb40b..3b730a86 100644 --- a/tests/Wayfarer.Tests/Controllers/TilesControllerTests.cs +++ b/tests/Wayfarer.Tests/Controllers/TilesControllerTests.cs @@ -22,6 +22,12 @@ namespace Wayfarer.Tests.Controllers; /// /// Public tiles endpoint behavior. /// +/// +/// Shares the "OutboundBudget" collection with +/// to prevent parallel execution — both classes mutate +/// static state via DrainForTesting/ResetForTesting. +/// +[Collection("OutboundBudget")] public class TilesControllerTests : TestBase { /// @@ -388,6 +394,39 @@ public async Task GetTile_AuthenticatedUser_UsesUserIdNotIp() } } + [Fact] + public async Task GetTile_Returns503WithRetryAfter_WhenBudgetExhausted() + { + var cacheDir = Path.Combine(Path.GetTempPath(), Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(cacheDir); + var handler = new FakeHttpMessageHandler(HttpStatusCode.OK, new byte[] { 1, 2, 3 }); + var controller = BuildController(handler: handler, cacheDir: cacheDir); + controller.ControllerContext.HttpContext.Request.Headers["Referer"] = "http://example.com/page"; + + // Drain the outbound budget so the next tile fetch returns budget-exhausted. + TileCacheService.OutboundBudget.DrainForTesting(); + + try + { + // Use valid coordinates at zoom >= 9 (DB metadata threshold) + var result = await controller.GetTile(10, 100, 100); + + var statusResult = Assert.IsType(result); + Assert.Equal(503, statusResult.StatusCode); + Assert.Equal("Tile server busy. Please retry shortly.", statusResult.Value); + Assert.Equal("5", controller.Response.Headers["Retry-After"].ToString()); + } + finally + { + // Restore budget for other tests. + TileCacheService.OutboundBudget.ResetForTesting(); + if (Directory.Exists(cacheDir)) + { + Directory.Delete(cacheDir, true); + } + } + } + private TilesController BuildController(TileCacheService? tileService = null, ApplicationDbContext? dbContext = null!, string? cacheDir = null, HttpMessageHandler? handler = null, IApplicationSettingsService? settingsService = null) { dbContext ??= CreateDbContext(); diff --git a/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs b/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs index 44b2dcc4..08990561 100644 --- a/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs +++ b/tests/Wayfarer.Tests/Services/TileCacheServiceTests.cs @@ -19,6 +19,12 @@ namespace Wayfarer.Tests.Services; /// /// Tile cache behaviors: storing, retrieving, and purging cached tiles. /// +/// +/// Shares the "OutboundBudget" collection with +/// to prevent parallel execution — both classes mutate +/// static state via DrainForTesting/ResetForTesting. +/// +[Collection("OutboundBudget")] public class TileCacheServiceTests : TestBase { [Fact] diff --git a/wwwroot/js/retryTileLayer.js b/wwwroot/js/retryTileLayer.js index b79cdd40..9ffb9bbc 100644 --- a/wwwroot/js/retryTileLayer.js +++ b/wwwroot/js/retryTileLayer.js @@ -90,6 +90,12 @@ const RetryTileLayer = L.TileLayer.extend({ fetch(url, { signal: signal }).then(function (response) { if (response.ok) { return response.blob().then(function (blob) { + // Guard: if the tile was removed (panned/zoomed away) while the blob + // was being read, skip assigning the blob URL. Without this check, + // Leaflet's _removeTile would have already replaced onload/onerror + // with falseFn, so the revokeObjectURL callback would never fire, + // leaking the blob URL. + if (signal.aborted) return; tile.onload = function () { URL.revokeObjectURL(tile.src); done(null, tile); @@ -102,14 +108,17 @@ const RetryTileLayer = L.TileLayer.extend({ }); } - // 503 = budget exhausted, transient — retry with Retry-After or backoff + // 503 = budget exhausted, transient — retry with Retry-After or backoff. + // Jitter (±25%) prevents thundering-herd retries when many tiles 503 simultaneously. if (response.status === 503 && attempt < maxRetries) { const retryAfter = response.headers.get('Retry-After'); const parsed = retryAfter ? parseInt(retryAfter, 10) : NaN; - let delayMs = !isNaN(parsed) + let delayMs = !isNaN(parsed) && parsed > 0 ? parsed * 1000 : baseDelay * Math.pow(2, attempt); - delayMs = Math.min(delayMs, 10000); + delayMs = Math.max(delayMs, baseDelay); // floor: never below base delay + delayMs = Math.min(delayMs, 10000); // cap: never above 10s + delayMs *= (0.75 + Math.random() * 0.5); // jitter ±25% setTimeout(function () { // Check if tile was removed while waiting — abort signal is set. @@ -126,9 +135,10 @@ const RetryTileLayer = L.TileLayer.extend({ // Tile was removed (panned/zoomed away) — silently stop. if (err.name === 'AbortError') return; - // Network error — retry if attempts remain + // Network error (or body-read failure mid-transfer) — retry if attempts remain if (attempt < maxRetries) { - const delayMs = Math.min(baseDelay * Math.pow(2, attempt), 10000); + let delayMs = Math.min(baseDelay * Math.pow(2, attempt), 10000); + delayMs *= (0.75 + Math.random() * 0.5); // jitter ±25% setTimeout(function () { if (!signal.aborted) { layer._fetchWithRetry(url, tile, done, attempt + 1, signal); @@ -144,7 +154,9 @@ const RetryTileLayer = L.TileLayer.extend({ /** * Creates a tile layer with retry support using the app's tile proxy config. * Reads URL and attribution from window.wayfarerTileConfig (injected by _Layout.cshtml). - * @param {Object} [opts] - Additional L.TileLayer options to merge (e.g., {zoomAnimation: true}). + * @param {Object} [opts] - Additional L.TileLayer options to merge. Supports standard Leaflet + * options (e.g., {zoomAnimation: true}) plus retry tuning: maxRetries (default 5), + * retryDelayMs (default 1000). * @returns {L.TileLayer} The tile layer instance (call .addTo(map) on the result). */ export const createTileLayer = (opts) => {