From b30d76ddecc277d8830a67de1f1c808e80824f12 Mon Sep 17 00:00:00 2001 From: Alex Hope-O'Connor Date: Tue, 23 Dec 2025 21:03:21 +1000 Subject: [PATCH] Refactor configuration system and remove OpenAPI - Remove OpenAPI/Swagger support and all related code - Standardize configuration: remove hardcoded defaults, all defaults from appsettings.json - Configuration values throw exceptions if missing (except Debug:Enabled defaults to false) - Refactor CacheManagementService: extract ProcessLocationsAsync helper, remove duplicate code - Combine UpdateStaggerSeconds and LocationStaggerSeconds into single LocationStaggerSeconds config - Move config reads to constructors for better performance - Update README and docker-compose.yml to reflect configuration changes --- BomLocalService.csproj | 1 - Controllers/RadarController.cs | 39 ++++++- Program.cs | 55 +++------ README.md | 12 +- Services/BomRadarService.cs | 24 +++- Services/BrowserService.cs | 10 +- Services/CacheCleanupService.cs | 16 ++- Services/CacheManagementService.cs | 107 ++++++++++-------- Services/CacheService.cs | 8 +- Services/DebugService.cs | 23 ++-- .../Steps/Capture/CaptureFramesStep.cs | 33 +++++- .../Steps/Map/ResetToFirstFrameStep.cs | 7 +- .../Scraping/Steps/Map/WaitForMapReadyStep.cs | 7 +- .../Steps/Navigation/NavigateHomepageStep.cs | 3 +- .../Steps/Search/SelectSearchResultStep.cs | 7 +- .../Workflows/RadarScrapingWorkflow.cs | 16 ++- Services/TimeParsingService.cs | 12 +- Utilities/CacheHelper.cs | 24 +++- Utilities/FilePathHelper.cs | 6 +- appsettings.json | 1 - docker-compose.yml | 1 - 21 files changed, 265 insertions(+), 147 deletions(-) diff --git a/BomLocalService.csproj b/BomLocalService.csproj index 48826b1..084858f 100644 --- a/BomLocalService.csproj +++ b/BomLocalService.csproj @@ -7,7 +7,6 @@ - diff --git a/Controllers/RadarController.cs b/Controllers/RadarController.cs index 26078c9..e3fdd19 100644 --- a/Controllers/RadarController.cs +++ b/Controllers/RadarController.cs @@ -60,8 +60,19 @@ public class RadarController : ControllerBase { // Get cache status to include in 404 response // Use cache service directly since this is cache management, not radar-specific - var cacheExpirationMinutes = (int)_configuration.GetValue("CacheExpirationMinutes", 15.5); - var cacheManagementCheckIntervalMinutes = _configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + var cacheExpirationMinutesConfig = _configuration.GetValue("CacheExpirationMinutes"); + if (!cacheExpirationMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheExpirationMinutes configuration is required. Set it in appsettings.json or via CACHEEXPIRATIONMINUTES environment variable."); + } + var cacheExpirationMinutes = (int)cacheExpirationMinutesConfig.Value; + + var cacheManagementCheckIntervalMinutesConfig = _configuration.GetValue("CacheManagement:CheckIntervalMinutes"); + if (!cacheManagementCheckIntervalMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:CheckIntervalMinutes configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__CHECKINTERVALMINUTES environment variable."); + } + var cacheManagementCheckIntervalMinutes = cacheManagementCheckIntervalMinutesConfig.Value; var cacheStatus = await _cacheService.GetCacheStatusAsync( suburb, state, @@ -288,7 +299,14 @@ public class RadarController : ControllerBase // Validate time range size to prevent excessive data loading // Default: base limit on cache retention, but allow override via config - var cacheRetentionHours = _configuration.GetValue("CacheRetentionHours", 24); + var cacheRetentionHoursConfig = _configuration.GetValue("CacheRetentionHours"); + if (!cacheRetentionHoursConfig.HasValue) + { + throw new InvalidOperationException("CacheRetentionHours configuration is required. Set it in appsettings.json or via CACHERETENTIONHOURS environment variable."); + } + var cacheRetentionHours = cacheRetentionHoursConfig.Value; + + // TimeSeries:MaxTimeRangeHours is optional (nullable) var configuredMaxHours = _configuration.GetValue("TimeSeries:MaxTimeRangeHours"); // Use configured value if set, otherwise use cache retention (with minimum of 24 hours) @@ -331,8 +349,19 @@ public class RadarController : ControllerBase }); // Get cache status to include in 404 response - var cacheExpirationMinutes = (int)_configuration.GetValue("CacheExpirationMinutes", 15.5); - var cacheManagementCheckIntervalMinutes = _configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + var cacheExpirationMinutesConfig = _configuration.GetValue("CacheExpirationMinutes"); + if (!cacheExpirationMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheExpirationMinutes configuration is required. Set it in appsettings.json or via CACHEEXPIRATIONMINUTES environment variable."); + } + var cacheExpirationMinutes = (int)cacheExpirationMinutesConfig.Value; + + var cacheManagementCheckIntervalMinutesConfig = _configuration.GetValue("CacheManagement:CheckIntervalMinutes"); + if (!cacheManagementCheckIntervalMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:CheckIntervalMinutes configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__CHECKINTERVALMINUTES environment variable."); + } + var cacheManagementCheckIntervalMinutes = cacheManagementCheckIntervalMinutesConfig.Value; var cacheStatus = await _cacheService.GetCacheStatusAsync( suburb, state, diff --git a/Program.cs b/Program.cs index 966ed4b..008c276 100644 --- a/Program.cs +++ b/Program.cs @@ -14,39 +14,25 @@ var builder = WebApplication.CreateBuilder(args); // AddControllersWithViews includes AddControllers, so we use it for both MVC and API controllers builder.Services.AddControllersWithViews(); builder.Services.AddEndpointsApiExplorer(); -builder.Services.AddOpenApi(); builder.Services.AddHealthChecks(); // Configure CORS - MUST be added before other services -var corsOrigins = builder.Configuration.GetValue("Cors:AllowedOrigins", "*"); -var corsMethods = builder.Configuration.GetValue("Cors:AllowedMethods", "GET,POST,OPTIONS"); -var corsHeaders = builder.Configuration.GetValue("Cors:AllowedHeaders", "*"); -var corsAllowCredentials = builder.Configuration.GetValue("Cors:AllowCredentials", false); +// Configuration values come from appsettings.json (defaults) and can be overridden via environment variables +// Environment variables use double underscore for nested keys (e.g., CORS__ALLOWEDORIGINS) +var corsOrigins = builder.Configuration.GetValue("Cors:AllowedOrigins") + ?? throw new InvalidOperationException("Cors:AllowedOrigins configuration is required. Set it in appsettings.json or via CORS__ALLOWEDORIGINS environment variable."); +var corsMethods = builder.Configuration.GetValue("Cors:AllowedMethods") + ?? throw new InvalidOperationException("Cors:AllowedMethods configuration is required. Set it in appsettings.json or via CORS__ALLOWEDMETHODS environment variable."); +var corsHeaders = builder.Configuration.GetValue("Cors:AllowedHeaders") + ?? throw new InvalidOperationException("Cors:AllowedHeaders configuration is required. Set it in appsettings.json or via CORS__ALLOWEDHEADERS environment variable."); -// Support environment variable override (comma-separated for multiple origins) -var corsOriginsEnv = Environment.GetEnvironmentVariable("CORS__ALLOWEDORIGINS"); -if (!string.IsNullOrEmpty(corsOriginsEnv)) +// For bool, check if the key exists in configuration (GetValue returns false if not found, which is ambiguous) +var corsAllowCredentialsKey = builder.Configuration["Cors:AllowCredentials"]; +if (corsAllowCredentialsKey == null) { - corsOrigins = corsOriginsEnv; -} - -var corsMethodsEnv = Environment.GetEnvironmentVariable("CORS__ALLOWEDMETHODS"); -if (!string.IsNullOrEmpty(corsMethodsEnv)) -{ - corsMethods = corsMethodsEnv; -} - -var corsHeadersEnv = Environment.GetEnvironmentVariable("CORS__ALLOWEDHEADERS"); -if (!string.IsNullOrEmpty(corsHeadersEnv)) -{ - corsHeaders = corsHeadersEnv; -} - -var corsAllowCredentialsEnv = Environment.GetEnvironmentVariable("CORS__ALLOWCREDENTIALS"); -if (!string.IsNullOrEmpty(corsAllowCredentialsEnv) && bool.TryParse(corsAllowCredentialsEnv, out var parsedCredentials)) -{ - corsAllowCredentials = parsedCredentials; + throw new InvalidOperationException("Cors:AllowCredentials configuration is required. Set it in appsettings.json or via CORS__ALLOWCREDENTIALS environment variable."); } +var corsAllowCredentials = builder.Configuration.GetValue("Cors:AllowCredentials"); builder.Services.AddCors(options => { @@ -126,23 +112,12 @@ builder.Services.AddSingleton(); builder.Services.AddHostedService(); -// Add configuration - appsettings.json provides default values -// All values can be overridden via environment variables -builder.Configuration.AddJsonFile("appsettings.json", optional: false, reloadOnChange: true); - var app = builder.Build(); // Configure the HTTP request pipeline -if (app.Environment.IsDevelopment()) -{ - app.MapOpenApi(); -} - // HTTPS redirection is optional and disabled by default for Docker flexibility -// Users can enable it by setting ENABLE_HTTPS_REDIRECTION=true environment variable -// or EnableHttpsRedirection=true in appsettings.json -var enableHttpsRedirection = builder.Configuration.GetValue("EnableHttpsRedirection", false) || - Environment.GetEnvironmentVariable("ENABLE_HTTPS_REDIRECTION")?.Equals("true", StringComparison.OrdinalIgnoreCase) == true; +// Users can enable it by setting EnableHttpsRedirection=true in appsettings.json or ENABLEHTTPSREDIRECTION=true environment variable +var enableHttpsRedirection = builder.Configuration.GetValue("EnableHttpsRedirection", false); if (enableHttpsRedirection) { app.UseHttpsRedirection(); diff --git a/README.md b/README.md index 1afc742..e8164ba 100644 --- a/README.md +++ b/README.md @@ -218,7 +218,7 @@ All configuration can be done via environment variables, which override the defa |----------|-------------|---------| | `ASPNETCORE_ENVIRONMENT` | Runtime environment (Development/Production) | `Production` | | `ASPNETCORE_URLS` | URLs the service listens on | `http://+:8080` | -| `ENABLE_HTTPS_REDIRECTION` | Enable HTTPS redirection | `false` | +| `ENABLEHTTPSREDIRECTION` | Enable HTTPS redirection | `false` | #### Application Configuration @@ -235,8 +235,7 @@ All configuration can be done via environment variables, which override the defa |----------|-------------|---------|---------| | `CACHEMANAGEMENT__CHECKINTERVALMINUTES` | Interval between cache validity checks | `5` | `10` | | `CACHEMANAGEMENT__INITIALDELAYSECONDS` | Delay before first cache check on startup | `10` | `30` | -| `CACHEMANAGEMENT__UPDATESTAGGERSECONDS` | Delay between triggering cache updates | `2` | `5` | -| `CACHEMANAGEMENT__LOCATIONSTAGGERSECONDS` | Delay between processing different locations | `1` | `2` | +| `CACHEMANAGEMENT__LOCATIONSTAGGERSECONDS` | Delay between processing different locations (used for both initial and periodic updates) | `1` | `2` | #### Cache Cleanup @@ -880,13 +879,6 @@ DELETE /api/cache/{suburb}/{state} } ``` -### OpenAPI Documentation - -When running in Development mode, OpenAPI documentation is available at: -``` -http://localhost:8082/openapi/v1.json -``` - ## Cache Update Estimation The service uses a **metrics-based estimation system** to provide accurate estimates of cache update completion times. This ensures clients receive meaningful `nextUpdateTime` values that adapt to the actual hardware performance. diff --git a/Services/BomRadarService.cs b/Services/BomRadarService.cs index 34ebb81..6784d52 100644 --- a/Services/BomRadarService.cs +++ b/Services/BomRadarService.cs @@ -32,9 +32,27 @@ public class BomRadarService : IBomRadarService, IDisposable _scrapingService = scrapingService; _debugService = debugService; _configuration = configuration; - _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); - _cacheManagementCheckIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); - _timeSeriesWarningFolderCount = configuration.GetValue("TimeSeries:WarningFolderCount", 200); + + var cacheExpirationMinutesConfig = configuration.GetValue("CacheExpirationMinutes"); + if (!cacheExpirationMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheExpirationMinutes configuration is required. Set it in appsettings.json or via CACHEEXPIRATIONMINUTES environment variable."); + } + _cacheExpirationMinutes = cacheExpirationMinutesConfig.Value; + + var cacheManagementCheckIntervalMinutesConfig = configuration.GetValue("CacheManagement:CheckIntervalMinutes"); + if (!cacheManagementCheckIntervalMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:CheckIntervalMinutes configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__CHECKINTERVALMINUTES environment variable."); + } + _cacheManagementCheckIntervalMinutes = cacheManagementCheckIntervalMinutesConfig.Value; + + var timeSeriesWarningFolderCountConfig = configuration.GetValue("TimeSeries:WarningFolderCount"); + if (!timeSeriesWarningFolderCountConfig.HasValue) + { + throw new InvalidOperationException("TimeSeries:WarningFolderCount configuration is required. Set it in appsettings.json or via TIMESERIES__WARNINGFOLDERCOUNT environment variable."); + } + _timeSeriesWarningFolderCount = timeSeriesWarningFolderCountConfig.Value; // Calculate estimated cache update duration _estimatedUpdateDurationSeconds = CacheHelper.GetEstimatedUpdateDurationSeconds(configuration, CachedDataType.Radar); diff --git a/Services/BrowserService.cs b/Services/BrowserService.cs index 715ffca..e13d75c 100644 --- a/Services/BrowserService.cs +++ b/Services/BrowserService.cs @@ -15,7 +15,15 @@ public class BrowserService : IBrowserService public BrowserService(ILogger logger, IConfiguration configuration, IDebugService debugService) { _logger = logger; - _timezone = configuration.GetValue("Timezone") ?? "Australia/Brisbane"; + // Get timezone from configuration (default from appsettings.json, can be overridden via TIMEZONE environment variable) + var timezone = configuration.GetValue("Timezone"); + + if (string.IsNullOrEmpty(timezone)) + { + throw new InvalidOperationException("Timezone configuration is required. Set it in appsettings.json or via TIMEZONE environment variable."); + } + + _timezone = timezone; _debugService = debugService; } diff --git a/Services/CacheCleanupService.cs b/Services/CacheCleanupService.cs index 2b2d2e8..678ad37 100644 --- a/Services/CacheCleanupService.cs +++ b/Services/CacheCleanupService.cs @@ -14,8 +14,20 @@ public class CacheCleanupService : BackgroundService { _logger = logger; _cacheDirectory = FilePathHelper.GetCacheDirectory(configuration); - _retentionHours = configuration.GetValue("CacheRetentionHours", 24); - var cleanupIntervalHours = configuration.GetValue("CacheCleanup:IntervalHours", 1); + + var retentionHoursConfig = configuration.GetValue("CacheRetentionHours"); + if (!retentionHoursConfig.HasValue) + { + throw new InvalidOperationException("CacheRetentionHours configuration is required. Set it in appsettings.json or via CACHERETENTIONHOURS environment variable."); + } + _retentionHours = retentionHoursConfig.Value; + + var cleanupIntervalHoursConfig = configuration.GetValue("CacheCleanup:IntervalHours"); + if (!cleanupIntervalHoursConfig.HasValue) + { + throw new InvalidOperationException("CacheCleanup:IntervalHours configuration is required. Set it in appsettings.json or via CACHECLEANUP__INTERVALHOURS environment variable."); + } + var cleanupIntervalHours = cleanupIntervalHoursConfig.Value; _cleanupInterval = TimeSpan.FromHours(cleanupIntervalHours); } diff --git a/Services/CacheManagementService.cs b/Services/CacheManagementService.cs index 59392e5..851d767 100644 --- a/Services/CacheManagementService.cs +++ b/Services/CacheManagementService.cs @@ -11,6 +11,8 @@ public class CacheManagementService : BackgroundService private readonly IBrowserService _browserService; private readonly IConfiguration _configuration; private readonly TimeSpan _checkInterval; + private readonly TimeSpan _locationStaggerInterval; + private readonly TimeSpan _initialDelayInterval; private readonly HashSet _activeUpdates = new(); private readonly object _lock = new(); @@ -24,14 +26,34 @@ public class CacheManagementService : BackgroundService _bomRadarService = bomRadarService; _browserService = browserService; _configuration = configuration; - var checkIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + + var checkIntervalMinutesConfig = configuration.GetValue("CacheManagement:CheckIntervalMinutes"); + if (!checkIntervalMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:CheckIntervalMinutes configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__CHECKINTERVALMINUTES environment variable."); + } + var checkIntervalMinutes = checkIntervalMinutesConfig.Value; if (checkIntervalMinutes <= 0 || checkIntervalMinutes > 60) { throw new ArgumentException("CacheManagement:CheckIntervalMinutes must be between 1 and 60", nameof(configuration)); } - _checkInterval = TimeSpan.FromMinutes(checkIntervalMinutes); + + // LocationStaggerSeconds is used for delays between processing locations (both initial and periodic) + var locationStaggerSecondsConfig = configuration.GetValue("CacheManagement:LocationStaggerSeconds"); + if (!locationStaggerSecondsConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:LocationStaggerSeconds configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__LOCATIONSTAGGERSECONDS environment variable."); + } + _locationStaggerInterval = TimeSpan.FromSeconds(locationStaggerSecondsConfig.Value); + + var initialDelaySecondsConfig = configuration.GetValue("CacheManagement:InitialDelaySeconds"); + if (!initialDelaySecondsConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:InitialDelaySeconds configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__INITIALDELAYSECONDS environment variable."); + } + _initialDelayInterval = TimeSpan.FromSeconds(initialDelaySecondsConfig.Value); } protected override async Task ExecuteAsync(CancellationToken stoppingToken) @@ -39,8 +61,7 @@ public class CacheManagementService : BackgroundService _logger.LogInformation("Cache management service started. Check interval: {Interval}", _checkInterval); // Wait a bit for the service to fully initialize - var initialDelaySeconds = _configuration.GetValue("CacheManagement:InitialDelaySeconds", 10); - await Task.Delay(TimeSpan.FromSeconds(initialDelaySeconds), stoppingToken); + await Task.Delay(_initialDelayInterval, stoppingToken); // Pre-warm the browser before starting cache updates _logger.LogInformation("Pre-warming browser before cache updates"); @@ -58,28 +79,7 @@ public class CacheManagementService : BackgroundService // Initial cache update on startup _logger.LogInformation("Performing initial cache update for {Count} locations", locationsToManage.Count); - int initialUpdatesTriggered = 0; - int initialCachesValid = 0; - - foreach (var (suburb, state) in locationsToManage) - { - if (stoppingToken.IsCancellationRequested) - break; - - var (triggered, isValid) = await UpdateCacheIfNeededAsync(suburb, state, stoppingToken); - if (triggered) - { - initialUpdatesTriggered++; - } - else if (isValid) - { - initialCachesValid++; - } - - // Stagger updates to avoid overwhelming the system - var updateStaggerSeconds = _configuration.GetValue("CacheManagement:UpdateStaggerSeconds", 2); - await Task.Delay(TimeSpan.FromSeconds(updateStaggerSeconds), stoppingToken); - } + var (initialUpdatesTriggered, initialCachesValid) = await ProcessLocationsAsync(locationsToManage, stoppingToken); _logger.LogInformation("Initial cache update completed: {UpdatesTriggered} updates triggered, {CachesValid} caches valid", initialUpdatesTriggered, initialCachesValid); @@ -95,28 +95,7 @@ public class CacheManagementService : BackgroundService locationsToManage = GetLocationsToManage(); _logger.LogInformation("Starting periodic cache check for {Count} locations", locationsToManage.Count); - int updatesTriggered = 0; - int cachesValid = 0; - - foreach (var (suburb, state) in locationsToManage) - { - if (stoppingToken.IsCancellationRequested) - break; - - var (triggered, isValid) = await UpdateCacheIfNeededAsync(suburb, state, stoppingToken); - if (triggered) - { - updatesTriggered++; - } - else if (isValid) - { - cachesValid++; - } - - // Small delay between locations - var locationStaggerSeconds = _configuration.GetValue("CacheManagement:LocationStaggerSeconds", 1); - await Task.Delay(TimeSpan.FromSeconds(locationStaggerSeconds), stoppingToken); - } + var (updatesTriggered, cachesValid) = await ProcessLocationsAsync(locationsToManage, stoppingToken); _logger.LogInformation("Periodic cache check completed: {UpdatesTriggered} updates triggered, {CachesValid} caches valid", updatesTriggered, cachesValid); @@ -128,6 +107,38 @@ public class CacheManagementService : BackgroundService } } + /// + /// Processes a list of locations, checking and updating cache as needed + /// + private async Task<(int updatesTriggered, int cachesValid)> ProcessLocationsAsync( + List<(string suburb, string state)> locations, + CancellationToken cancellationToken) + { + int updatesTriggered = 0; + int cachesValid = 0; + + foreach (var (suburb, state) in locations) + { + if (cancellationToken.IsCancellationRequested) + break; + + var (triggered, isValid) = await UpdateCacheIfNeededAsync(suburb, state, cancellationToken); + if (triggered) + { + updatesTriggered++; + } + else if (isValid) + { + cachesValid++; + } + + // Stagger processing to avoid overwhelming the system + await Task.Delay(_locationStaggerInterval, cancellationToken); + } + + return (updatesTriggered, cachesValid); + } + private async Task<(bool updateTriggered, bool isValid)> UpdateCacheIfNeededAsync(string suburb, string state, CancellationToken cancellationToken) { var locationKey = LocationHelper.GetLocationKey(suburb, state); diff --git a/Services/CacheService.cs b/Services/CacheService.cs index 8f8a944..0d7f3be 100644 --- a/Services/CacheService.cs +++ b/Services/CacheService.cs @@ -27,7 +27,13 @@ public class CacheService : ICacheService _logger = logger; _configuration = configuration; _cacheDirectory = FilePathHelper.GetCacheDirectory(configuration); - _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); + + var cacheExpirationMinutesConfig = configuration.GetValue("CacheExpirationMinutes"); + if (!cacheExpirationMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheExpirationMinutes configuration is required. Set it in appsettings.json or via CACHEEXPIRATIONMINUTES environment variable."); + } + _cacheExpirationMinutes = cacheExpirationMinutesConfig.Value; if (_cacheExpirationMinutes <= 0) { diff --git a/Services/DebugService.cs b/Services/DebugService.cs index 238a434..f27b757 100644 --- a/Services/DebugService.cs +++ b/Services/DebugService.cs @@ -14,20 +14,17 @@ namespace BomLocalService.Services; public DebugService(ILogger logger, IConfiguration configuration) { _logger = logger; - _waitMs = configuration.GetValue("Debug:WaitMs", 2000); + + var waitMsConfig = configuration.GetValue("Debug:WaitMs"); + if (!waitMsConfig.HasValue) + { + throw new InvalidOperationException("Debug:WaitMs configuration is required. Set it in appsettings.json or via DEBUG__WAITMS environment variable."); + } + _waitMs = waitMsConfig.Value; - // Check all possible ways the config might be set - var debugEnabledEnv = Environment.GetEnvironmentVariable("DEBUG__ENABLED"); - var debugEnabledConfig = configuration.GetValue("Debug:Enabled", false); - var debugEnabledFromConfig = configuration["Debug:Enabled"]; - - _logger.LogInformation("Debug configuration check - Env var DEBUG__ENABLED: {EnvVar}, Config Debug:Enabled: {ConfigValue}, Config string: {ConfigString}", - debugEnabledEnv ?? "null", debugEnabledConfig, debugEnabledFromConfig ?? "null"); - - // Try environment variable first, then config - _enabled = !string.IsNullOrEmpty(debugEnabledEnv) - ? bool.TryParse(debugEnabledEnv, out var envBool) && envBool - : debugEnabledConfig; + // Get debug enabled from configuration (default from appsettings.json, can be overridden via DEBUG__ENABLED environment variable) + // Note: bool defaults to false if not found, which is acceptable for Debug:Enabled + _enabled = configuration.GetValue("Debug:Enabled", false); var cacheDirectory = FilePathHelper.GetCacheDirectory(configuration); _debugDirectory = Path.Combine(cacheDirectory, "debug"); diff --git a/Services/Scraping/Steps/Capture/CaptureFramesStep.cs b/Services/Scraping/Steps/Capture/CaptureFramesStep.cs index 68a8ffe..e4f95b7 100644 --- a/Services/Scraping/Steps/Capture/CaptureFramesStep.cs +++ b/Services/Scraping/Steps/Capture/CaptureFramesStep.cs @@ -25,15 +25,38 @@ public class CaptureFramesStep : BaseScrapingStep : base(logger, selectorService, debugService, configuration) { _cacheService = cacheService; - _tileRenderWaitMs = configuration.GetValue("Screenshot:TileRenderWaitMs", 5000); + + var tileRenderWaitMsConfig = configuration.GetValue("Screenshot:TileRenderWaitMs"); + if (!tileRenderWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:TileRenderWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__TILERENDERWAITMS environment variable."); + } + _tileRenderWaitMs = tileRenderWaitMsConfig.Value; var cropSection = configuration.GetSection("Screenshot:Crop"); + + var cropXConfig = cropSection.GetValue("X"); + if (!cropXConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:Crop:X configuration is required. Set it in appsettings.json or via SCREENSHOT__CROP__X environment variable."); + } + var cropYConfig = cropSection.GetValue("Y"); + if (!cropYConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:Crop:Y configuration is required. Set it in appsettings.json or via SCREENSHOT__CROP__Y environment variable."); + } + var cropRightOffsetConfig = cropSection.GetValue("RightOffset"); + if (!cropRightOffsetConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:Crop:RightOffset configuration is required. Set it in appsettings.json or via SCREENSHOT__CROP__RIGHTOFFSET environment variable."); + } + _cropConfig = new ScreenshotCropConfig { - X = cropSection.GetValue("X", 0), - Y = cropSection.GetValue("Y", 0), - RightOffset = cropSection.GetValue("RightOffset", 0), - Height = cropSection.GetValue("Height") + X = cropXConfig.Value, + Y = cropYConfig.Value, + RightOffset = cropRightOffsetConfig.Value, + Height = cropSection.GetValue("Height") // Height is optional (nullable) }; } diff --git a/Services/Scraping/Steps/Map/ResetToFirstFrameStep.cs b/Services/Scraping/Steps/Map/ResetToFirstFrameStep.cs index 1fa356c..9a99a48 100644 --- a/Services/Scraping/Steps/Map/ResetToFirstFrameStep.cs +++ b/Services/Scraping/Steps/Map/ResetToFirstFrameStep.cs @@ -86,7 +86,12 @@ public class ResetToFirstFrameStep : BaseScrapingStep await SaveDebugAsync(context, 10, "scrubber_at_position_0", cancellationToken); // Wait for frame 0 tiles to fully render - var tileRenderWaitMs = Configuration.GetValue("Screenshot:TileRenderWaitMs", 5000); + var tileRenderWaitMsConfig = Configuration.GetValue("Screenshot:TileRenderWaitMs"); + if (!tileRenderWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:TileRenderWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__TILERENDERWAITMS environment variable."); + } + var tileRenderWaitMs = tileRenderWaitMsConfig.Value; await context.Page.WaitForTimeoutAsync(tileRenderWaitMs); context.CurrentState = PageState.Frame0Selected; diff --git a/Services/Scraping/Steps/Map/WaitForMapReadyStep.cs b/Services/Scraping/Steps/Map/WaitForMapReadyStep.cs index d83a73d..6e77cda 100644 --- a/Services/Scraping/Steps/Map/WaitForMapReadyStep.cs +++ b/Services/Scraping/Steps/Map/WaitForMapReadyStep.cs @@ -18,7 +18,12 @@ public class WaitForMapReadyStep : BaseScrapingStep IConfiguration configuration) : base(logger, selectorService, debugService, configuration) { - _tileRenderWaitMs = configuration.GetValue("Screenshot:TileRenderWaitMs", 5000); + var tileRenderWaitMsConfig = configuration.GetValue("Screenshot:TileRenderWaitMs"); + if (!tileRenderWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:TileRenderWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__TILERENDERWAITMS environment variable."); + } + _tileRenderWaitMs = tileRenderWaitMsConfig.Value; } public override bool CanExecute(ScrapingContext context) diff --git a/Services/Scraping/Steps/Navigation/NavigateHomepageStep.cs b/Services/Scraping/Steps/Navigation/NavigateHomepageStep.cs index 85a36fa..a12fc20 100644 --- a/Services/Scraping/Steps/Navigation/NavigateHomepageStep.cs +++ b/Services/Scraping/Steps/Navigation/NavigateHomepageStep.cs @@ -18,7 +18,8 @@ public class NavigateHomepageStep : BaseScrapingStep IConfiguration configuration) : base(logger, selectorService, debugService, configuration) { - _baseUrl = configuration.GetValue("Scraping:BaseUrl") ?? "https://www.bom.gov.au/"; + _baseUrl = configuration.GetValue("Scraping:BaseUrl") + ?? throw new InvalidOperationException("Scraping:BaseUrl configuration is required. Set it in appsettings.json or via SCRAPING__BASEURL environment variable."); } public override bool CanExecute(ScrapingContext context) diff --git a/Services/Scraping/Steps/Search/SelectSearchResultStep.cs b/Services/Scraping/Steps/Search/SelectSearchResultStep.cs index 433d431..1733ec1 100644 --- a/Services/Scraping/Steps/Search/SelectSearchResultStep.cs +++ b/Services/Scraping/Steps/Search/SelectSearchResultStep.cs @@ -174,7 +174,12 @@ public class SelectSearchResultStep : BaseScrapingStep // Wait for forecast page to load await context.Page.WaitForLoadStateAsync(LoadState.DOMContentLoaded, new PageWaitForLoadStateOptions { Timeout = 15000 }); - var dynamicContentWaitMs = Configuration.GetValue("Screenshot:DynamicContentWaitMs", 2000); + var dynamicContentWaitMsConfig = Configuration.GetValue("Screenshot:DynamicContentWaitMs"); + if (!dynamicContentWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:DynamicContentWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__DYNAMICCONTENTWAITMS environment variable."); + } + var dynamicContentWaitMs = dynamicContentWaitMsConfig.Value; await context.Page.WaitForTimeoutAsync(dynamicContentWaitMs); await SaveDebugAsync(context, 5, "forecast_page_loaded", cancellationToken); diff --git a/Services/Scraping/Workflows/RadarScrapingWorkflow.cs b/Services/Scraping/Workflows/RadarScrapingWorkflow.cs index 4358746..42449bf 100644 --- a/Services/Scraping/Workflows/RadarScrapingWorkflow.cs +++ b/Services/Scraping/Workflows/RadarScrapingWorkflow.cs @@ -44,8 +44,20 @@ public class RadarScrapingWorkflow : IWorkflow _stepRegistry = stepRegistry; _configuration = configuration; _cacheService = cacheService; - _cacheExpirationMinutes = configuration.GetValue("CacheExpirationMinutes", 12.5); - _cacheManagementCheckIntervalMinutes = configuration.GetValue("CacheManagement:CheckIntervalMinutes", 5); + + var cacheExpirationMinutesConfig = configuration.GetValue("CacheExpirationMinutes"); + if (!cacheExpirationMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheExpirationMinutes configuration is required. Set it in appsettings.json or via CACHEEXPIRATIONMINUTES environment variable."); + } + _cacheExpirationMinutes = cacheExpirationMinutesConfig.Value; + + var cacheManagementCheckIntervalMinutesConfig = configuration.GetValue("CacheManagement:CheckIntervalMinutes"); + if (!cacheManagementCheckIntervalMinutesConfig.HasValue) + { + throw new InvalidOperationException("CacheManagement:CheckIntervalMinutes configuration is required. Set it in appsettings.json or via CACHEMANAGEMENT__CHECKINTERVALMINUTES environment variable."); + } + _cacheManagementCheckIntervalMinutes = cacheManagementCheckIntervalMinutesConfig.Value; } public async Task ExecuteAsync(ScrapingContext context, CancellationToken cancellationToken) diff --git a/Services/TimeParsingService.cs b/Services/TimeParsingService.cs index f1c99dc..508a9e6 100644 --- a/Services/TimeParsingService.cs +++ b/Services/TimeParsingService.cs @@ -14,7 +14,13 @@ public class TimeParsingService : ITimeParsingService public TimeParsingService(ILogger logger, IConfiguration configuration) { _logger = logger; - var timezone = configuration.GetValue("Timezone") ?? "Australia/Brisbane"; + // Get timezone from configuration (default from appsettings.json, can be overridden via TIMEZONE environment variable) + var timezone = configuration.GetValue("Timezone"); + + if (string.IsNullOrEmpty(timezone)) + { + throw new InvalidOperationException("Timezone configuration is required. Set it in appsettings.json or via TIMEZONE environment variable."); + } try { @@ -23,8 +29,8 @@ public class TimeParsingService : ITimeParsingService } catch (TimeZoneNotFoundException) { - _logger.LogWarning("Timezone {Timezone} not found, defaulting to Australia/Brisbane", timezone); - _timeZoneInfo = TimeZoneInfo.FindSystemTimeZoneById("Australia/Brisbane"); + _logger.LogError("Timezone {Timezone} not found. Please use a valid IANA timezone identifier.", timezone); + throw new InvalidOperationException($"Invalid timezone '{timezone}'. Please use a valid IANA timezone identifier (e.g., Australia/Sydney, Australia/Brisbane)."); } } diff --git a/Utilities/CacheHelper.cs b/Utilities/CacheHelper.cs index e6a6648..2395d27 100644 --- a/Utilities/CacheHelper.cs +++ b/Utilities/CacheHelper.cs @@ -14,8 +14,12 @@ public static class CacheHelper public static int GetFrameCountForDataType(IConfiguration configuration, CachedDataType dataType) { var dataTypeName = dataType.ToString(); - var frameCount = configuration.GetValue($"CachedDataTypes:{dataTypeName}:FrameCount", 7); - return frameCount; + var frameCountConfig = configuration.GetValue($"CachedDataTypes:{dataTypeName}:FrameCount"); + if (!frameCountConfig.HasValue) + { + throw new InvalidOperationException($"CachedDataTypes:{dataTypeName}:FrameCount configuration is required. Set it in appsettings.json or via CACHEDDATATYPES__{dataTypeName.ToUpperInvariant()}__FRAMECOUNT environment variable."); + } + return frameCountConfig.Value; } /// @@ -71,8 +75,20 @@ public static class CacheHelper { // Calculate based on actual wait times and frame count var frameCount = GetFrameCountForDataType(configuration, dataType); - var tileRenderWaitMs = configuration.GetValue("Screenshot:TileRenderWaitMs", 5000); - var dynamicContentWaitMs = configuration.GetValue("Screenshot:DynamicContentWaitMs", 2000); + + var tileRenderWaitMsConfig = configuration.GetValue("Screenshot:TileRenderWaitMs"); + if (!tileRenderWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:TileRenderWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__TILERENDERWAITMS environment variable."); + } + var tileRenderWaitMs = tileRenderWaitMsConfig.Value; + + var dynamicContentWaitMsConfig = configuration.GetValue("Screenshot:DynamicContentWaitMs"); + if (!dynamicContentWaitMsConfig.HasValue) + { + throw new InvalidOperationException("Screenshot:DynamicContentWaitMs configuration is required. Set it in appsettings.json or via SCREENSHOT__DYNAMICCONTENTWAITMS environment variable."); + } + var dynamicContentWaitMs = dynamicContentWaitMsConfig.Value; // Rough calculation: // - Initial page load and navigation: ~10-15 seconds diff --git a/Utilities/FilePathHelper.cs b/Utilities/FilePathHelper.cs index c43813f..c781510 100644 --- a/Utilities/FilePathHelper.cs +++ b/Utilities/FilePathHelper.cs @@ -5,12 +5,12 @@ namespace BomLocalService.Utilities; public static class FilePathHelper { /// - /// Gets the cache directory path from configuration or returns default + /// Gets the cache directory path from configuration (default from appsettings.json, can be overridden via CACHEDIRECTORY environment variable) /// public static string GetCacheDirectory(IConfiguration configuration) { - return configuration.GetValue("CacheDirectory") - ?? Path.Combine(AppContext.BaseDirectory, "cache"); + return configuration.GetValue("CacheDirectory") + ?? throw new InvalidOperationException("CacheDirectory configuration is required. Set it in appsettings.json or via CACHEDIRECTORY environment variable."); } /// diff --git a/appsettings.json b/appsettings.json index debaea5..fa748d0 100644 --- a/appsettings.json +++ b/appsettings.json @@ -18,7 +18,6 @@ "CacheManagement": { "CheckIntervalMinutes": 5, "InitialDelaySeconds": 10, - "UpdateStaggerSeconds": 2, "LocationStaggerSeconds": 1 }, "CacheCleanup": { diff --git a/docker-compose.yml b/docker-compose.yml index a3355d2..9289ef6 100644 --- a/docker-compose.yml +++ b/docker-compose.yml @@ -32,7 +32,6 @@ services: - CACHEEXPIRATIONMINUTES=${CACHEEXPIRATIONMINUTES:-12.5} - CACHEMANAGEMENT__CHECKINTERVALMINUTES=${CACHEMANAGEMENT__CHECKINTERVALMINUTES:-5} - CACHEMANAGEMENT__INITIALDELAYSECONDS=${CACHEMANAGEMENT__INITIALDELAYSECONDS:-10} - - CACHEMANAGEMENT__UPDATESTAGGERSECONDS=${CACHEMANAGEMENT__UPDATESTAGGERSECONDS:-2} - CACHEMANAGEMENT__LOCATIONSTAGGERSECONDS=${CACHEMANAGEMENT__LOCATIONSTAGGERSECONDS:-1} - CACHECLEANUP__INTERVALHOURS=${CACHECLEANUP__INTERVALHOURS:-1} - SCREENSHOT__DYNAMICCONTENTWAITMS=${SCREENSHOT__DYNAMICCONTENTWAITMS:-2000}