From ffd3636d1ccdfabb7b70ee92efa9fc33f13f7982 Mon Sep 17 00:00:00 2001 From: Alex Hope-O'Connor Date: Tue, 23 Dec 2025 21:52:42 +1000 Subject: [PATCH] Remove minutes ago parsing, use UTC timestamps directly - Refactor TimeParsingService to parse time strings to UTC only - Remove minutesAgo from RadarFrame and FrameMetadata models - Update CaptureFramesStep to extract UTC timestamps directly - Update CacheService to store/load ObservationTime instead of MinutesAgo - Update BomRadarService to use AbsoluteObservationTime directly - Remove all backward compatibility code for old cache folders - Update documentation and test SPA to calculate minutes ago on client - Fix step registration to register concrete types for DI resolution - Fix hosted service registration to use reflection correctly --- Extensions/ServiceCollectionExtensions.cs | 259 ++++++++++++++++++ Models/FrameMetadata.cs | 5 +- Models/RadarFrame.cs | 12 +- Models/RadarResponse.cs | 3 +- Program.cs | 122 +-------- README.md | 13 +- Services/BomRadarService.cs | 29 +- Services/CacheCleanupService.cs | 3 +- Services/CacheManagementService.cs | 3 +- Services/CacheService.cs | 21 +- Services/Interfaces/IBomRadarService.cs | 3 +- Services/Interfaces/IBrowserService.cs | 3 +- Services/Interfaces/ICacheService.cs | 5 +- Services/Interfaces/IDebugService.cs | 3 +- Services/Interfaces/IScrapingService.cs | 3 +- Services/Interfaces/ISelectorService.cs | 3 +- Services/Interfaces/ITimeParsingService.cs | 3 +- .../Registration/IServiceRegistration.cs | 30 ++ Services/Scraping/IScrapingStep.cs | 5 +- Services/Scraping/IScrapingStepRegistry.cs | 4 +- Services/Scraping/IWorkflowFactory.cs | 4 +- Services/Scraping/ScrapingStepRegistry.cs | 28 +- .../Steps/Capture/CaptureFramesStep.cs | 164 ++++++----- Services/Scraping/WorkflowFactory.cs | 8 +- .../Workflows/IRadarScrapingWorkflow.cs | 12 + .../Workflows/ITemperatureMapWorkflow.cs | 12 + .../Workflows/RadarScrapingWorkflow.cs | 2 +- .../Workflows/TemperatureMapWorkflow.cs | 2 +- Services/TimeParsingService.cs | 129 ++++----- Utilities/ResponseBuilder.cs | 13 +- Views/RadarTest/Index.cshtml | 64 +++-- 31 files changed, 598 insertions(+), 372 deletions(-) create mode 100644 Extensions/ServiceCollectionExtensions.cs create mode 100644 Services/Interfaces/Registration/IServiceRegistration.cs create mode 100644 Services/Scraping/Workflows/IRadarScrapingWorkflow.cs create mode 100644 Services/Scraping/Workflows/ITemperatureMapWorkflow.cs diff --git a/Extensions/ServiceCollectionExtensions.cs b/Extensions/ServiceCollectionExtensions.cs new file mode 100644 index 0000000..270d090 --- /dev/null +++ b/Extensions/ServiceCollectionExtensions.cs @@ -0,0 +1,259 @@ +using BomLocalService.Services.Interfaces; +using BomLocalService.Services.Interfaces.Registration; +using BomLocalService.Services.Scraping; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.DependencyInjection; +using System.Reflection; + +namespace BomLocalService.Extensions; + +public static class ServiceCollectionExtensions +{ + /// + /// Configures CORS with settings from configuration. + /// 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). + /// + public static IServiceCollection AddCorsConfiguration(this IServiceCollection services, IConfiguration configuration) + { + var corsOrigins = 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 = 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 = configuration.GetValue("Cors:AllowedHeaders") + ?? throw new InvalidOperationException("Cors:AllowedHeaders configuration is required. Set it in appsettings.json or via CORS__ALLOWEDHEADERS environment variable."); + + // For bool, check if the key exists in configuration (GetValue returns false if not found, which is ambiguous) + var corsAllowCredentialsKey = configuration["Cors:AllowCredentials"]; + if (corsAllowCredentialsKey == null) + { + throw new InvalidOperationException("Cors:AllowCredentials configuration is required. Set it in appsettings.json or via CORS__ALLOWCREDENTIALS environment variable."); + } + var corsAllowCredentials = configuration.GetValue("Cors:AllowCredentials"); + + services.AddCors(options => + { + options.AddDefaultPolicy(policy => + { + if (corsOrigins == "*") + { + policy.AllowAnyOrigin(); + } + else + { + // Split comma-separated origins + var origins = corsOrigins.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + policy.WithOrigins(origins); + } + + // Split comma-separated methods + var methods = corsMethods.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + policy.WithMethods(methods); + + // Split comma-separated headers or allow all + if (corsHeaders == "*") + { + policy.AllowAnyHeader(); + } + else + { + var headers = corsHeaders.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); + policy.WithHeaders(headers); + } + + if (corsAllowCredentials) + { + policy.AllowCredentials(); + } + }); + }); + + return services; + } + + /// + /// Scans the assembly for services implementing registration interfaces and registers them automatically. + /// Services are registered as their concrete interface (that inherits from registration interface). + /// Properly handles generic interfaces by using concrete interfaces that hide the generics. + /// + public static IServiceCollection ScanAndRegisterServices(this IServiceCollection services, Assembly assembly) + { + var registeredTypes = new HashSet(); // Track registered types to avoid duplicates + + // Get all types from the assembly with improved filtering + var types = GetRegisterableTypes(assembly); + + // Register Singleton services + RegisterServicesByLifetime( + services, + types, + registeredTypes); + + // Register Scoped services + RegisterServicesByLifetime( + services, + types, + registeredTypes); + + // Register Transient services + RegisterServicesByLifetime( + services, + types, + registeredTypes); + + // Register Hosted Services + RegisterHostedServices(services, types, registeredTypes); + + return services; + } + + /// + /// Gets all types that should be considered for registration. + /// Filters out abstract classes, interfaces, generic type definitions, nested private types, + /// compiler-generated types, and types from excluded namespaces. + /// + private static List GetRegisterableTypes(Assembly assembly) + { + return assembly.GetTypes() + .Where(t => + // Must be a class (not interface, struct, enum, etc.) + t.IsClass + // Must be concrete (not abstract) + && !t.IsAbstract + // Must not be a generic type definition (but closed generics are OK) + && !t.IsGenericTypeDefinition + // Must be public (or nested public in a public type) + && (t.IsPublic || (t.IsNestedPublic && t.DeclaringType?.IsPublic == true)) + // Must not be compiler-generated (e.g., async state machines, iterator classes) + && !t.IsDefined(typeof(System.Runtime.CompilerServices.CompilerGeneratedAttribute), inherit: false) + // Must not be a nested private/internal type + && !(t.IsNested && !t.IsNestedPublic) + // Exclude test namespaces if any (optional - adjust as needed) + && !t.Namespace?.StartsWith("BomLocalService.Tests", StringComparison.Ordinal) == true + ) + .ToList(); + } + + private static void RegisterServicesByLifetime( + IServiceCollection services, + List types, + HashSet registeredTypes) + where TRegistrationInterface : class + { + foreach (var implementationType in types) + { + // Skip if already registered + if (registeredTypes.Contains(implementationType)) + continue; + + // Get all interfaces implemented by this type + var allInterfaces = implementationType.GetInterfaces().ToList(); + + // Find interfaces that inherit from TRegistrationInterface (but not the registration interface itself) + var registrationInterfaces = allInterfaces + .Where(i => typeof(TRegistrationInterface).IsAssignableFrom(i) + && i != typeof(TRegistrationInterface) + && !IsGenericTypeDefinition(i)) + .ToList(); + + if (registrationInterfaces.Count == 0) + continue; + + // For each registration interface, find the most specific concrete interface + // Priority: concrete interfaces (like IRadarScrapingWorkflow) over generic interfaces (like IWorkflow) + foreach (var registrationInterface in registrationInterfaces) + { + // Skip if this is a generic interface definition (we prefer concrete interfaces) + if (registrationInterface.IsGenericTypeDefinition) + continue; + + // Check if this interface is already registered to avoid duplicates + var alreadyRegistered = services.Any(s => + s.ServiceType == registrationInterface && + s.ImplementationType == implementationType); + + if (alreadyRegistered) + continue; + + // Determine lifetime based on registration interface + var lifetime = GetLifetime(); + + // Register the service by interface + if (lifetime == ServiceLifetime.Singleton) + { + services.AddSingleton(registrationInterface, implementationType); + // Also register as concrete type for direct resolution (e.g., for ScrapingStepRegistry) + services.AddSingleton(implementationType); + } + else if (lifetime == ServiceLifetime.Scoped) + { + services.AddScoped(registrationInterface, implementationType); + services.AddScoped(implementationType); + } + else if (lifetime == ServiceLifetime.Transient) + { + services.AddTransient(registrationInterface, implementationType); + services.AddTransient(implementationType); + } + + registeredTypes.Add(implementationType); + } + } + } + + private static bool IsGenericTypeDefinition(Type type) + { + return type.IsGenericTypeDefinition || + (type.IsGenericType && type.GetGenericTypeDefinition() == type); + } + + private static ServiceLifetime GetLifetime() + where TRegistrationInterface : class + { + if (typeof(ISingletonService).IsAssignableFrom(typeof(TRegistrationInterface))) + return ServiceLifetime.Singleton; + if (typeof(IScopedService).IsAssignableFrom(typeof(TRegistrationInterface))) + return ServiceLifetime.Scoped; + if (typeof(ITransientService).IsAssignableFrom(typeof(TRegistrationInterface))) + return ServiceLifetime.Transient; + + throw new InvalidOperationException($"Unknown registration interface: {typeof(TRegistrationInterface).Name}"); + } + + private static void RegisterHostedServices( + IServiceCollection services, + List types, + HashSet registeredTypes) + { + foreach (var type in types) + { + // Skip if already registered + if (registeredTypes.Contains(type)) + continue; + + // Check if it's a BackgroundService that implements IHostedServiceRegistration + if (typeof(Microsoft.Extensions.Hosting.BackgroundService).IsAssignableFrom(type) && + typeof(IHostedServiceRegistration).IsAssignableFrom(type)) + { + // Check if already registered + var alreadyRegistered = services.Any(s => + s.ServiceType == typeof(Microsoft.Extensions.Hosting.IHostedService) && + s.ImplementationType == type); + + if (!alreadyRegistered) + { + // Use AddHostedService via reflection - this is the recommended way + // It properly handles lifecycle management for hosted services + // AddHostedService is generic, so we need to call it via reflection + var addHostedServiceMethod = typeof(Microsoft.Extensions.DependencyInjection.ServiceCollectionHostedServiceExtensions) + .GetMethod(nameof(Microsoft.Extensions.DependencyInjection.ServiceCollectionHostedServiceExtensions.AddHostedService), + new[] { typeof(IServiceCollection) })! + .MakeGenericMethod(type); + addHostedServiceMethod.Invoke(null, new object[] { services }); + registeredTypes.Add(type); + } + } + } + } +} + diff --git a/Models/FrameMetadata.cs b/Models/FrameMetadata.cs index b8d5e0e..8a97eac 100644 --- a/Models/FrameMetadata.cs +++ b/Models/FrameMetadata.cs @@ -11,8 +11,9 @@ public class FrameMetadata public int FrameIndex { get; set; } /// - /// Number of minutes ago this frame represents (40, 35, 30, 25, 20, 15, 10). + /// The absolute UTC observation time for this frame. + /// Parsed directly from the frame display label during capture. /// - public int MinutesAgo { get; set; } + public DateTime ObservationTime { get; set; } } diff --git a/Models/RadarFrame.cs b/Models/RadarFrame.cs index 8982d0a..7e58705 100644 --- a/Models/RadarFrame.cs +++ b/Models/RadarFrame.cs @@ -2,22 +2,14 @@ namespace BomLocalService.Models; /// /// Represents a single radar frame (historical precipitation data). -/// Frame 0 is oldest (40 minutes ago), Frame 6 is newest (10 minutes ago). /// Radar frames can be joined across cache folders to create extended historical slideshows. /// public class RadarFrame : CachedFrame { - /// - /// Number of minutes ago this frame represents (40, 35, 30, 25, 20, 15, 10). - /// Frame 0 = 40 minutes ago, Frame 6 = 10 minutes ago. - /// This is relative to the cache folder's observation time. - /// - public int MinutesAgo { get; set; } - /// /// The absolute UTC observation time for this frame. - /// Calculated as: ObservationTime - MinutesAgo. - /// This is set when frames are joined across cache folders in timeseries responses. + /// Parsed directly from the frame display label during capture. + /// Client calculates "minutes ago" dynamically from this timestamp. /// public DateTime? AbsoluteObservationTime { get; set; } diff --git a/Models/RadarResponse.cs b/Models/RadarResponse.cs index 00e530a..67ea464 100644 --- a/Models/RadarResponse.cs +++ b/Models/RadarResponse.cs @@ -8,7 +8,8 @@ public class RadarResponse { /// /// List of all captured frames (typically 7 frames: 0-6). - /// Frame 0 is oldest (40 minutes ago), Frame 6 is newest (10 minutes ago). + /// Each frame contains an absoluteObservationTime UTC timestamp. + /// Client should calculate "minutes ago" dynamically from the timestamp. /// public List Frames { get; set; } = new(); diff --git a/Program.cs b/Program.cs index 008c276..8021e3c 100644 --- a/Program.cs +++ b/Program.cs @@ -1,12 +1,6 @@ -using BomLocalService.Services; +using BomLocalService.Extensions; using BomLocalService.Services.Interfaces; using BomLocalService.Services.Scraping; -using BomLocalService.Services.Scraping.Steps.Navigation; -using BomLocalService.Services.Scraping.Steps.Search; -using BomLocalService.Services.Scraping.Steps.Map; -using BomLocalService.Services.Scraping.Steps.Metadata; -using BomLocalService.Services.Scraping.Steps.Capture; -using BomLocalService.Services.Scraping.Workflows; var builder = WebApplication.CreateBuilder(args); @@ -17,100 +11,10 @@ builder.Services.AddEndpointsApiExplorer(); builder.Services.AddHealthChecks(); // Configure CORS - MUST be added before other services -// 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."); +builder.Services.AddCorsConfiguration(builder.Configuration); -// 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) -{ - 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 => -{ - options.AddDefaultPolicy(policy => - { - if (corsOrigins == "*") - { - policy.AllowAnyOrigin(); - } - else - { - // Split comma-separated origins - var origins = corsOrigins.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - policy.WithOrigins(origins); - } - - // Split comma-separated methods - var methods = corsMethods.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - policy.WithMethods(methods); - - // Split comma-separated headers or allow all - if (corsHeaders == "*") - { - policy.AllowAnyHeader(); - } - else - { - var headers = corsHeaders.Split(',', StringSplitOptions.RemoveEmptyEntries | StringSplitOptions.TrimEntries); - policy.WithHeaders(headers); - } - - if (corsAllowCredentials) - { - policy.AllowCredentials(); - } - }); -}); - -// Register core services via interfaces (order matters - dependencies must be registered first) -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); -builder.Services.AddSingleton(); - -// Register scraping step registry -builder.Services.AddSingleton(); - -// Register all scraping steps -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); -builder.Services.AddScoped(); - -// Register workflows -builder.Services.AddScoped(); -builder.Services.AddScoped(); - -// Register workflow factory -builder.Services.AddSingleton(); - -// Register scraping service (depends on workflow factory) -builder.Services.AddSingleton(); - -// Register BOM Radar Service as singleton (orchestrator, depends on all above services) -builder.Services.AddSingleton(); - -// Register background services (order matters - management service needs radar service) -builder.Services.AddHostedService(); -builder.Services.AddHostedService(); +// Register all services via assembly scanning +builder.Services.ScanAndRegisterServices(typeof(Program).Assembly); var app = builder.Build(); @@ -142,24 +46,6 @@ app.MapControllers(); // Map health check endpoint for Docker health monitoring app.MapHealthChecks("/api/health"); -// Auto-register all scraping steps in the registry -var stepRegistry = app.Services.GetRequiredService(); -var stepTypes = typeof(IScrapingStep).Assembly.GetTypes() - .Where(t => typeof(IScrapingStep).IsAssignableFrom(t) && !t.IsInterface && !t.IsAbstract && !t.IsGenericType); -foreach (var stepType in stepTypes) -{ - try - { - var step = (IScrapingStep)ActivatorUtilities.CreateInstance(app.Services, stepType); - stepRegistry.RegisterStep(step); - } - catch (Exception ex) - { - var logger = app.Services.GetRequiredService>(); - logger.LogWarning(ex, "Failed to register step {StepType}", stepType.Name); - } -} - // Cleanup incomplete cache folders from previous crashes/restarts before starting services var cacheService = app.Services.GetRequiredService(); var deletedCount = cacheService.CleanupIncompleteCacheFolders(); diff --git a/README.md b/README.md index e8164ba..fe591c6 100644 --- a/README.md +++ b/README.md @@ -523,8 +523,7 @@ GET /api/radar/{suburb}/{state} { "frameIndex": 0, "imageUrl": "/api/radar/Brisbane/QLD/frame/0", - "minutesAgo": 0, - "observationTime": "2025-01-15T10:00:00Z" + "absoluteObservationTime": "2025-01-15T10:00:00Z" } ], "observationTime": "2025-01-15T10:00:00Z", @@ -539,7 +538,7 @@ GET /api/radar/{suburb}/{state} ``` **Response Fields:** -- `frames`: Array of radar frame objects with image URLs and timing information +- `frames`: Array of radar frame objects with image URLs. Each frame contains `absoluteObservationTime` (UTC timestamp). Client should calculate "minutes ago" dynamically from this timestamp. - `observationTime`: UTC timestamp when the observation was made - `forecastTime`: UTC timestamp for the forecast - `weatherStation`: Name of the weather station @@ -669,8 +668,7 @@ GET /api/radar/{suburb}/{state}/timeseries?startTime={iso8601}&endTime={iso8601} { "frameIndex": 0, "imageUrl": "/api/radar/Brisbane/QLD/frame/0?cacheFolder=Brisbane_QLD_20250115_100000", - "absoluteObservationTime": "2025-01-15T10:00:00Z", - "minutesAgo": 0 + "absoluteObservationTime": "2025-01-15T10:00:00Z" } ] } @@ -1008,7 +1006,10 @@ if (radarData.frames && radarData.frames.length > 0) { // Or loop through all frames for animation radarData.frames.forEach((frame, index) => { - console.log(`Frame ${index}: ${frame.imageUrl} (${frame.minutesAgo} min ago)`); + const minutesAgo = frame.absoluteObservationTime + ? Math.round((Date.now() - new Date(frame.absoluteObservationTime).getTime()) / 60000) + : null; + console.log(`Frame ${index}: ${frame.imageUrl}${minutesAgo !== null ? ` (${minutesAgo} min ago)` : ''}`); }); } ``` diff --git a/Services/BomRadarService.cs b/Services/BomRadarService.cs index 6784d52..4e922d9 100644 --- a/Services/BomRadarService.cs +++ b/Services/BomRadarService.cs @@ -519,32 +519,27 @@ public class BomRadarService : IBomRadarService, IDisposable if (radarFrames.Count > 0) { - // Generate URLs and calculate absolute observation times for each frame + // Generate URLs and filter frames by unique absolute observation times var uniqueFrames = new List(); foreach (var frame in radarFrames) { frame.ImageUrl = $"/api/radar/{encodedSuburb}/{encodedState}/frame/{frame.FrameIndex}?cacheFolder={Uri.EscapeDataString(folderInfo.FolderName)}"; - // Calculate absolute observation time for this frame - // minutesAgo is relative to the cache folder's observation time - // Validate that ObservationTime is reasonable (not default/min value) - if (folderInfo.ObservationTime > DateTime.MinValue.AddYears(1) && frame.MinutesAgo >= 0) + // Use absolute observation time directly (already set during capture) + // Only include frames with unique absolute observation times + // Since we process folders newest-first, this ensures we keep frames from the most recent cache folder + if (frame.AbsoluteObservationTime.HasValue && + frame.AbsoluteObservationTime.Value > DateTime.MinValue.AddYears(1) && + !seenAbsoluteTimes.Contains(frame.AbsoluteObservationTime.Value)) { - frame.AbsoluteObservationTime = folderInfo.ObservationTime.AddMinutes(-frame.MinutesAgo); - - // Only include frames with unique absolute observation times - // Since we process folders newest-first, this ensures we keep frames from the most recent cache folder - if (frame.AbsoluteObservationTime.HasValue && !seenAbsoluteTimes.Contains(frame.AbsoluteObservationTime.Value)) - { - seenAbsoluteTimes.Add(frame.AbsoluteObservationTime.Value); - uniqueFrames.Add(frame); - } + seenAbsoluteTimes.Add(frame.AbsoluteObservationTime.Value); + uniqueFrames.Add(frame); } - else + else if (!frame.AbsoluteObservationTime.HasValue) { - _logger.LogWarning("Skipping frame {FrameIndex} from folder {FolderName}: Invalid ObservationTime ({ObservationTime}) or MinutesAgo ({MinutesAgo})", - frame.FrameIndex, folderInfo.FolderName, folderInfo.ObservationTime, frame.MinutesAgo); + _logger.LogWarning("Skipping frame {FrameIndex} from folder {FolderName}: Missing AbsoluteObservationTime", + frame.FrameIndex, folderInfo.FolderName); } } diff --git a/Services/CacheCleanupService.cs b/Services/CacheCleanupService.cs index 678ad37..4a46e72 100644 --- a/Services/CacheCleanupService.cs +++ b/Services/CacheCleanupService.cs @@ -1,9 +1,10 @@ +using BomLocalService.Services.Interfaces.Registration; using BomLocalService.Utilities; using Microsoft.Extensions.Hosting; namespace BomLocalService.Services; -public class CacheCleanupService : BackgroundService +public class CacheCleanupService : BackgroundService, IHostedServiceRegistration { private readonly ILogger _logger; private readonly string _cacheDirectory; diff --git a/Services/CacheManagementService.cs b/Services/CacheManagementService.cs index 851d767..67ea2ec 100644 --- a/Services/CacheManagementService.cs +++ b/Services/CacheManagementService.cs @@ -1,10 +1,11 @@ using BomLocalService.Services.Interfaces; +using BomLocalService.Services.Interfaces.Registration; using BomLocalService.Utilities; using Microsoft.Extensions.Hosting; namespace BomLocalService.Services; -public class CacheManagementService : BackgroundService +public class CacheManagementService : BackgroundService, IHostedServiceRegistration { private readonly ILogger _logger; private readonly IBomRadarService _bomRadarService; diff --git a/Services/CacheService.cs b/Services/CacheService.cs index 0d7f3be..22a7377 100644 --- a/Services/CacheService.cs +++ b/Services/CacheService.cs @@ -154,18 +154,18 @@ public class CacheService : ICacheService return null; } - // Load frame metadata to get accurate minutesAgo + // Load frame metadata to get observation time var framesMetadata = await LoadFramesMetadataAsync(cacheFolderPath, CachedDataType.Radar, cancellationToken); var frameMetadata = framesMetadata.FirstOrDefault(f => f.FrameIndex == frameIndex); - var minutesAgo = frameMetadata != null - ? frameMetadata.MinutesAgo - : 40 - (frameIndex * 5); + var observationTime = frameMetadata?.ObservationTime > DateTime.MinValue.AddYears(1) + ? frameMetadata.ObservationTime + : (DateTime?)null; return new RadarFrame { FrameIndex = frameIndex, ImagePath = framePath, - MinutesAgo = minutesAgo + AbsoluteObservationTime = observationTime }; } @@ -200,7 +200,7 @@ public class CacheService : ICacheService var framesMetadata = frames.Select(f => new FrameMetadata { FrameIndex = f.FrameIndex, - MinutesAgo = f.MinutesAgo + ObservationTime = f.AbsoluteObservationTime ?? throw new InvalidOperationException($"Frame {f.FrameIndex} missing AbsoluteObservationTime") }).ToList(); var framesPath = FilePathHelper.GetFramesMetadataFilePath(cacheFolderPath, dataType); @@ -424,7 +424,8 @@ public class CacheService : ICacheService // Load frame metadata from frames.json if available var framesMetadata = await LoadFramesMetadataAsync(folderPath, dataType, cancellationToken); - var metadataDict = framesMetadata.ToDictionary(f => f.FrameIndex, f => f.MinutesAgo); + var metadataDict = framesMetadata.Where(f => f.ObservationTime > DateTime.MinValue.AddYears(1)) + .ToDictionary(f => f.FrameIndex, f => f.ObservationTime); // Load frames from data type subfolder for (int i = 0; i < frameCount; i++) @@ -434,15 +435,13 @@ public class CacheService : ICacheService { if (dataType == CachedDataType.Radar) { - var minutesAgo = metadataDict.ContainsKey(i) - ? metadataDict[i] - : 40 - (i * 5); + var observationTime = metadataDict.ContainsKey(i) ? metadataDict[i] : (DateTime?)null; frames.Add(new RadarFrame { FrameIndex = i, ImagePath = framePath, - MinutesAgo = minutesAgo + AbsoluteObservationTime = observationTime }); } else diff --git a/Services/Interfaces/IBomRadarService.cs b/Services/Interfaces/IBomRadarService.cs index c56d22f..cdb2d2d 100644 --- a/Services/Interfaces/IBomRadarService.cs +++ b/Services/Interfaces/IBomRadarService.cs @@ -1,4 +1,5 @@ using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; namespace BomLocalService.Services.Interfaces; @@ -6,7 +7,7 @@ namespace BomLocalService.Services.Interfaces; /// Main service interface for BOM radar screenshot operations. /// Orchestrates cache management, browser automation, and web scraping to provide radar screenshots for Australian locations. /// -public interface IBomRadarService +public interface IBomRadarService : ISingletonService { /// /// Gets cached radar data for a location. diff --git a/Services/Interfaces/IBrowserService.cs b/Services/Interfaces/IBrowserService.cs index 2b0d4f6..1e0ffca 100644 --- a/Services/Interfaces/IBrowserService.cs +++ b/Services/Interfaces/IBrowserService.cs @@ -1,3 +1,4 @@ +using BomLocalService.Services.Interfaces.Registration; using Microsoft.Playwright; namespace BomLocalService.Services.Interfaces; @@ -6,7 +7,7 @@ namespace BomLocalService.Services.Interfaces; /// Service interface for managing Playwright browser instances and automation. /// Handles browser lifecycle, context creation, and anti-detection measures for web scraping. /// -public interface IBrowserService : IDisposable +public interface IBrowserService : ISingletonService, IDisposable { /// /// Creates a new browser context with proper configuration for BOM website scraping. diff --git a/Services/Interfaces/ICacheService.cs b/Services/Interfaces/ICacheService.cs index 5eb381f..d487812 100644 --- a/Services/Interfaces/ICacheService.cs +++ b/Services/Interfaces/ICacheService.cs @@ -1,4 +1,5 @@ using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; namespace BomLocalService.Services.Interfaces; @@ -6,7 +7,7 @@ namespace BomLocalService.Services.Interfaces; /// Service interface for managing cached radar screenshot files and metadata. /// Handles file system operations for storing and retrieving cached BOM radar screenshots. /// -public interface ICacheService +public interface ICacheService : ISingletonService { /// /// Gets the cached screenshot folder path and associated metadata for a location and data type. @@ -90,7 +91,7 @@ public interface ICacheService Task SaveMetadataAsync(string cacheFolderPath, LastUpdatedInfo metadata, CancellationToken cancellationToken = default); /// - /// Saves frame metadata (frame index and minutes ago) to a frames.json file in the data type subfolder. + /// Saves frame metadata (frame index and observation time) to a frames.json file in the data type subfolder. /// /// Full path to the cache folder /// The type of cached data diff --git a/Services/Interfaces/IDebugService.cs b/Services/Interfaces/IDebugService.cs index 77b889f..e9ee651 100644 --- a/Services/Interfaces/IDebugService.cs +++ b/Services/Interfaces/IDebugService.cs @@ -1,3 +1,4 @@ +using BomLocalService.Services.Interfaces.Registration; using Microsoft.Playwright; namespace BomLocalService.Services.Interfaces; @@ -6,7 +7,7 @@ namespace BomLocalService.Services.Interfaces; /// Service interface for debug file generation during web scraping operations. /// When enabled, saves screenshots, HTML, console logs, and network request information for troubleshooting. /// -public interface IDebugService +public interface IDebugService : ISingletonService { /// /// Indicates whether debug mode is enabled. diff --git a/Services/Interfaces/IScrapingService.cs b/Services/Interfaces/IScrapingService.cs index c9ac07f..10b8f0b 100644 --- a/Services/Interfaces/IScrapingService.cs +++ b/Services/Interfaces/IScrapingService.cs @@ -1,4 +1,5 @@ using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; using Microsoft.Playwright; namespace BomLocalService.Services.Interfaces; @@ -7,7 +8,7 @@ namespace BomLocalService.Services.Interfaces; /// Service interface for scraping radar screenshots from the BOM website. /// Orchestrates the multi-step process of navigating BOM, finding locations, and capturing radar screenshots. /// -public interface IScrapingService +public interface IScrapingService : ISingletonService { /// /// Scrapes a radar screenshot from the BOM website for a given location. diff --git a/Services/Interfaces/ISelectorService.cs b/Services/Interfaces/ISelectorService.cs index 420c3fa..4084608 100644 --- a/Services/Interfaces/ISelectorService.cs +++ b/Services/Interfaces/ISelectorService.cs @@ -1,4 +1,5 @@ using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; using Microsoft.Playwright; namespace BomLocalService.Services.Interfaces; @@ -6,7 +7,7 @@ namespace BomLocalService.Services.Interfaces; /// /// Service for finding page elements using configured selectors /// -public interface ISelectorService +public interface ISelectorService : ISingletonService { /// /// Finds an element using the configured selectors, trying each in order until one is found diff --git a/Services/Interfaces/ITimeParsingService.cs b/Services/Interfaces/ITimeParsingService.cs index 11ae08b..515c021 100644 --- a/Services/Interfaces/ITimeParsingService.cs +++ b/Services/Interfaces/ITimeParsingService.cs @@ -1,4 +1,5 @@ using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; using Microsoft.Playwright; namespace BomLocalService.Services.Interfaces; @@ -7,7 +8,7 @@ namespace BomLocalService.Services.Interfaces; /// Service interface for parsing time and metadata information from BOM website content. /// Extracts observation times, forecast times, weather station names, and distances from BOM weather map pages. /// -public interface ITimeParsingService +public interface ITimeParsingService : ISingletonService { /// /// Extracts last updated information from a BOM weather map page. diff --git a/Services/Interfaces/Registration/IServiceRegistration.cs b/Services/Interfaces/Registration/IServiceRegistration.cs new file mode 100644 index 0000000..f919451 --- /dev/null +++ b/Services/Interfaces/Registration/IServiceRegistration.cs @@ -0,0 +1,30 @@ +namespace BomLocalService.Services.Interfaces.Registration; + +/// +/// Marker interface for services that should be registered as Singleton +/// +public interface ISingletonService +{ +} + +/// +/// Marker interface for services that should be registered as Scoped +/// +public interface IScopedService +{ +} + +/// +/// Marker interface for services that should be registered as Transient +/// +public interface ITransientService +{ +} + +/// +/// Marker interface for hosted services (BackgroundService implementations) +/// +public interface IHostedServiceRegistration +{ +} + diff --git a/Services/Scraping/IScrapingStep.cs b/Services/Scraping/IScrapingStep.cs index 15fed03..2c8c74f 100644 --- a/Services/Scraping/IScrapingStep.cs +++ b/Services/Scraping/IScrapingStep.cs @@ -1,9 +1,12 @@ +using BomLocalService.Services.Interfaces.Registration; + namespace BomLocalService.Services.Scraping; /// /// Interface for a single scraping step +/// Steps are stateless (all request data is in ScrapingContext), so singleton lifetime is appropriate /// -public interface IScrapingStep +public interface IScrapingStep : ISingletonService { /// /// Unique name of the step diff --git a/Services/Scraping/IScrapingStepRegistry.cs b/Services/Scraping/IScrapingStepRegistry.cs index 9be02d0..feade31 100644 --- a/Services/Scraping/IScrapingStepRegistry.cs +++ b/Services/Scraping/IScrapingStepRegistry.cs @@ -1,9 +1,11 @@ +using BomLocalService.Services.Interfaces.Registration; + namespace BomLocalService.Services.Scraping; /// /// Registry for managing scraping steps /// -public interface IScrapingStepRegistry +public interface IScrapingStepRegistry : ISingletonService { void RegisterStep(IScrapingStep step); IScrapingStep? GetStep(string name); diff --git a/Services/Scraping/IWorkflowFactory.cs b/Services/Scraping/IWorkflowFactory.cs index 1e7a2fa..027a9a3 100644 --- a/Services/Scraping/IWorkflowFactory.cs +++ b/Services/Scraping/IWorkflowFactory.cs @@ -1,9 +1,11 @@ +using BomLocalService.Services.Interfaces.Registration; + namespace BomLocalService.Services.Scraping; /// /// Factory for creating workflows /// -public interface IWorkflowFactory +public interface IWorkflowFactory : ISingletonService { IWorkflow GetWorkflow(string name); } diff --git a/Services/Scraping/ScrapingStepRegistry.cs b/Services/Scraping/ScrapingStepRegistry.cs index 92de591..4c86bab 100644 --- a/Services/Scraping/ScrapingStepRegistry.cs +++ b/Services/Scraping/ScrapingStepRegistry.cs @@ -1,3 +1,5 @@ +using System.Reflection; + namespace BomLocalService.Services.Scraping; public class ScrapingStepRegistry : IScrapingStepRegistry @@ -5,9 +7,33 @@ public class ScrapingStepRegistry : IScrapingStepRegistry private readonly Dictionary _steps = new(); private readonly ILogger _logger; - public ScrapingStepRegistry(ILogger logger) + public ScrapingStepRegistry(ILogger logger, IServiceProvider serviceProvider) { _logger = logger; + AutoRegisterSteps(serviceProvider); + } + + private void AutoRegisterSteps(IServiceProvider serviceProvider) + { + var stepTypes = typeof(IScrapingStep).Assembly.GetTypes() + .Where(t => typeof(IScrapingStep).IsAssignableFrom(t) + && !t.IsInterface + && !t.IsAbstract + && !t.IsGenericType); + + foreach (var stepType in stepTypes) + { + try + { + // Use DI to resolve the step (ensures proper dependency injection) + var step = (IScrapingStep)serviceProvider.GetRequiredService(stepType); + RegisterStep(step); + } + catch (Exception ex) + { + _logger.LogWarning(ex, "Failed to auto-register step {StepType}", stepType.Name); + } + } } public void RegisterStep(IScrapingStep step) diff --git a/Services/Scraping/Steps/Capture/CaptureFramesStep.cs b/Services/Scraping/Steps/Capture/CaptureFramesStep.cs index e4f95b7..ab731e9 100644 --- a/Services/Scraping/Steps/Capture/CaptureFramesStep.cs +++ b/Services/Scraping/Steps/Capture/CaptureFramesStep.cs @@ -78,7 +78,7 @@ public class CaptureFramesStep : BaseScrapingStep var frames = new List(); var stepForwardButton = SelectorService.GetLocator(context.Page, Selectors.StepForwardButton); - int? previousMinutesAgo = null; + DateTime? previousTimestamp = null; for (int frameIndex = 0; frameIndex < frameCount; frameIndex++) { @@ -90,37 +90,48 @@ public class CaptureFramesStep : BaseScrapingStep // This is especially important for frame 0 which was just selected in ResetToFirstFrame await context.Page.WaitForTimeoutAsync(300); - // Try extracting with a retry in case the label is still updating - var minutesAgo = await ExtractMinutesAgoFromDisplayAsync(context.Page); - if (minutesAgo == null) + // Try extracting timestamp with a retry in case the label is still updating + var frameTimestamp = await ExtractTimestampFromDisplayAsync(context.Page); + if (frameTimestamp == null) { // Retry once after a short wait in case label was updating await context.Page.WaitForTimeoutAsync(200); - minutesAgo = await ExtractMinutesAgoFromDisplayAsync(context.Page); - } - if (minutesAgo == null && context.FrameInfo != null && frameIndex < context.FrameInfo.Count) - { - var (_, defaultMinutesAgo) = context.FrameInfo[frameIndex]; - minutesAgo = defaultMinutesAgo; - Logger.LogWarning("Step {Step}: Failed to extract minutes from display label for frame {FrameIndex}, using default: {MinutesAgo}", Name, frameIndex, minutesAgo); + frameTimestamp = await ExtractTimestampFromDisplayAsync(context.Page); } - if (frameIndex > 0 && previousMinutesAgo.HasValue && minutesAgo == previousMinutesAgo.Value) + // Fallback: calculate expected timestamp from observation time and frame index if we can't parse it + if (frameTimestamp == null && context.LastUpdatedInfo?.ObservationTime != null && context.FrameInfo != null && frameIndex < context.FrameInfo.Count) { - Logger.LogWarning("Step {Step}: Frame {FrameIndex} has same minutesAgo ({MinutesAgo}) as previous frame. Waiting for display to update...", Name, frameIndex, minutesAgo); - await WaitForDisplayLabelToChangeAsync(context.Page, previousMinutesAgo.Value); - minutesAgo = await ExtractMinutesAgoFromDisplayAsync(context.Page); - if (minutesAgo == null || minutesAgo == previousMinutesAgo.Value) + var (_, defaultMinutesAgo) = context.FrameInfo[frameIndex]; + frameTimestamp = context.LastUpdatedInfo.ObservationTime.AddMinutes(-defaultMinutesAgo); + Logger.LogWarning("Step {Step}: Failed to extract timestamp from display label for frame {FrameIndex}, calculated from observation time: {Timestamp}", + Name, frameIndex, frameTimestamp); + } + + if (frameIndex > 0 && previousTimestamp.HasValue && frameTimestamp == previousTimestamp.Value) + { + Logger.LogWarning("Step {Step}: Frame {FrameIndex} has same timestamp ({Timestamp}) as previous frame. Waiting for display to update...", + Name, frameIndex, frameTimestamp); + await WaitForDisplayLabelToChangeAsync(context.Page, previousTimestamp.Value); + frameTimestamp = await ExtractTimestampFromDisplayAsync(context.Page); + if (frameTimestamp == null || frameTimestamp == previousTimestamp.Value) { - if (context.FrameInfo != null && frameIndex < context.FrameInfo.Count) + if (context.LastUpdatedInfo?.ObservationTime != null && context.FrameInfo != null && frameIndex < context.FrameInfo.Count) { var (_, defaultMinutesAgo) = context.FrameInfo[frameIndex]; - minutesAgo = defaultMinutesAgo; - Logger.LogWarning("Step {Step}: Display label did not update for frame {FrameIndex}, using calculated default: {MinutesAgo}", Name, frameIndex, minutesAgo); + frameTimestamp = context.LastUpdatedInfo.ObservationTime.AddMinutes(-defaultMinutesAgo); + Logger.LogWarning("Step {Step}: Display label did not update for frame {FrameIndex}, calculated from observation time: {Timestamp}", + Name, frameIndex, frameTimestamp); } } } + if (frameTimestamp == null) + { + Logger.LogError("Step {Step}: Could not determine timestamp for frame {FrameIndex}, skipping", Name, frameIndex); + continue; + } + var radarFolder = FilePathHelper.GetDataTypeFolderPath(context.CacheFolderPath, CachedDataType.Radar); if (!Directory.Exists(radarFolder)) { @@ -134,13 +145,13 @@ public class CaptureFramesStep : BaseScrapingStep { FrameIndex = frameIndex, ImagePath = framePath, - MinutesAgo = minutesAgo ?? 0 + AbsoluteObservationTime = frameTimestamp.Value }); - previousMinutesAgo = minutesAgo; + previousTimestamp = frameTimestamp; - Logger.LogInformation("Step {Step}: Frame {FrameIndex} saved: {Path} ({MinutesAgo} minutes ago)", - Name, frameIndex, framePath, minutesAgo ?? 0); + Logger.LogInformation("Step {Step}: Frame {FrameIndex} saved: {Path} (timestamp: {Timestamp} UTC)", + Name, frameIndex, framePath, frameTimestamp.Value); _cacheService.RecordUpdateProgressByFolder(context.CacheFolderPath, CacheUpdatePhase.CapturingFrames, frameIndex + 1, frameCount); @@ -150,13 +161,13 @@ public class CaptureFramesStep : BaseScrapingStep { await DismissModalOverlaysAsync(context.Page); - var currentMinutesAgo = await ExtractMinutesAgoFromDisplayAsync(context.Page); + var currentTimestamp = await ExtractTimestampFromDisplayAsync(context.Page); await stepForwardButton.ClickAsync(new LocatorClickOptions { Force = true }); - if (currentMinutesAgo.HasValue) + if (currentTimestamp.HasValue) { - await WaitForDisplayLabelToChangeAsync(context.Page, currentMinutesAgo.Value); + await WaitForDisplayLabelToChangeAsync(context.Page, currentTimestamp.Value); } else { @@ -187,7 +198,10 @@ public class CaptureFramesStep : BaseScrapingStep } } - private async Task ExtractMinutesAgoFromDisplayAsync(IPage page) + /// + /// Extracts the UTC timestamp from the frame display label + /// + private async Task ExtractTimestampFromDisplayAsync(IPage page) { try { @@ -200,7 +214,7 @@ public class CaptureFramesStep : BaseScrapingStep } var trimmedLabel = timeLabel.Trim(); - Logger.LogInformation("Extracting minutes from display label: '{Label}'", trimmedLabel); + Logger.LogInformation("Extracting timestamp from display label: '{Label}'", trimmedLabel); // Parse timestamp format (current BOM website format): "Wednesday 17 Dec, 11:05 pm" or "17 Dec, 11:05 pm" Logger.LogInformation("Trying timestamp pattern: '{Pattern}'", TextPatterns.TimestampPattern); @@ -209,19 +223,10 @@ public class CaptureFramesStep : BaseScrapingStep { var timestampStr = timestampMatch.Groups[0].Value; Logger.LogInformation("Matched timestamp pattern: '{Timestamp}' from label: '{Label}'", timestampStr, trimmedLabel); - if (TryParseTimestamp(timestampStr, out var timestamp)) + if (TryParseTimestamp(timestampStr, out var frameTimestampUtc)) { - var minutesAgo = (int)(DateTime.UtcNow - timestamp).TotalMinutes; - Logger.LogInformation("Parsed timestamp: {Timestamp} UTC, calculated minutes ago: {Minutes}", timestamp, minutesAgo); - if (minutesAgo >= 0 && minutesAgo <= 120) // Reasonable range: 0-2 hours - { - Logger.LogInformation("Successfully calculated minutes ago from timestamp: {Minutes}", minutesAgo); - return minutesAgo; - } - else - { - Logger.LogWarning("Calculated minutes ago ({Minutes}) outside reasonable range (0-120)", minutesAgo); - } + Logger.LogInformation("Successfully parsed frame timestamp: {Timestamp} UTC", frameTimestampUtc); + return frameTimestampUtc; } else { @@ -237,17 +242,27 @@ public class CaptureFramesStep : BaseScrapingStep } catch (Exception ex) { - Logger.LogDebug(ex, "Failed to extract minutes from display label"); + Logger.LogDebug(ex, "Failed to extract timestamp from display label"); return null; } } - private bool TryParseTimestamp(string timestampStr, out DateTime timestamp) + private bool TryParseTimestamp(string timestampStr, out DateTime timestampUtc) { - timestamp = DateTime.MinValue; + timestampUtc = DateTime.MinValue; try { + // Get configured timezone + var timezone = Configuration.GetValue("Timezone"); + if (string.IsNullOrEmpty(timezone)) + { + Logger.LogWarning("Timezone not configured, cannot parse timestamp correctly"); + return false; + } + + var timeZoneInfo = TimeZoneInfo.FindSystemTimeZoneById(timezone); + // Try common Australian date formats // Format: "Wednesday 17 Dec, 11:05 pm" or "17 Dec, 11:05 pm" var formats = new[] @@ -261,57 +276,68 @@ public class CaptureFramesStep : BaseScrapingStep }; var culture = new System.Globalization.CultureInfo("en-AU"); + DateTime localTime = default; + bool parsed = false; foreach (var format in formats) { if (DateTime.TryParseExact(timestampStr, format, culture, - System.Globalization.DateTimeStyles.AssumeLocal, out timestamp)) + System.Globalization.DateTimeStyles.None, out localTime)) { - // If year is not specified, assume current year - if (timestamp.Year == 1) - { - timestamp = new DateTime(DateTime.Now.Year, timestamp.Month, timestamp.Day, - timestamp.Hour, timestamp.Minute, timestamp.Second); - } - - // If the parsed time is in the future (likely same day next year), adjust - if (timestamp > DateTime.Now && timestamp < DateTime.Now.AddDays(1)) - { - // Already correct - } - else if (timestamp > DateTime.Now) - { - // Likely parsed as next year, adjust to this year - timestamp = timestamp.AddYears(-1); - } - - return true; + parsed = true; + break; } } - return false; + if (!parsed) + { + return false; + } + + // If year is not specified, assume current year + if (localTime.Year == 1) + { + localTime = new DateTime(DateTime.Now.Year, localTime.Month, localTime.Day, + localTime.Hour, localTime.Minute, localTime.Second); + } + + // Get current time in the configured timezone to determine the date + var nowInTz = TimeZoneInfo.ConvertTimeFromUtc(DateTime.UtcNow, timeZoneInfo); + var dateTimeInTz = nowInTz.Date.Add(localTime.TimeOfDay); + + // If the time is in the future, it must be from yesterday + if (dateTimeInTz > nowInTz) + { + dateTimeInTz = dateTimeInTz.AddDays(-1); + } + + // Convert to UTC + timestampUtc = TimeZoneInfo.ConvertTimeToUtc(dateTimeInTz, timeZoneInfo); + + return true; } - catch + catch (Exception ex) { + Logger.LogWarning(ex, "Failed to parse timestamp: {Timestamp}", timestampStr); return false; } } - private async Task WaitForDisplayLabelToChangeAsync(IPage page, int currentMinutesAgo, int maxWaitMs = 5000) + private async Task WaitForDisplayLabelToChangeAsync(IPage page, DateTime currentTimestamp, int maxWaitMs = 5000) { try { var startTime = DateTime.UtcNow; while ((DateTime.UtcNow - startTime).TotalMilliseconds < maxWaitMs) { - var newMinutesAgo = await ExtractMinutesAgoFromDisplayAsync(page); - if (newMinutesAgo.HasValue && newMinutesAgo.Value != currentMinutesAgo) + var newTimestamp = await ExtractTimestampFromDisplayAsync(page); + if (newTimestamp.HasValue && newTimestamp.Value != currentTimestamp) { return; } await page.WaitForTimeoutAsync(200); } - Logger.LogDebug("Display label did not change from {CurrentMinutesAgo} within {MaxWaitMs}ms", currentMinutesAgo, maxWaitMs); + Logger.LogDebug("Display label did not change from {CurrentTimestamp} within {MaxWaitMs}ms", currentTimestamp, maxWaitMs); } catch (Exception ex) { diff --git a/Services/Scraping/WorkflowFactory.cs b/Services/Scraping/WorkflowFactory.cs index 3f18978..7fe0806 100644 --- a/Services/Scraping/WorkflowFactory.cs +++ b/Services/Scraping/WorkflowFactory.cs @@ -1,3 +1,5 @@ +using BomLocalService.Services.Scraping.Workflows; + namespace BomLocalService.Services.Scraping; public class WorkflowFactory : IWorkflowFactory @@ -13,8 +15,10 @@ public class WorkflowFactory : IWorkflowFactory { return name switch { - "RadarScraping" => (IWorkflow)_serviceProvider.GetRequiredService(), - "TemperatureMap" => (IWorkflow)_serviceProvider.GetRequiredService(), + "RadarScraping" => _serviceProvider.GetRequiredService() as IWorkflow + ?? throw new InvalidOperationException($"IRadarScrapingWorkflow is not IWorkflow<{typeof(TResponse).Name}>"), + "TemperatureMap" => _serviceProvider.GetRequiredService() as IWorkflow + ?? throw new InvalidOperationException($"ITemperatureMapWorkflow is not IWorkflow<{typeof(TResponse).Name}>"), _ => throw new ArgumentException($"Unknown workflow: {name}") }; } diff --git a/Services/Scraping/Workflows/IRadarScrapingWorkflow.cs b/Services/Scraping/Workflows/IRadarScrapingWorkflow.cs new file mode 100644 index 0000000..88c578b --- /dev/null +++ b/Services/Scraping/Workflows/IRadarScrapingWorkflow.cs @@ -0,0 +1,12 @@ +using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; + +namespace BomLocalService.Services.Scraping.Workflows; + +/// +/// Concrete interface for radar scraping workflow (hides generic IWorkflow<RadarResponse>) +/// +public interface IRadarScrapingWorkflow : IWorkflow, IScopedService +{ +} + diff --git a/Services/Scraping/Workflows/ITemperatureMapWorkflow.cs b/Services/Scraping/Workflows/ITemperatureMapWorkflow.cs new file mode 100644 index 0000000..e1bc6bb --- /dev/null +++ b/Services/Scraping/Workflows/ITemperatureMapWorkflow.cs @@ -0,0 +1,12 @@ +using BomLocalService.Models; +using BomLocalService.Services.Interfaces.Registration; + +namespace BomLocalService.Services.Scraping.Workflows; + +/// +/// Concrete interface for temperature map workflow (hides generic IWorkflow<RadarResponse>) +/// +public interface ITemperatureMapWorkflow : IWorkflow, IScopedService +{ +} + diff --git a/Services/Scraping/Workflows/RadarScrapingWorkflow.cs b/Services/Scraping/Workflows/RadarScrapingWorkflow.cs index 42449bf..5600b9a 100644 --- a/Services/Scraping/Workflows/RadarScrapingWorkflow.cs +++ b/Services/Scraping/Workflows/RadarScrapingWorkflow.cs @@ -5,7 +5,7 @@ using BomLocalService.Utilities; namespace BomLocalService.Services.Scraping.Workflows; -public class RadarScrapingWorkflow : IWorkflow +public class RadarScrapingWorkflow : IRadarScrapingWorkflow { private readonly ILogger _logger; private readonly IScrapingStepRegistry _stepRegistry; diff --git a/Services/Scraping/Workflows/TemperatureMapWorkflow.cs b/Services/Scraping/Workflows/TemperatureMapWorkflow.cs index 4176105..f845e13 100644 --- a/Services/Scraping/Workflows/TemperatureMapWorkflow.cs +++ b/Services/Scraping/Workflows/TemperatureMapWorkflow.cs @@ -7,7 +7,7 @@ namespace BomLocalService.Services.Scraping.Workflows; /// /// Future workflow for temperature map scraping (not yet implemented) /// -public class TemperatureMapWorkflow : IWorkflow +public class TemperatureMapWorkflow : ITemperatureMapWorkflow { private readonly ILogger _logger; diff --git a/Services/TimeParsingService.cs b/Services/TimeParsingService.cs index 508a9e6..cff3573 100644 --- a/Services/TimeParsingService.cs +++ b/Services/TimeParsingService.cs @@ -67,113 +67,77 @@ public class TimeParsingService : ITimeParsingService } catch (Exception ex) { - _logger.LogWarning(ex, "Failed to extract last updated information"); - return new LastUpdatedInfo - { - ObservationTime = DateTime.UtcNow, - ForecastTime = DateTime.UtcNow - }; + _logger.LogError(ex, "Failed to extract last updated information"); + throw; } } /// /// Parses last updated text to extract observation time, forecast time, weather station, and distance + /// Parses actual time strings to UTC timestamps - client calculates "minutes ago" dynamically /// public LastUpdatedInfo ParseLastUpdatedText(string text) { var info = new LastUpdatedInfo(); _logger.LogInformation("Parsing last updated text: {Text}", text); - // Parse observation time - use "minutes ago" as primary source for accuracy - // "Minutes ago" is reliable and doesn't require date guessing - // Time string is used only as fallback if "minutes ago" is unavailable - var minutesAgoMatch = Regex.Match(text, @"Observations:\s*(\d+)\s*minutes?\s*ago", RegexOptions.IgnoreCase); - if (minutesAgoMatch.Success && int.TryParse(minutesAgoMatch.Groups[1].Value, out var minutesAgo)) + // Parse observation time from time string (e.g., "9:40 pm AEST") + // Ignore "minutes ago" - client will calculate this dynamically from UTC timestamp + var observationMatch = Regex.Match(text, @"Observations:\s*(?:\d+\s*minutes?\s*ago)?[,\s]+([\d:]+(?:\s*[ap]m)?)\s+([A-Z]{3,4})(?:at|\s+at|,|$)", RegexOptions.IgnoreCase); + if (observationMatch.Success && observationMatch.Groups.Count >= 3 && observationMatch.Groups[1].Success) { - // Use "minutes ago" as primary source - it's accurate and doesn't require date guessing - info.ObservationTime = DateTime.UtcNow.AddMinutes(-minutesAgo); - _logger.LogInformation("Calculated observation time from 'minutes ago': {MinutesAgo} minutes ago = {Time} UTC", - minutesAgo, info.ObservationTime); - } - else - { - // Fallback to time string parsing only if "minutes ago" is not available - var observationMatch = Regex.Match(text, @"Observations:\s*(?:\d+\s*minutes?\s*ago)?[,\s]+([\d:]+(?:\s*[ap]m)?)\s+([A-Z]{3,4})(?:at|\s+at|,|$)", RegexOptions.IgnoreCase); - if (observationMatch.Success && observationMatch.Groups.Count >= 3 && observationMatch.Groups[1].Success) + var timeStr = observationMatch.Groups[1].Value.Trim(); + var timezoneStr = observationMatch.Groups[2].Success ? observationMatch.Groups[2].Value.Trim() : null; + + // Clean up timezone string - remove "at" if it got concatenated (e.g., "AESTat" -> "AEST") + if (!string.IsNullOrEmpty(timezoneStr) && timezoneStr.EndsWith("at", StringComparison.OrdinalIgnoreCase) && timezoneStr.Length > 2) { - var timeStr = observationMatch.Groups[1].Value.Trim(); - var timezoneStr = observationMatch.Groups[2].Success ? observationMatch.Groups[2].Value.Trim() : null; - - // Clean up timezone string - remove "at" if it got concatenated (e.g., "AESTat" -> "AEST") - if (!string.IsNullOrEmpty(timezoneStr) && timezoneStr.EndsWith("at", StringComparison.OrdinalIgnoreCase) && timezoneStr.Length > 2) - { - timezoneStr = timezoneStr.Substring(0, timezoneStr.Length - 2); - } - - if (TryParseTimeString(timeStr, timezoneStr, out var observationTime)) - { - info.ObservationTime = observationTime; - _logger.LogInformation("Used time string fallback for observation time: {Time} UTC (from '{TimeStr}' {TzStr})", - observationTime, timeStr, timezoneStr ?? "default timezone"); - } - else - { - _logger.LogWarning("Failed to parse observation time from both 'minutes ago' and time string, using current time"); - info.ObservationTime = DateTime.UtcNow; - } + timezoneStr = timezoneStr.Substring(0, timezoneStr.Length - 2); + } + + if (TryParseTimeString(timeStr, timezoneStr, out var observationTime)) + { + info.ObservationTime = observationTime; + _logger.LogInformation("Parsed observation time: {Time} UTC (from '{TimeStr}' {TzStr})", + observationTime, timeStr, timezoneStr ?? "default timezone"); } else { - _logger.LogWarning("No observation time found in text, using current time"); - info.ObservationTime = DateTime.UtcNow; + _logger.LogError("Failed to parse observation time from time string '{TimeStr}' with timezone '{TzStr}'", timeStr, timezoneStr ?? "default"); + throw new InvalidOperationException($"Failed to parse observation time from text: {text}"); } } - - // Parse forecast time - use "minutes ago" or "an hour ago" as primary source - // Handle both "X minutes ago" and "an hour ago" formats - var forecastMinutesAgoMatch = Regex.Match(text, @"Forecast:\s*(\d+)\s*minutes?\s*ago", RegexOptions.IgnoreCase); - var forecastHourAgoMatch = Regex.Match(text, @"Forecast:\s*an\s+hour\s+ago", RegexOptions.IgnoreCase); - - if (forecastMinutesAgoMatch.Success && int.TryParse(forecastMinutesAgoMatch.Groups[1].Value, out var forecastMinutesAgo)) - { - // Use "minutes ago" as primary source - info.ForecastTime = DateTime.UtcNow.AddMinutes(-forecastMinutesAgo); - _logger.LogInformation("Calculated forecast time from 'minutes ago': {MinutesAgo} minutes ago = {Time} UTC", - forecastMinutesAgo, info.ForecastTime); - } - else if (forecastHourAgoMatch.Success) - { - // Handle "an hour ago" format - info.ForecastTime = DateTime.UtcNow.AddHours(-1); - _logger.LogInformation("Calculated forecast time from 'an hour ago': {Time} UTC", info.ForecastTime); - } else { - // Fallback to time string parsing only if "minutes ago" or "an hour ago" is not available - var forecastMatch = Regex.Match(text, @"Forecast:\s*(?:an\s+hour\s+ago|\d+\s*minutes?\s*ago)?[,\s]*([\d:]+(?:\s*[ap]m)?)\s+([A-Z]+)(?:\s+at|$)", RegexOptions.IgnoreCase); - if (forecastMatch.Success && forecastMatch.Groups[1].Success) + _logger.LogError("No observation time found in text: {Text}", text); + throw new InvalidOperationException($"Could not find observation time in text: {text}"); + } + + // Parse forecast time from time string (e.g., "7:50 pm AEST") + // Ignore "minutes ago" - client will calculate this dynamically from UTC timestamp + var forecastMatch = Regex.Match(text, @"Forecast:\s*(?:an\s+hour\s+ago|\d+\s*minutes?\s*ago)?[,\s]*([\d:]+(?:\s*[ap]m)?)\s+([A-Z]+)(?:\s+at|$)", RegexOptions.IgnoreCase); + if (forecastMatch.Success && forecastMatch.Groups[1].Success) + { + var timeStr = forecastMatch.Groups[1].Value.Trim(); + var timezoneStr = forecastMatch.Groups[2].Success ? forecastMatch.Groups[2].Value.Trim() : null; + + if (TryParseTimeString(timeStr, timezoneStr, out var forecastTime)) { - var timeStr = forecastMatch.Groups[1].Value.Trim(); - var timezoneStr = forecastMatch.Groups[2].Success ? forecastMatch.Groups[2].Value.Trim() : null; - - if (TryParseTimeString(timeStr, timezoneStr, out var forecastTime)) - { - info.ForecastTime = forecastTime; - _logger.LogInformation("Used time string fallback for forecast time: {Time} UTC (from '{TimeStr}' {TzStr})", - forecastTime, timeStr, timezoneStr ?? "default timezone"); - } - else - { - _logger.LogWarning("Failed to parse forecast time from both 'minutes ago' and time string, using current time"); - info.ForecastTime = DateTime.UtcNow; - } + info.ForecastTime = forecastTime; + _logger.LogInformation("Parsed forecast time: {Time} UTC (from '{TimeStr}' {TzStr})", + forecastTime, timeStr, timezoneStr ?? "default timezone"); } else { - _logger.LogWarning("No forecast time found in text, using current time"); - info.ForecastTime = DateTime.UtcNow; + _logger.LogError("Failed to parse forecast time from time string '{TimeStr}' with timezone '{TzStr}'", timeStr, timezoneStr ?? "default"); + throw new InvalidOperationException($"Failed to parse forecast time from text: {text}"); } } + else + { + _logger.LogError("No forecast time found in text: {Text}", text); + throw new InvalidOperationException($"Could not find forecast time in text: {text}"); + } // Extract weather station name var stationMatch = Regex.Match(text, @"at\s+([^,]+)\s+weather\s+station", RegexOptions.IgnoreCase); @@ -256,8 +220,7 @@ public class TimeParsingService : ITimeParsingService var dateTimeInTz = nowInTz.Date.Add(localTime.TimeOfDay); // Only adjust to yesterday if the time is clearly in the future - // Don't use the 12-hour threshold - it's too aggressive for recent observations - // This method is now only used as fallback when "minutes ago" is unavailable + // This ensures we get the correct date for the observation/forecast time if (dateTimeInTz > nowInTz) { // Time is in the future, so it must be from yesterday diff --git a/Utilities/ResponseBuilder.cs b/Utilities/ResponseBuilder.cs index 1e22a2f..8309dd0 100644 --- a/Utilities/ResponseBuilder.cs +++ b/Utilities/ResponseBuilder.cs @@ -63,19 +63,8 @@ public static class ResponseBuilder } } - // Calculate AbsoluteObservationTime for all frames if metadata is available + // AbsoluteObservationTime is already set during capture, no calculation needed // This ensures consistency with the time series endpoint - if (metadata != null && metadata.ObservationTime > DateTime.MinValue.AddYears(1)) - { - foreach (var frame in frames) - { - // Only calculate if not already set and MinutesAgo is valid - if (!frame.AbsoluteObservationTime.HasValue && frame.MinutesAgo >= 0) - { - frame.AbsoluteObservationTime = metadata.ObservationTime.AddMinutes(-frame.MinutesAgo); - } - } - } // Calculate NextUpdateTime based on cache status: // - If cache is valid: NextUpdateTime = max(CacheExpiresAt, next background service check) diff --git a/Views/RadarTest/Index.cshtml b/Views/RadarTest/Index.cshtml index c0ac06c..ddda89b 100644 --- a/Views/RadarTest/Index.cshtml +++ b/Views/RadarTest/Index.cshtml @@ -1222,32 +1222,37 @@ }); }); + // Filter out frames without absoluteObservationTime (backend should have filtered these, so this catches bugs) + const invalidFrames = allFrames.filter((frame, idx) => !frame.absoluteObservationTime); + if (invalidFrames.length > 0) { + console.error('Backend returned frames missing absoluteObservationTime (should have been filtered):', { + count: invalidFrames.length, + frames: invalidFrames.map((f, idx) => ({ + index: idx, + frameIndex: f.frameIndex, + cacheFolder: f.cacheFolderName, + imageUrl: f.imageUrl + })) + }); + } + const validFrames = allFrames.filter(frame => frame.absoluteObservationTime); + // Validate chronological order to detect backend issues let previousTime = null; const outOfOrderFrames = []; - allFrames.forEach((frame, idx) => { - if (frame.absoluteObservationTime) { - const currentTime = new Date(frame.absoluteObservationTime).getTime(); - if (previousTime !== null && currentTime < previousTime) { - outOfOrderFrames.push({ - index: idx, - frameIndex: frame.frameIndex, - cacheFolder: frame.cacheFolderName, - absoluteTime: frame.absoluteObservationTime, - previousTime: new Date(previousTime).toISOString(), - timeDiffMinutes: (currentTime - previousTime) / 1000 / 60 - }); - } - previousTime = currentTime; - } else { - // Missing absoluteObservationTime is also an issue - console.warn(`Frame at index ${idx} missing absoluteObservationTime`, { + validFrames.forEach((frame, idx) => { + const currentTime = new Date(frame.absoluteObservationTime).getTime(); + if (previousTime !== null && currentTime < previousTime) { + outOfOrderFrames.push({ + index: idx, frameIndex: frame.frameIndex, cacheFolder: frame.cacheFolderName, - minutesAgo: frame.minutesAgo, - observationTime: frame.observationTime + absoluteTime: frame.absoluteObservationTime, + previousTime: new Date(previousTime).toISOString(), + timeDiffMinutes: (currentTime - previousTime) / 1000 / 60 }); } + previousTime = currentTime; }); if (outOfOrderFrames.length > 0) { @@ -1257,6 +1262,10 @@ }); } + // Use only valid frames going forward + allFrames.length = 0; + allFrames.push(...validFrames); + // Re-index frames sequentially for display allFrames.forEach((frame, idx) => { frame.sequentialIndex = idx; @@ -1988,10 +1997,9 @@ // Update time display const timeDisplay = document.getElementById('current-frame-time'); if (timeDisplay) { - if (isExtendedMode && frame.absoluteObservationTime) { - timeDisplay.textContent = formatDate(frame.absoluteObservationTime); - } else if (frame.minutesAgo !== undefined) { - timeDisplay.textContent = `${frame.minutesAgo} min ago`; + if (frame.absoluteObservationTime) { + const minutesAgo = Math.round((Date.now() - new Date(frame.absoluteObservationTime).getTime()) / 60000); + timeDisplay.textContent = `${minutesAgo} min ago`; } else { timeDisplay.textContent = '-'; } @@ -2075,7 +2083,10 @@ : (frame.cacheTimestamp ? formatDate(frame.cacheTimestamp) : ''); altText = `Radar frame ${frameNum}${timeInfo ? ' (' + timeInfo + ')' : ''}`; } else { - altText = `Radar frame ${frame.frameIndex} (${frame.minutesAgo} minutes ago)`; + const minutesAgo = frame.absoluteObservationTime + ? Math.round((Date.now() - new Date(frame.absoluteObservationTime).getTime()) / 60000) + : null; + altText = `Radar frame ${frame.frameIndex}${minutesAgo !== null ? ` (${minutesAgo} minutes ago)` : ''}`; } // Check if image is already loaded (from preload) @@ -2103,8 +2114,11 @@ document.getElementById('frame-info').textContent = `Frame ${frameNum} of ${frames.length - 1}${timeInfo ? ' • ' + timeInfo : ''}`; } else { + const minutesAgo = frame.absoluteObservationTime + ? Math.round((Date.now() - new Date(frame.absoluteObservationTime).getTime()) / 60000) + : null; document.getElementById('frame-info').textContent = - `Frame ${frame.frameIndex} of ${frames.length - 1} • ${frame.minutesAgo} minutes ago`; + `Frame ${frame.frameIndex} of ${frames.length - 1}${minutesAgo !== null ? ` • ${minutesAgo} minutes ago` : ''}`; } }