2.3 Startup vs reload asymmetry (FramesRegistry)
FramesRegistry validates strictly on construction (fail-fast at startup) and leniently on hot-reload (skip invalid frames with a warning, keep valid ones serving). Exposes TryGetByName/TryGetByMac over the current valid set. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -82,7 +82,7 @@ Each type lives in `src/FrameProcessor/Domain/`. Tests in `tests/FrameProcessor.
|
||||
- Bind via `IOptionsMonitor<FramesOptions>`.
|
||||
- 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.
|
||||
|
||||
148
src/FrameProcessor/Configuration/FramesRegistry.cs
Normal file
148
src/FrameProcessor/Configuration/FramesRegistry.cs
Normal file
@@ -0,0 +1,148 @@
|
||||
using FrameProcessor.Domain;
|
||||
using Microsoft.Extensions.Logging;
|
||||
using Microsoft.Extensions.Options;
|
||||
using SixLabors.ImageSharp;
|
||||
|
||||
namespace FrameProcessor.Configuration;
|
||||
|
||||
/// <summary>
|
||||
/// Holds the current set of valid frames parsed from <c>frames.json</c> and exposes
|
||||
/// lookups by <see cref="FrameName"/> and <see cref="MacAddress"/>. 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.
|
||||
/// </summary>
|
||||
public sealed class FramesRegistry : IDisposable
|
||||
{
|
||||
private readonly FramesOptionsValidator _validator;
|
||||
private readonly ILogger<FramesRegistry> _logger;
|
||||
private readonly IDisposable? _changeSubscription;
|
||||
private volatile FrameSet _frames;
|
||||
|
||||
public FramesRegistry(
|
||||
IOptionsMonitor<FramesOptions> monitor,
|
||||
FramesOptionsValidator validator,
|
||||
ILogger<FramesRegistry> logger)
|
||||
{
|
||||
_validator = validator;
|
||||
_logger = logger;
|
||||
|
||||
_frames = BuildStrict(monitor.CurrentValue);
|
||||
_changeSubscription = monitor.OnChange(OnFramesChanged);
|
||||
}
|
||||
|
||||
public IReadOnlyCollection<Frame> All => (IReadOnlyCollection<Frame>)_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<string>());
|
||||
}
|
||||
|
||||
var byName = new Dictionary<FrameName, Frame>();
|
||||
var byMac = new Dictionary<MacAddress, Frame>();
|
||||
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<FrameName, Frame>();
|
||||
var byMac = new Dictionary<MacAddress, Frame>();
|
||||
|
||||
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<string>()));
|
||||
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<FrameName, Frame> ByName,
|
||||
IReadOnlyDictionary<MacAddress, Frame> ByMac);
|
||||
}
|
||||
14
src/FrameProcessor/Domain/Frame.cs
Normal file
14
src/FrameProcessor/Domain/Frame.cs
Normal file
@@ -0,0 +1,14 @@
|
||||
namespace FrameProcessor.Domain;
|
||||
|
||||
/// <summary>
|
||||
/// A configured frame after validation and parsing. All raw strings from
|
||||
/// <c>frames.json</c> have been converted to their typed values; consumers can use
|
||||
/// the contents directly without re-validating.
|
||||
/// </summary>
|
||||
public sealed record Frame(
|
||||
FrameName Name,
|
||||
MacAddress Mac,
|
||||
Resolution Resolution,
|
||||
Orientation Orientation,
|
||||
DitherAlgorithm Dithering,
|
||||
IReadOnlyList<PaletteEntry> Palette);
|
||||
@@ -1,5 +1,4 @@
|
||||
using FrameProcessor.Configuration;
|
||||
using Microsoft.Extensions.Options;
|
||||
|
||||
var builder = WebApplication.CreateBuilder(args);
|
||||
|
||||
@@ -27,13 +26,20 @@ builder.Services.AddOptions<ApiKeyOptions>()
|
||||
.ValidateDataAnnotations()
|
||||
.ValidateOnStart();
|
||||
|
||||
builder.Services.AddSingleton<IValidateOptions<FramesOptions>, FramesOptionsValidator>();
|
||||
// FramesOptions is bound but not validated via the options pipeline so that
|
||||
// IOptionsMonitor<FramesOptions> 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<FramesOptions>()
|
||||
.Bind(builder.Configuration)
|
||||
.ValidateOnStart();
|
||||
.Bind(builder.Configuration);
|
||||
builder.Services.AddSingleton<FramesOptionsValidator>();
|
||||
builder.Services.AddSingleton<FramesRegistry>();
|
||||
|
||||
var app = builder.Build();
|
||||
|
||||
// Eagerly resolve FramesRegistry so an invalid frames.json fails startup fast.
|
||||
_ = app.Services.GetRequiredService<FramesRegistry>();
|
||||
|
||||
app.MapControllers();
|
||||
|
||||
app.Run();
|
||||
|
||||
289
tests/FrameProcessor.Tests/FramesRegistryTests.cs
Normal file
289
tests/FrameProcessor.Tests/FramesRegistryTests.cs
Normal file
@@ -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<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = { Invalid() },
|
||||
});
|
||||
|
||||
var ex = Assert.Throws<OptionsValidationException>(
|
||||
() => new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.Instance));
|
||||
|
||||
Assert.NotEmpty(ex.Failures);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Construction_StrictOnStartup_FailsIfOneFrameIsBad()
|
||||
{
|
||||
var monitor = new TestOptionsMonitor<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = { LivingRoom(), Invalid() },
|
||||
});
|
||||
|
||||
Assert.Throws<OptionsValidationException>(
|
||||
() => new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.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<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = { LivingRoom() },
|
||||
});
|
||||
var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.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<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = { LivingRoom() },
|
||||
});
|
||||
var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.Instance);
|
||||
|
||||
monitor.Emit(new FramesOptions { Frames = { Invalid() } });
|
||||
|
||||
Assert.Empty(registry.All);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Reload_LogsWarningForSkippedFrame()
|
||||
{
|
||||
var logger = new ListLogger<FramesRegistry>();
|
||||
var monitor = new TestOptionsMonitor<FramesOptions>(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<FramesRegistry>();
|
||||
var monitor = new TestOptionsMonitor<FramesOptions>(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<FramesRegistry>();
|
||||
var monitor = new TestOptionsMonitor<FramesOptions>(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<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = { LivingRoom() },
|
||||
});
|
||||
var registry = new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.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<FramesOptions>(new FramesOptions
|
||||
{
|
||||
Frames = frames.ToList(),
|
||||
});
|
||||
return new FramesRegistry(monitor, new FramesOptionsValidator(), NullLogger<FramesRegistry>.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<PaletteEntryOptions>
|
||||
{
|
||||
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<PaletteEntryOptions>
|
||||
{
|
||||
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<PaletteEntryOptions>(),
|
||||
};
|
||||
|
||||
private sealed class TestOptionsMonitor<T> : IOptionsMonitor<T>
|
||||
{
|
||||
private T _current;
|
||||
private Action<T, string?>? _listeners;
|
||||
|
||||
public TestOptionsMonitor(T initial) { _current = initial; }
|
||||
|
||||
public T CurrentValue => _current;
|
||||
|
||||
public T Get(string? name) => _current;
|
||||
|
||||
public IDisposable? OnChange(Action<T, string?> 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<T> : ILogger<T>
|
||||
{
|
||||
public List<LogEntry> Entries { get; } = new();
|
||||
|
||||
public IDisposable? BeginScope<TState>(TState state) where TState : notnull => null;
|
||||
|
||||
public bool IsEnabled(LogLevel logLevel) => true;
|
||||
|
||||
public void Log<TState>(
|
||||
LogLevel logLevel,
|
||||
EventId eventId,
|
||||
TState state,
|
||||
Exception? exception,
|
||||
Func<TState, Exception?, string> formatter)
|
||||
{
|
||||
Entries.Add(new LogEntry(logLevel, formatter(state, exception)));
|
||||
}
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user