diff --git a/IMPLEMENTATION.md b/IMPLEMENTATION.md index 1f00aa8..f975da6 100644 --- a/IMPLEMENTATION.md +++ b/IMPLEMENTATION.md @@ -82,7 +82,7 @@ Each type lives in `src/FrameProcessor/Domain/`. Tests in `tests/FrameProcessor. - Bind via `IOptionsMonitor`. - Custom validator enforcing rules from `SPEC.md` §6.2 (URL-safe name, MAC parseable, palette ≥2, hex parseable, dithering known). -### [ ] 2.3 Startup vs reload asymmetry +### [x] 2.3 Startup vs reload asymmetry - On startup: throw on any invalid frame (fail-fast). - On `OnChange`: log warning, skip invalid frame, keep valid ones serving (`PLAN.md` line 130, `CLAUDE.md` "frames.json reload asymmetry"). - Add a `FramesRegistry` service that exposes `TryGetByName(FrameName)` / `TryGetByMac(MacAddress)` over the current valid set. diff --git a/src/FrameProcessor/Configuration/FramesRegistry.cs b/src/FrameProcessor/Configuration/FramesRegistry.cs new file mode 100644 index 0000000..b71c4c6 --- /dev/null +++ b/src/FrameProcessor/Configuration/FramesRegistry.cs @@ -0,0 +1,148 @@ +using FrameProcessor.Domain; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using SixLabors.ImageSharp; + +namespace FrameProcessor.Configuration; + +/// +/// Holds the current set of valid frames parsed from frames.json and exposes +/// lookups by and . Enforces the +/// startup-vs-reload asymmetry from SPEC.md §6.2 and CLAUDE.md "frames.json reload +/// asymmetry": an invalid frame at startup fails fast; an invalid frame at hot-reload +/// is logged and skipped while the remaining valid frames keep serving. +/// +public sealed class FramesRegistry : IDisposable +{ + private readonly FramesOptionsValidator _validator; + private readonly ILogger _logger; + private readonly IDisposable? _changeSubscription; + private volatile FrameSet _frames; + + public FramesRegistry( + IOptionsMonitor monitor, + FramesOptionsValidator validator, + ILogger logger) + { + _validator = validator; + _logger = logger; + + _frames = BuildStrict(monitor.CurrentValue); + _changeSubscription = monitor.OnChange(OnFramesChanged); + } + + public IReadOnlyCollection All => (IReadOnlyCollection)_frames.ByName.Values; + + public bool TryGetByName(FrameName name, out Frame frame) + => _frames.ByName.TryGetValue(name, out frame!); + + public bool TryGetByMac(MacAddress mac, out Frame frame) + => _frames.ByMac.TryGetValue(mac, out frame!); + + public void Dispose() => _changeSubscription?.Dispose(); + + private FrameSet BuildStrict(FramesOptions options) + { + var result = _validator.Validate(null, options); + if (result.Failed) + { + throw new OptionsValidationException( + nameof(FramesOptions), + typeof(FramesOptions), + result.Failures ?? Array.Empty()); + } + + var byName = new Dictionary(); + var byMac = new Dictionary(); + foreach (var frameOptions in options.Frames) + { + var frame = ToFrame(frameOptions); + byName[frame.Name] = frame; + byMac[frame.Mac] = frame; + } + + return new FrameSet(byName, byMac); + } + + private void OnFramesChanged(FramesOptions options) + { + var byName = new Dictionary(); + var byMac = new Dictionary(); + + for (var i = 0; i < options.Frames.Count; i++) + { + var frameOptions = options.Frames[i]; + var single = new FramesOptions { Frames = { frameOptions } }; + var result = _validator.Validate(null, single); + if (result.Failed) + { + _logger.LogWarning( + "Skipping invalid frame at Frames[{Index}] on reload: {Errors}", + i, + string.Join("; ", result.Failures ?? Array.Empty())); + continue; + } + + var frame = ToFrame(frameOptions); + + if (byName.ContainsKey(frame.Name)) + { + _logger.LogWarning( + "Skipping frame at Frames[{Index}] on reload: duplicate name '{Name}'.", + i, + frame.Name); + continue; + } + + if (byMac.ContainsKey(frame.Mac)) + { + _logger.LogWarning( + "Skipping frame at Frames[{Index}] on reload: duplicate MAC '{Mac}'.", + i, + frame.Mac); + continue; + } + + byName[frame.Name] = frame; + byMac[frame.Mac] = frame; + } + + _frames = new FrameSet(byName, byMac); + _logger.LogInformation("Reloaded frames; {Count} active.", byName.Count); + } + + private static Frame ToFrame(FrameOptions opts) + { + var name = FrameName.Parse(opts.Name!); + var mac = MacAddress.Parse(opts.Mac!); + var resolution = new Resolution(opts.Resolution!.Width, opts.Resolution.Height); + var orientation = opts.Orientation switch + { + "landscape" => Orientation.Landscape, + "portrait" => Orientation.Portrait, + _ => throw new InvalidOperationException( + $"Validator allowed unexpected orientation '{opts.Orientation}'."), + }; + var dithering = opts.Dithering switch + { + "floyd-steinberg" => DitherAlgorithm.FloydSteinberg, + "atkinson" => DitherAlgorithm.Atkinson, + "stucki" => DitherAlgorithm.Stucki, + "jarvis" => DitherAlgorithm.Jarvis, + _ => throw new InvalidOperationException( + $"Validator allowed unexpected dithering '{opts.Dithering}'."), + }; + var palette = opts.Palette + .Select(p => new PaletteEntry( + p.Name!, + Color.ParseHex(p.Color!), + Color.ParseHex(p.DeviceColor!))) + .ToList(); + + return new Frame(name, mac, resolution, orientation, dithering, palette); + } + + private sealed record FrameSet( + IReadOnlyDictionary ByName, + IReadOnlyDictionary ByMac); +} diff --git a/src/FrameProcessor/Domain/Frame.cs b/src/FrameProcessor/Domain/Frame.cs new file mode 100644 index 0000000..90484b8 --- /dev/null +++ b/src/FrameProcessor/Domain/Frame.cs @@ -0,0 +1,14 @@ +namespace FrameProcessor.Domain; + +/// +/// A configured frame after validation and parsing. All raw strings from +/// frames.json have been converted to their typed values; consumers can use +/// the contents directly without re-validating. +/// +public sealed record Frame( + FrameName Name, + MacAddress Mac, + Resolution Resolution, + Orientation Orientation, + DitherAlgorithm Dithering, + IReadOnlyList Palette); diff --git a/src/FrameProcessor/Program.cs b/src/FrameProcessor/Program.cs index d20bc1b..76852f5 100644 --- a/src/FrameProcessor/Program.cs +++ b/src/FrameProcessor/Program.cs @@ -1,5 +1,4 @@ using FrameProcessor.Configuration; -using Microsoft.Extensions.Options; var builder = WebApplication.CreateBuilder(args); @@ -27,13 +26,20 @@ builder.Services.AddOptions() .ValidateDataAnnotations() .ValidateOnStart(); -builder.Services.AddSingleton, FramesOptionsValidator>(); +// FramesOptions is bound but not validated via the options pipeline so that +// IOptionsMonitor can fire OnChange with invalid content during +// hot-reload without throwing. FramesRegistry takes responsibility for both +// strict startup validation (in its constructor) and lenient reload validation. builder.Services.AddOptions() - .Bind(builder.Configuration) - .ValidateOnStart(); + .Bind(builder.Configuration); +builder.Services.AddSingleton(); +builder.Services.AddSingleton(); var app = builder.Build(); +// Eagerly resolve FramesRegistry so an invalid frames.json fails startup fast. +_ = app.Services.GetRequiredService(); + app.MapControllers(); app.Run(); diff --git a/tests/FrameProcessor.Tests/FramesRegistryTests.cs b/tests/FrameProcessor.Tests/FramesRegistryTests.cs new file mode 100644 index 0000000..500d4e5 --- /dev/null +++ b/tests/FrameProcessor.Tests/FramesRegistryTests.cs @@ -0,0 +1,289 @@ +using FrameProcessor.Configuration; +using FrameProcessor.Domain; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; + +namespace FrameProcessor.Tests; + +public class FramesRegistryTests +{ + [Fact] + public void Construction_StrictOnStartup_ThrowsOnInvalidFrame() + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { Invalid() }, + }); + + var ex = Assert.Throws( + () => new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance)); + + Assert.NotEmpty(ex.Failures); + } + + [Fact] + public void Construction_StrictOnStartup_FailsIfOneFrameIsBad() + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom(), Invalid() }, + }); + + Assert.Throws( + () => new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance)); + } + + [Fact] + public void TryGetByName_FindsParsedFrame() + { + var registry = BuildWith(LivingRoom()); + + Assert.True(registry.TryGetByName(FrameName.Parse("living-room"), out var frame)); + Assert.Equal("living-room", frame.Name.Value); + Assert.Equal("aabbccddeeff", frame.Mac.ToString()); + Assert.Equal(1600, frame.Resolution.Width); + Assert.Equal(1200, frame.Resolution.Height); + Assert.Equal(Orientation.Landscape, frame.Orientation); + Assert.Equal(DitherAlgorithm.FloydSteinberg, frame.Dithering); + Assert.Equal(2, frame.Palette.Count); + } + + [Fact] + public void TryGetByMac_LooksUpRegardlessOfInputFormat() + { + var registry = BuildWith(LivingRoom()); + + Assert.True(MacAddress.TryParse("AA-BB-CC-DD-EE-FF", out var mac)); + Assert.True(registry.TryGetByMac(mac, out var frame)); + Assert.Equal("living-room", frame.Name.Value); + } + + [Fact] + public void TryGetByName_UnknownReturnsFalse() + { + var registry = BuildWith(LivingRoom()); + + Assert.False(registry.TryGetByName(FrameName.Parse("kitchen"), out _)); + } + + [Fact] + public void TryGetByMac_UnknownReturnsFalse() + { + var registry = BuildWith(LivingRoom()); + + Assert.True(MacAddress.TryParse("11:22:33:44:55:66", out var mac)); + Assert.False(registry.TryGetByMac(mac, out _)); + } + + [Fact] + public void Reload_DropsInvalidFrameAndKeepsValidOnes() + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance); + + monitor.Emit(new FramesOptions + { + Frames = { LivingRoom(), Invalid(), Kitchen() }, + }); + + Assert.True(registry.TryGetByName(FrameName.Parse("living-room"), out _)); + Assert.True(registry.TryGetByName(FrameName.Parse("kitchen"), out _)); + Assert.Equal(2, registry.All.Count); + } + + [Fact] + public void Reload_DoesNotThrowOnAllInvalid() + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance); + + monitor.Emit(new FramesOptions { Frames = { Invalid() } }); + + Assert.Empty(registry.All); + } + + [Fact] + public void Reload_LogsWarningForSkippedFrame() + { + var logger = new ListLogger(); + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), logger); + + monitor.Emit(new FramesOptions { Frames = { LivingRoom(), Invalid() } }); + + Assert.Contains(logger.Entries, + e => e.Level == LogLevel.Warning && e.Message.Contains("Skipping invalid frame")); + } + + [Fact] + public void Reload_SkipsDuplicateNameAcrossSurvivingFrames() + { + var logger = new ListLogger(); + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), logger); + + var firstWithName = LivingRoom(); + firstWithName.Mac = "11:22:33:44:55:66"; + var secondWithSameName = LivingRoom(); + secondWithSameName.Mac = "77:88:99:AA:BB:CC"; + + monitor.Emit(new FramesOptions { Frames = { firstWithName, secondWithSameName } }); + + Assert.Single(registry.All); + Assert.Contains(logger.Entries, + e => e.Level == LogLevel.Warning && e.Message.Contains("duplicate name")); + } + + [Fact] + public void Reload_SkipsDuplicateMacAcrossSurvivingFrames() + { + var logger = new ListLogger(); + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), logger); + + var firstWithMac = LivingRoom(); + var secondWithSameMac = LivingRoom(); + secondWithSameMac.Name = "kitchen"; + secondWithSameMac.Mac = "aabbccddeeff"; + + monitor.Emit(new FramesOptions { Frames = { firstWithMac, secondWithSameMac } }); + + Assert.Single(registry.All); + Assert.Contains(logger.Entries, + e => e.Level == LogLevel.Warning && e.Message.Contains("duplicate MAC")); + } + + [Fact] + public void Dispose_UnsubscribesFromMonitor() + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = { LivingRoom() }, + }); + var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance); + + registry.Dispose(); + + monitor.Emit(new FramesOptions { Frames = { LivingRoom(), Kitchen() } }); + + Assert.Single(registry.All); + Assert.True(registry.TryGetByName(FrameName.Parse("living-room"), out _)); + } + + private static FramesRegistry BuildWith(params FrameOptions[] frames) + { + var monitor = new TestOptionsMonitor(new FramesOptions + { + Frames = frames.ToList(), + }); + return new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger.Instance); + } + + private static FrameOptions LivingRoom() => new() + { + Name = "living-room", + Mac = "AA:BB:CC:DD:EE:FF", + Resolution = new ResolutionOptions { Width = 1600, Height = 1200 }, + Orientation = "landscape", + Dithering = "floyd-steinberg", + Palette = new List + { + new() { Name = "black", Color = "#1F2226", DeviceColor = "#000000" }, + new() { Name = "white", Color = "#B9C7C9", DeviceColor = "#FFFFFF" }, + }, + }; + + private static FrameOptions Kitchen() => new() + { + Name = "kitchen", + Mac = "11:22:33:44:55:66", + Resolution = new ResolutionOptions { Width = 800, Height = 600 }, + Orientation = "portrait", + Dithering = "atkinson", + Palette = new List + { + new() { Name = "black", Color = "#000000", DeviceColor = "#000000" }, + new() { Name = "white", Color = "#FFFFFF", DeviceColor = "#FFFFFF" }, + }, + }; + + private static FrameOptions Invalid() => new() + { + Name = "bad name", + Mac = "not-a-mac", + Resolution = null, + Orientation = "diagonal", + Dithering = "ordered", + Palette = new List(), + }; + + private sealed class TestOptionsMonitor : IOptionsMonitor + { + private T _current; + private Action? _listeners; + + public TestOptionsMonitor(T initial) { _current = initial; } + + public T CurrentValue => _current; + + public T Get(string? name) => _current; + + public IDisposable? OnChange(Action listener) + { + _listeners += listener; + return new Unsubscriber(() => _listeners -= listener); + } + + public void Emit(T newValue) + { + _current = newValue; + _listeners?.Invoke(newValue, null); + } + + private sealed class Unsubscriber : IDisposable + { + private readonly Action _onDispose; + + public Unsubscriber(Action onDispose) { _onDispose = onDispose; } + + public void Dispose() => _onDispose(); + } + } + + private sealed record LogEntry(LogLevel Level, string Message); + + private sealed class ListLogger : ILogger + { + public List Entries { get; } = new(); + + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log( + LogLevel logLevel, + EventId eventId, + TState state, + Exception? exception, + Func formatter) + { + Entries.Add(new LogEntry(logLevel, formatter(state, exception))); + } + } +}