From d60706f62510748e67a8c3a02df183bb2235a1b6 Mon Sep 17 00:00:00 2001 From: Alex Hope-O'Connor Date: Tue, 16 Dec 2025 02:33:51 +1000 Subject: [PATCH] Fix: Improve cache stability and configuration consistency Active Folder Exclusion Fixes: - Exclude active cache folders in GetCachedFrameAsync, GetCachedFramesAsync, and GetAllCacheFoldersAsync - Prevents TaskCanceledException when reading metadata from folders being written to - Matches pattern used in GetCachedRadarAsync for consistency Cache Configuration Fixes: - Standardize default CacheExpirationMinutes to 12.5 (matches appsettings.json) - Add cache configuration to ScrapingService (removes hardcoded values) - Fix check interval calculation bug for intervals that don't divide 60 evenly - Fix NextUpdateTime calculation to account for background check intervals - Add configuration validation to all service constructors All fixes verified - service running stably with 0 exceptions in recent logs --- Services/BomRadarService.cs | 35 +++++++++++++++++++++++++----- Services/CacheManagementService.cs | 6 +++++ Services/CacheService.cs | 27 ++++++++++++++++++----- Services/ScrapingService.cs | 19 +++++++++++++--- Utilities/ResponseBuilder.cs | 29 +++++++++++++++++++++++-- 5 files changed, 100 insertions(+), 16 deletions(-) diff --git a/Services/BomRadarService.cs b/Services/BomRadarService.cs index 8b91fb2..9a3d100 100644 --- a/Services/BomRadarService.cs +++ b/Services/BomRadarService.cs @@ -30,8 +30,17 @@ public class BomRadarService : IBomRadarService, IDisposable _scrapingService = scrapingService; _debugService = debugService; _configuration = configuration; - _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 15.5); + _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); _cacheManagementCheckIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + + if (_cacheExpirationMinutes <= 0) + { + throw new ArgumentException("CacheExpirationMinutes must be greater than 0", nameof(configuration)); + } + if (_cacheManagementCheckIntervalMinutes <= 0 || _cacheManagementCheckIntervalMinutes > 60) + { + throw new ArgumentException("CacheManagement:CheckIntervalMinutes must be between 1 and 60", nameof(configuration)); + } } public async Task GetCachedRadarAsync(string suburb, string state, CancellationToken cancellationToken = default) @@ -275,11 +284,25 @@ public class BomRadarService : IBomRadarService, IDisposable // Remove from active tracking once complete _cacheService.ClearActiveCacheFolder(locationKey); - // Update result with cache state - result.IsUpdating = false; - result.CacheIsValid = true; - result.CacheExpiresAt = result.ObservationTime.AddMinutes(_cacheExpirationMinutes); - result.NextUpdateTime = result.CacheExpiresAt; + // Update result with cache state - rebuild response to get correct NextUpdateTime calculation + var cacheExpiresAt = result.ObservationTime.AddMinutes(_cacheExpirationMinutes); + var metadata = new LastUpdatedInfo + { + ObservationTime = result.ObservationTime, + ForecastTime = result.ForecastTime, + WeatherStation = result.WeatherStation, + Distance = result.Distance + }; + result = ResponseBuilder.CreateRadarResponse( + cacheFolderPath: newCacheFolderPath, + frames: result.Frames, + metadata: metadata, + suburb: suburb, + state: state, + cacheIsValid: true, + cacheExpiresAt: cacheExpiresAt, + isUpdating: false, + cacheManagementCheckIntervalMinutes: _cacheManagementCheckIntervalMinutes); return result; } diff --git a/Services/CacheManagementService.cs b/Services/CacheManagementService.cs index 4de50a8..59392e5 100644 --- a/Services/CacheManagementService.cs +++ b/Services/CacheManagementService.cs @@ -25,6 +25,12 @@ public class CacheManagementService : BackgroundService _browserService = browserService; _configuration = configuration; var checkIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + + if (checkIntervalMinutes <= 0 || checkIntervalMinutes > 60) + { + throw new ArgumentException("CacheManagement:CheckIntervalMinutes must be between 1 and 60", nameof(configuration)); + } + _checkInterval = TimeSpan.FromMinutes(checkIntervalMinutes); } diff --git a/Services/CacheService.cs b/Services/CacheService.cs index f1611e1..cb172fc 100644 --- a/Services/CacheService.cs +++ b/Services/CacheService.cs @@ -19,7 +19,12 @@ public class CacheService : ICacheService _logger = logger; _configuration = configuration; _cacheDirectory = FilePathHelper.GetCacheDirectory(configuration); - _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 15.5); + _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); + + if (_cacheExpirationMinutes <= 0) + { + throw new ArgumentException("CacheExpirationMinutes must be greater than 0", nameof(configuration)); + } // Ensure cache directory exists Directory.CreateDirectory(_cacheDirectory); @@ -90,7 +95,10 @@ public class CacheService : ICacheService string state, CancellationToken cancellationToken = default) { - var (cacheFolderPath, _) = await GetCachedScreenshotWithMetadataAsync(suburb, state, CachedDataType.Radar, null, cancellationToken); + // Exclude active cache folder (currently being written to) to avoid reading incomplete data + var locationKey = LocationHelper.GetLocationKey(suburb, state); + var excludeFolder = GetActiveCacheFolder(locationKey); + var (cacheFolderPath, _) = await GetCachedScreenshotWithMetadataAsync(suburb, state, CachedDataType.Radar, excludeFolder, cancellationToken); if (string.IsNullOrEmpty(cacheFolderPath) || !Directory.Exists(cacheFolderPath)) { @@ -322,6 +330,10 @@ public class CacheService : ICacheService string state, CancellationToken cancellationToken = default) { + // Exclude active cache folder (currently being written to) to avoid reading incomplete data + var locationKey = LocationHelper.GetLocationKey(suburb, state); + var excludeFolder = GetActiveCacheFolder(locationKey); + var pattern = FilePathHelper.GetCacheFolderPattern(suburb, state); var folders = Directory.GetDirectories(_cacheDirectory, pattern) .Select(f => @@ -342,6 +354,13 @@ public class CacheService : ICacheService if (!Directory.Exists(folder.Folder)) continue; + // Skip the folder if it's being excluded (currently being written to) + if (!string.IsNullOrEmpty(excludeFolder) && Path.GetFullPath(folder.Folder).Equals(Path.GetFullPath(excludeFolder), StringComparison.OrdinalIgnoreCase)) + { + _logger.LogDebug("Skipping excluded cache folder (being written to): {Folder}", folder.Folder); + continue; + } + var metadata = await LoadMetadataAsync(folder.Folder, cancellationToken); if (metadata == null) continue; @@ -621,9 +640,7 @@ public class CacheService : ICacheService { // Cache is invalid/missing - next update would be after background service check status.Message = status.CacheExists ? "Cache is stale" : "No cache exists"; - var now = DateTime.UtcNow; - var minutesUntilNextCheck = cacheManagementCheckIntervalMinutes - (now.Minute % cacheManagementCheckIntervalMinutes); - status.NextUpdateTime = now.AddMinutes(minutesUntilNextCheck); + status.NextUpdateTime = ResponseBuilder.CalculateNextServiceCheck(cacheManagementCheckIntervalMinutes); } return status; diff --git a/Services/ScrapingService.cs b/Services/ScrapingService.cs index a83128e..99ad2de 100644 --- a/Services/ScrapingService.cs +++ b/Services/ScrapingService.cs @@ -15,6 +15,8 @@ public class ScrapingService : IScrapingService private readonly int _dynamicContentWaitMs; private readonly int _tileRenderWaitMs; private readonly ScreenshotCropConfig _cropConfig; + private readonly double _cacheExpirationMinutes; + private readonly int _cacheManagementCheckIntervalMinutes; // Selector constants private static readonly string[] SearchButtonSelectors = new[] @@ -67,6 +69,18 @@ public class ScrapingService : IScrapingService _logger.LogInformation("Screenshot crop config: X={X}, Y={Y}, RightOffset={RightOffset}, Height={Height}", _cropConfig.X, _cropConfig.Y, _cropConfig.RightOffset, _cropConfig.Height); + + _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); + _cacheManagementCheckIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + + if (_cacheExpirationMinutes <= 0) + { + throw new ArgumentException("CacheExpirationMinutes must be greater than 0", nameof(configuration)); + } + if (_cacheManagementCheckIntervalMinutes <= 0 || _cacheManagementCheckIntervalMinutes > 60) + { + throw new ArgumentException("CacheManagement:CheckIntervalMinutes must be between 1 and 60", nameof(configuration)); + } } /// @@ -544,9 +558,8 @@ public class ScrapingService : IScrapingService await _cacheService.SaveFramesMetadataAsync(cacheFolderPath, CachedDataType.Radar, frames, cancellationToken); // Step 23: Return response with all frames - // Note: ScrapingService doesn't have access to cache management check interval - // Default to 5 minutes (standard check interval) - return ResponseBuilder.CreateRadarResponse(cacheFolderPath, frames, lastUpdatedInfo, suburb, state, cacheIsValid: true, cacheExpiresAt: null, isUpdating: false, cacheManagementCheckIntervalMinutes: 5); + var cacheExpiresAt = lastUpdatedInfo.ObservationTime.AddMinutes(_cacheExpirationMinutes); + return ResponseBuilder.CreateRadarResponse(cacheFolderPath, frames, lastUpdatedInfo, suburb, state, cacheIsValid: true, cacheExpiresAt: cacheExpiresAt, isUpdating: false, cacheManagementCheckIntervalMinutes: _cacheManagementCheckIntervalMinutes); } catch (Exception ex) { diff --git a/Utilities/ResponseBuilder.cs b/Utilities/ResponseBuilder.cs index b5d7c18..3906406 100644 --- a/Utilities/ResponseBuilder.cs +++ b/Utilities/ResponseBuilder.cs @@ -4,6 +4,32 @@ namespace BomLocalService.Utilities; public static class ResponseBuilder { + /// + /// Calculates the next time the background cache management service will check for updates. + /// Rounds up to the next interval boundary within the current hour, or wraps to next hour if needed. + /// + public static DateTime CalculateNextServiceCheck(int checkIntervalMinutes) + { + var now = DateTime.UtcNow; + var currentMinute = now.Minute; + + // Calculate how many complete intervals have elapsed this hour + var intervalsElapsed = currentMinute / checkIntervalMinutes; + + // Next interval starts at (intervalsElapsed + 1) * checkIntervalMinutes + var nextIntervalStartMinute = (intervalsElapsed + 1) * checkIntervalMinutes; + + // If next interval is beyond 60 minutes, wrap to next hour + if (nextIntervalStartMinute >= 60) + { + // Next check is at the start of next hour (00 minutes) + return now.Date.AddHours(now.Hour + 1).AddMinutes(0); + } + + // Next check is within current hour + return now.Date.AddHours(now.Hour).AddMinutes(nextIntervalStartMinute); + } + /// /// Creates a RadarResponse from a cache folder path, frames, and metadata /// @@ -44,8 +70,7 @@ public static class ResponseBuilder // Calculate next background service check time (rounds up to next check interval) var now = DateTime.UtcNow; - var minutesUntilNextCheck = cacheManagementCheckIntervalMinutes - (now.Minute % cacheManagementCheckIntervalMinutes); - var nextServiceCheck = now.AddMinutes(minutesUntilNextCheck); + var nextServiceCheck = CalculateNextServiceCheck(cacheManagementCheckIntervalMinutes); if (isUpdating) {