mbproxy: close out the dashboard code-review minor findings
Resolves the remaining Minor items from the 2026-05-15 review so the web-UI dashboard work has no open follow-ups: a real-HubConnection end-to-end test for the SignalR feed, stable mbproxy.admin.broadcast.* log-event names, keyboard/aria accessibility on the fleet table, frontend JS hardening (URL-decode guard, NaN guards, shared util.js), reconciler<->capture-registry coverage, throwing-sink and embedded-asset tests, broadcaster polish, and a soft upper bound on AdminPushIntervalMs. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Joseph Doherty
@@ -243,7 +243,22 @@ public sealed class AdminEndpointTests
|
||||
response.Headers.CacheControl?.ToString().ShouldContain("immutable");
|
||||
|
||||
var bytes = await response.Content.ReadAsByteArrayAsync(TestContext.Current.CancellationToken);
|
||||
bytes.Length.ShouldBeGreaterThan(0);
|
||||
|
||||
// The served bytes must be the actual embedded asset — not some other resource
|
||||
// of the same length. Compare against the manifest resource directly.
|
||||
byte[] expected = ReadEmbeddedAsset(file);
|
||||
bytes.ShouldBe(expected, $"GET /assets/{file} must return the embedded asset verbatim");
|
||||
}
|
||||
|
||||
/// <summary>Reads a <c>wwwroot</c> asset straight from the assembly's manifest resources.</summary>
|
||||
private static byte[] ReadEmbeddedAsset(string fileName)
|
||||
{
|
||||
using var stream = typeof(Mbproxy.Admin.StatusHub).Assembly
|
||||
.GetManifestResourceStream("Mbproxy.Admin.wwwroot." + fileName)
|
||||
?? throw new InvalidOperationException($"Embedded asset not found: {fileName}");
|
||||
using var ms = new MemoryStream();
|
||||
stream.CopyTo(ms);
|
||||
return ms.ToArray();
|
||||
}
|
||||
|
||||
[Fact(Timeout = 5_000)]
|
||||
|
||||
@@ -0,0 +1,63 @@
|
||||
using System.Reflection;
|
||||
using Mbproxy.Admin;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace Mbproxy.Tests.Admin;
|
||||
|
||||
/// <summary>
|
||||
/// Guards the <c>Admin\wwwroot\*.*</c> embedded-resource glob in <c>Mbproxy.csproj</c>.
|
||||
/// A broken or narrowed glob would silently drop a UI asset from the single-file binary;
|
||||
/// the admin endpoint would then 404 it at runtime with no compile-time failure. This
|
||||
/// test fails the build instead by comparing the on-disk source folder against the
|
||||
/// assembly's manifest resources.
|
||||
/// </summary>
|
||||
[Trait("Category", "Unit")]
|
||||
public sealed class EmbeddedAssetsTests
|
||||
{
|
||||
private const string ResourcePrefix = "Mbproxy.Admin.wwwroot.";
|
||||
|
||||
[Fact]
|
||||
public void EveryWwwrootFile_IsEmbeddedAsAManifestResource()
|
||||
{
|
||||
var sourceDir = LocateWwwrootSource();
|
||||
var sourceFiles = Directory.GetFiles(sourceDir)
|
||||
.Select(Path.GetFileName)
|
||||
.Where(n => n is not null)
|
||||
.Select(n => n!)
|
||||
.ToArray();
|
||||
|
||||
sourceFiles.ShouldNotBeEmpty("the source wwwroot folder should contain UI assets");
|
||||
|
||||
var embedded = typeof(StatusHub).Assembly
|
||||
.GetManifestResourceNames()
|
||||
.Where(n => n.StartsWith(ResourcePrefix, StringComparison.Ordinal))
|
||||
.ToHashSet(StringComparer.Ordinal);
|
||||
|
||||
foreach (var file in sourceFiles)
|
||||
{
|
||||
embedded.ShouldContain(ResourcePrefix + file,
|
||||
$"wwwroot asset '{file}' is not embedded — check the EmbeddedResource glob in Mbproxy.csproj");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Walks up from the test assembly directory to the repo and returns
|
||||
/// <c>src/Mbproxy/Admin/wwwroot</c> — same upward-search pattern the simulator
|
||||
/// fixture uses to find <c>tests/sim</c>.
|
||||
/// </summary>
|
||||
private static string LocateWwwrootSource()
|
||||
{
|
||||
var dir = new DirectoryInfo(
|
||||
Path.GetDirectoryName(Assembly.GetExecutingAssembly().Location) ?? ".");
|
||||
while (dir is not null)
|
||||
{
|
||||
var candidate = Path.Combine(dir.FullName, "src", "Mbproxy", "Admin", "wwwroot");
|
||||
if (Directory.Exists(candidate))
|
||||
return candidate;
|
||||
dir = dir.Parent;
|
||||
}
|
||||
throw new DirectoryNotFoundException(
|
||||
"Could not locate src/Mbproxy/Admin/wwwroot above the test assembly.");
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,160 @@
|
||||
using System.Net;
|
||||
using System.Net.Http;
|
||||
using System.Net.Sockets;
|
||||
using System.Text.Json;
|
||||
using Mbproxy.Options;
|
||||
using Mbproxy.Proxy;
|
||||
using Microsoft.AspNetCore.SignalR.Client;
|
||||
using Microsoft.Extensions.Configuration;
|
||||
using Microsoft.Extensions.DependencyInjection;
|
||||
using Microsoft.Extensions.Hosting;
|
||||
using Serilog;
|
||||
using Shouldly;
|
||||
using Xunit;
|
||||
|
||||
namespace Mbproxy.Tests.Admin;
|
||||
|
||||
/// <summary>
|
||||
/// End-to-end test of the SignalR live feed: a real <see cref="HubConnection"/> against
|
||||
/// the live Kestrel admin host. Exercises the whole push path that no other test covers —
|
||||
/// <see cref="Mbproxy.Admin.StatusHub"/> group joins, the <c>MapHub</c> wiring, the
|
||||
/// <see cref="Mbproxy.Admin.SignalRStatusPushSink"/>, and the broadcaster loop —
|
||||
/// confirming that <c>SubscribeFleet</c> yields a <c>"fleet"</c> message and
|
||||
/// <c>SubscribePlc</c> yields a <c>"plc"</c> message.
|
||||
/// </summary>
|
||||
[Trait("Category", "E2E")]
|
||||
public sealed class HubStatusE2ETests
|
||||
{
|
||||
[Fact(Timeout = 15_000)]
|
||||
public async Task SubscribeFleet_ReceivesFleetSnapshot()
|
||||
{
|
||||
int adminPort = PickFreePort();
|
||||
int proxyPort = PickFreePort();
|
||||
|
||||
await using var host = BuildHost(adminPort, proxyPort);
|
||||
using var startCts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
|
||||
await host.Host.StartAsync(startCts.Token);
|
||||
await WaitForAdminAsync(adminPort);
|
||||
|
||||
await using var connection = new HubConnectionBuilder()
|
||||
.WithUrl($"http://127.0.0.1:{adminPort}/hub/status")
|
||||
.Build();
|
||||
|
||||
var fleet = new TaskCompletionSource<JsonElement>(
|
||||
TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
connection.On<JsonElement>("fleet", payload => fleet.TrySetResult(payload));
|
||||
|
||||
await connection.StartAsync(TestContext.Current.CancellationToken);
|
||||
await connection.InvokeAsync("SubscribeFleet", TestContext.Current.CancellationToken);
|
||||
|
||||
var snapshot = await fleet.Task.WaitAsync(
|
||||
TimeSpan.FromSeconds(8), TestContext.Current.CancellationToken);
|
||||
|
||||
// The fleet payload is a StatusResponse — assert a couple of its known fields.
|
||||
snapshot.TryGetProperty("service", out _).ShouldBeTrue("fleet payload must carry 'service'");
|
||||
snapshot.TryGetProperty("plcs", out var plcs).ShouldBeTrue("fleet payload must carry 'plcs'");
|
||||
plcs.ValueKind.ShouldBe(JsonValueKind.Array);
|
||||
}
|
||||
|
||||
[Fact(Timeout = 15_000)]
|
||||
public async Task SubscribePlc_ReceivesDetailSnapshot()
|
||||
{
|
||||
int adminPort = PickFreePort();
|
||||
int proxyPort = PickFreePort();
|
||||
|
||||
await using var host = BuildHost(adminPort, proxyPort);
|
||||
using var startCts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
|
||||
await host.Host.StartAsync(startCts.Token);
|
||||
await WaitForAdminAsync(adminPort);
|
||||
|
||||
await using var connection = new HubConnectionBuilder()
|
||||
.WithUrl($"http://127.0.0.1:{adminPort}/hub/status")
|
||||
.Build();
|
||||
|
||||
var detail = new TaskCompletionSource<JsonElement>(
|
||||
TaskCreationOptions.RunContinuationsAsynchronously);
|
||||
connection.On<JsonElement>("plc", payload => detail.TrySetResult(payload));
|
||||
|
||||
await connection.StartAsync(TestContext.Current.CancellationToken);
|
||||
// tabId is a stable per-page-load identifier the real client generates.
|
||||
await connection.InvokeAsync("SubscribePlc", "TestPLC", "tab-e2e",
|
||||
TestContext.Current.CancellationToken);
|
||||
|
||||
var snapshot = await detail.Task.WaitAsync(
|
||||
TimeSpan.FromSeconds(8), TestContext.Current.CancellationToken);
|
||||
|
||||
// The detail payload is a PlcDetailResponse { plc, debug }.
|
||||
snapshot.TryGetProperty("debug", out var debug).ShouldBeTrue("detail payload must carry 'debug'");
|
||||
debug.TryGetProperty("captureArmed", out _).ShouldBeTrue("debug must carry 'captureArmed'");
|
||||
debug.TryGetProperty("tags", out var tags).ShouldBeTrue("debug must carry 'tags'");
|
||||
tags.ValueKind.ShouldBe(JsonValueKind.Array);
|
||||
}
|
||||
|
||||
// ── Helpers ───────────────────────────────────────────────────────────────
|
||||
|
||||
private static HostHandle BuildHost(int adminPort, int proxyPort)
|
||||
{
|
||||
var config = new Dictionary<string, string?>
|
||||
{
|
||||
["Mbproxy:AdminPort"] = adminPort.ToString(),
|
||||
// Fast push cadence so the subscribed client sees a message promptly.
|
||||
["Mbproxy:AdminPushIntervalMs"] = "100",
|
||||
["Mbproxy:Plcs:0:Name"] = "TestPLC",
|
||||
["Mbproxy:Plcs:0:ListenPort"] = proxyPort.ToString(),
|
||||
["Mbproxy:Plcs:0:Host"] = "127.0.0.1",
|
||||
["Mbproxy:Plcs:0:Port"] = "502",
|
||||
["Mbproxy:Connection:BackendConnectTimeoutMs"] = "500",
|
||||
["Mbproxy:Connection:BackendRequestTimeoutMs"] = "500",
|
||||
};
|
||||
|
||||
var builder = Host.CreateApplicationBuilder();
|
||||
builder.Configuration.AddInMemoryCollection(config);
|
||||
builder.Services.AddSerilog(
|
||||
new LoggerConfiguration().MinimumLevel.Fatal().CreateLogger(), dispose: false);
|
||||
builder.AddMbproxyOptions();
|
||||
builder.Services.AddSingleton<IPduPipeline, NoopPduPipeline>();
|
||||
builder.Services.AddSingleton<ProxyWorker>();
|
||||
builder.Services.AddHostedService(sp => sp.GetRequiredService<ProxyWorker>());
|
||||
builder.AddMbproxyAdmin();
|
||||
|
||||
return new HostHandle(builder.Build());
|
||||
}
|
||||
|
||||
private static async Task WaitForAdminAsync(int adminPort)
|
||||
{
|
||||
using var http = new HttpClient();
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
|
||||
while (!cts.IsCancellationRequested)
|
||||
{
|
||||
try
|
||||
{
|
||||
var r = await http.GetAsync($"http://127.0.0.1:{adminPort}/status.json", cts.Token);
|
||||
if (r.StatusCode == HttpStatusCode.OK) return;
|
||||
}
|
||||
catch { }
|
||||
await Task.Delay(100, cts.Token).ConfigureAwait(false);
|
||||
}
|
||||
throw new TimeoutException($"Admin endpoint on port {adminPort} did not start in time.");
|
||||
}
|
||||
|
||||
private static int PickFreePort()
|
||||
{
|
||||
var l = new TcpListener(IPAddress.Loopback, 0);
|
||||
l.Start();
|
||||
int port = ((IPEndPoint)l.LocalEndpoint).Port;
|
||||
l.Stop();
|
||||
return port;
|
||||
}
|
||||
|
||||
private sealed class HostHandle(IHost host) : IAsyncDisposable
|
||||
{
|
||||
public IHost Host { get; } = host;
|
||||
|
||||
public async ValueTask DisposeAsync()
|
||||
{
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(5));
|
||||
try { await Host.StopAsync(cts.Token); } catch { }
|
||||
Host.Dispose();
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -64,3 +64,18 @@ internal sealed class FakeStatusPushSink : IStatusPushSink
|
||||
return Task.CompletedTask;
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// An <see cref="IStatusPushSink"/> whose every push fails with an exception from the
|
||||
/// supplied factory — used to prove <see cref="StatusBroadcaster.PushOnceAsync"/> swallows
|
||||
/// a transport fault (and that its <c>when (ex is not OperationCanceledException)</c> filter
|
||||
/// still lets a cancellation propagate).
|
||||
/// </summary>
|
||||
internal sealed class ThrowingStatusPushSink(Func<Exception> exceptionFactory) : IStatusPushSink
|
||||
{
|
||||
public Task PushFleetAsync(StatusResponse snapshot, CancellationToken ct)
|
||||
=> throw exceptionFactory();
|
||||
|
||||
public Task PushPlcAsync(string plcName, PlcDetailResponse detail, CancellationToken ct)
|
||||
=> throw exceptionFactory();
|
||||
}
|
||||
|
||||
@@ -39,7 +39,7 @@ public sealed class StatusBroadcasterTests
|
||||
}
|
||||
}
|
||||
|
||||
private static async Task<Harness> BuildAsync()
|
||||
private static async Task<Harness> BuildAsync(IStatusPushSink? sinkOverride = null)
|
||||
{
|
||||
var hostBuilder = Host.CreateApplicationBuilder();
|
||||
hostBuilder.Configuration.AddInMemoryCollection(new Dictionary<string, string?>
|
||||
@@ -68,7 +68,7 @@ public sealed class StatusBroadcasterTests
|
||||
var sink = new FakeStatusPushSink();
|
||||
|
||||
var broadcaster = new StatusBroadcaster(
|
||||
sink, builder, tracker, registry, options, NullLogger.Instance);
|
||||
sinkOverride ?? sink, builder, tracker, registry, options, NullLogger.Instance);
|
||||
|
||||
return new Harness(host, broadcaster, sink, builder, registry, tracker);
|
||||
}
|
||||
@@ -129,6 +129,31 @@ public sealed class StatusBroadcasterTests
|
||||
capture.IsArmed.ShouldBeFalse("the broadcaster disarms a capture once its last viewer leaves");
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PushOnce_SinkThrowsNonCancellation_FailureIsSwallowed()
|
||||
{
|
||||
// A SignalR transport fault on a push must not escape PushOnceAsync — the loop
|
||||
// has to survive it and retry on the next cycle.
|
||||
var throwing = new ThrowingStatusPushSink(() => new InvalidOperationException("boom"));
|
||||
await using var h = await BuildAsync(throwing);
|
||||
|
||||
await Should.NotThrowAsync(
|
||||
() => h.Broadcaster.PushOnceAsync(TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task PushOnce_SinkThrowsOperationCanceled_Propagates()
|
||||
{
|
||||
// The catch filters are `when (ex is not OperationCanceledException)` — a genuine
|
||||
// cancellation must propagate so the loop unwinds at shutdown instead of being
|
||||
// swallowed and retried.
|
||||
var throwing = new ThrowingStatusPushSink(() => new OperationCanceledException());
|
||||
await using var h = await BuildAsync(throwing);
|
||||
|
||||
await Should.ThrowAsync<OperationCanceledException>(
|
||||
() => h.Broadcaster.PushOnceAsync(TestContext.Current.CancellationToken));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public async Task StopAsync_DisarmsEveryCapture()
|
||||
{
|
||||
|
||||
@@ -68,13 +68,14 @@ public sealed class ConfigReconcilerTests : IAsyncDisposable
|
||||
|
||||
private ConfigReconciler BuildReconciler(
|
||||
IOptionsMonitor<MbproxyOptions> monitor,
|
||||
ServiceCounters? counters = null)
|
||||
ServiceCounters? counters = null,
|
||||
Mbproxy.Proxy.TagCaptureRegistry? captureRegistry = null)
|
||||
{
|
||||
return new ConfigReconciler(
|
||||
monitor,
|
||||
NullLoggerFactory.Instance,
|
||||
counters ?? new ServiceCounters(),
|
||||
new Mbproxy.Proxy.TagCaptureRegistry());
|
||||
captureRegistry ?? new Mbproxy.Proxy.TagCaptureRegistry());
|
||||
}
|
||||
|
||||
// The reconciler and supervisors tracked for cleanup.
|
||||
@@ -347,6 +348,53 @@ public sealed class ConfigReconcilerTests : IAsyncDisposable
|
||||
foreach (var s in supervisors.Values)
|
||||
_supervisors.Add(s);
|
||||
}
|
||||
|
||||
// ── Test 6: Reconciler ↔ TagCaptureRegistry wiring ────────────────────────────────────
|
||||
|
||||
/// <summary>
|
||||
/// The reconciler owns the tag-capture lifecycle for hot-reload: a PLC added by a
|
||||
/// reload must get a capture entry (<c>GetOrCreate</c>), and a PLC removed by a
|
||||
/// reload must have its capture entry dropped (<c>Remove</c>). Holds a real
|
||||
/// <see cref="Mbproxy.Proxy.TagCaptureRegistry"/> and asserts <c>TryGet</c> tracks
|
||||
/// the roster across an add reload and a remove reload.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public async Task Apply_AddThenRemovePlc_TagCaptureRegistryTracksRoster()
|
||||
{
|
||||
int portA = PickFreePort();
|
||||
int portB = PickFreePort();
|
||||
|
||||
var plcA = MakePlc("A", portA);
|
||||
var plcB = MakePlc("B", portB);
|
||||
var initial = MakeOptions([plcA]);
|
||||
var withB = MakeOptions([plcA, plcB]);
|
||||
|
||||
var supA = BuildSupervisor(plcA);
|
||||
_supervisors.Add(supA);
|
||||
await supA.StartAsync(CancellationToken.None);
|
||||
|
||||
var supervisors = new ConcurrentDictionary<string, PlcListenerSupervisor>(StringComparer.Ordinal)
|
||||
{
|
||||
["A"] = supA,
|
||||
};
|
||||
|
||||
var registry = new Mbproxy.Proxy.TagCaptureRegistry();
|
||||
var monitor = new FakeOptionsMonitor(initial);
|
||||
var reconciler = BuildReconciler(monitor, captureRegistry: registry);
|
||||
_reconcilers.Add(reconciler);
|
||||
reconciler.Attach(supervisors, initial);
|
||||
|
||||
using var cts = new CancellationTokenSource(TimeSpan.FromSeconds(10));
|
||||
|
||||
// Reload that adds PLC-B → the registry must gain a capture for B.
|
||||
Assert.True(await reconciler.ApplyAsync(withB, cts.Token));
|
||||
Assert.True(registry.TryGet("B", out _), "adding PLC-B must create its tag-value capture");
|
||||
_supervisors.Add(supervisors["B"]);
|
||||
|
||||
// Reload that removes PLC-B → the registry must drop B's capture.
|
||||
Assert.True(await reconciler.ApplyAsync(initial, cts.Token));
|
||||
Assert.False(registry.TryGet("B", out _), "removing PLC-B must drop its tag-value capture");
|
||||
}
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
|
||||
@@ -366,4 +366,35 @@ public sealed class ReloadValidatorTests
|
||||
Assert.False(valid);
|
||||
Assert.Contains(errors, e => e.Contains("AdminPushIntervalMs"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_AdminPushIntervalMs_AboveUpperBound_Fails()
|
||||
{
|
||||
// The soft upper bound (60 s) catches a seconds-as-milliseconds typo that
|
||||
// would make the "live" dashboard feed effectively non-live.
|
||||
var opts = new MbproxyOptions
|
||||
{
|
||||
Plcs = [MakePlc("PLC-A", 5020)],
|
||||
AdminPushIntervalMs = 60_001,
|
||||
};
|
||||
|
||||
bool valid = ReloadValidator.Validate(opts, out var errors);
|
||||
|
||||
Assert.False(valid);
|
||||
Assert.Contains(errors, e => e.Contains("AdminPushIntervalMs"));
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void Validate_AdminPushIntervalMs_AtUpperBound_Passes()
|
||||
{
|
||||
var opts = new MbproxyOptions
|
||||
{
|
||||
Plcs = [MakePlc("PLC-A", 5020)],
|
||||
AdminPushIntervalMs = 60_000,
|
||||
};
|
||||
|
||||
bool valid = ReloadValidator.Validate(opts, out var errors);
|
||||
|
||||
Assert.True(valid, string.Join("; ", errors));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -23,6 +23,9 @@
|
||||
<PackageReference Include="Shouldly" Version="4.3.0" />
|
||||
<!-- NModbus: Modbus TCP client for simulator smoke tests and e2e tests. -->
|
||||
<PackageReference Include="NModbus" Version="3.0.83" />
|
||||
<!-- SignalR .NET client — drives the /hub/status end-to-end test (a real
|
||||
HubConnection against the live Kestrel admin host). -->
|
||||
<PackageReference Include="Microsoft.AspNetCore.SignalR.Client" Version="10.0.0" />
|
||||
</ItemGroup>
|
||||
|
||||
<ItemGroup>
|
||||
|
||||
Reference in New Issue
Block a user