Merge fix/678-small: retention cancel/fallback tests, #672 tests, FOCAS race, TTL log wording, debug overlay, role-overlay suffix fix (#678)

2026-10-11 10:55:19 -04:00
parent 045a21da14 f292cf3beb
commit 1375f9349e
13 changed files with 279 additions and 20 deletions
@@ -71,7 +71,7 @@ public sealed class WireBackendTests
            await driver.InitializeAsync("{}", ct);

            await WaitFor(() =>
-                driver.GetDeviceState(DeviceHost) is { FixedTreeCache: not null }, TimeSpan.FromSeconds(5));
+                driver.GetDeviceState(DeviceHost) is { FixedTreeCache: not null }, WaitTimeout);

            var state = driver.GetDeviceState(DeviceHost);
            state.ShouldNotBeNull();
@@ -82,7 +82,7 @@ public sealed class WireBackendTests

            await WaitFor(() =>
                state.LastFixedSnapshots.ContainsKey($"{DeviceHost}/Axes/X/AbsolutePosition"),
-                TimeSpan.FromSeconds(3));
+                WaitTimeout);
            state.LastFixedSnapshots[$"{DeviceHost}/Axes/X/AbsolutePosition"].ShouldBe(123456);
        }
    }
@@ -136,7 +136,7 @@ public sealed class WireBackendTests

            await WaitFor(() =>
                driver.GetDeviceState(DeviceHost) is { LastProgramInfo: not null },
-                TimeSpan.FromSeconds(5));
+                WaitTimeout);

            var snapshots = await driver.ReadAsync(
                [$"{DeviceHost}/Program/Name",
@@ -194,7 +194,7 @@ public sealed class WireBackendTests
            {
                var state = driver.GetDeviceState(DeviceHost);
                return state is not null && state.LastTimers.Count == 4;
-            }, TimeSpan.FromSeconds(5));
+            }, WaitTimeout);

            var snapshots = await driver.ReadAsync(
                [$"{DeviceHost}/Timers/PowerOnSeconds",
@@ -265,8 +265,11 @@ public sealed class WireBackendTests
            {
                var state = driver.GetDeviceState(DeviceHost);
                return state?.FixedTreeCache is { Capabilities.SpindleLoad: true, Capabilities.SpindleMotorSpeed: true }
-                    && state.LastSpindleLoads.Count >= 2;
-            }, TimeSpan.FromSeconds(5));
+                    && state.LastSpindleLoads.Count >= 2
+                    // Motor speeds are cached after a SECOND wire round-trip (FocasDriver spindle poll), so the loads
+                    // arriving first does not mean the speeds have — waiting on loads alone raced under load (#678.3).
+                    && state.LastSpindleMotorSpeeds.Count >= 2;
+            }, WaitTimeout);

            var snapshots = await driver.ReadAsync(
                [$"{DeviceHost}/Spindle/S1/Load",
@@ -282,6 +285,10 @@ public sealed class WireBackendTests
        }
    }

+    /// <summary>Generous: the sim fixture answers fast, but a loaded CI host (e.g. a concurrent docker build) does not.</summary>
+    private static readonly TimeSpan WaitTimeout = TimeSpan.FromSeconds(15);
+
+    /// <summary>Polls <paramref name="pred"/>; FAILS the test with a clear message on timeout rather than returning silently.</summary>
    private static async Task WaitFor(Func<bool> pred, TimeSpan timeout)
    {
        var deadline = DateTime.UtcNow + timeout;
@@ -290,5 +297,7 @@ public sealed class WireBackendTests
            if (pred()) return;
            await Task.Delay(50);
        }
+        if (pred()) return;
+        Assert.Fail($"Condition not met within {timeout.TotalSeconds:0.#}s waiting on the FOCAS wire backend.");
    }
 }
@@ -178,6 +178,80 @@ public sealed class RetentionTests(SqlHistorianFixture fx)
        (await LockIsFreeAsync(s)).ShouldBeTrue();
    }

+    [Fact]
+    public async Task Cancelling_mid_command_while_the_delete_is_blocked_still_releases_the_applock()
+    {
+        fx.SkipIfUnavailable();
+        var (c, b, _, s) = Make();
+        await SeedAsync(b, s);
+
+        // A second session holds an exclusive table lock, so the purger's first DELETE blocks server-side.
+        await using var holder = new SqlConnection(fx.ConnectionString);
+        await holder.OpenAsync();
+        await using var tx = (SqlTransaction)await holder.BeginTransactionAsync();
+        await using (var cmd = holder.CreateCommand())
+        {
+            cmd.Transaction = tx;
+            cmd.CommandText = $"SELECT COUNT(*) FROM [{s}].Sample WITH (TABLOCKX, HOLDLOCK)";
+            await cmd.ExecuteScalarAsync();
+        }
+
+        using var cts = new CancellationTokenSource();
+        var purger = new SqlServerHistorianRetention(c, b, TimeSpan.FromHours(1), 30, 4000, time: new FixedTime(Now));
+        var purge = purger.PurgeAsync(cts.Token);
+        await Task.Delay(500);
+        purge.IsCompleted.ShouldBeFalse("the DELETE must be blocked behind the holder's table lock");
+        cts.Cancel();
+
+        // SqlClient may surface the attention as a SqlException rather than OperationCanceledException.
+        await Should.ThrowAsync<Exception>(() => purge);
+
+        await tx.RollbackAsync();
+        (await LockIsFreeAsync(s)).ShouldBeTrue();
+        (await CountAsync($"SELECT COUNT(*) FROM [{s}].Sample")).ShouldBe(21);
+    }
+
+    [Fact]
+    public async Task A_failing_release_falls_back_to_clearing_the_pool_so_the_applock_is_freed()
+    {
+        fx.SkipIfUnavailable();
+        var (c, b, _, s) = Make();
+        await SeedAsync(b, s);
+        var purger = new SqlServerHistorianRetention(c, b, TimeSpan.FromHours(1), 30, 4000, time: new FixedTime(Now))
+        {
+            ReleaseSql = "EXEC sp_this_procedure_does_not_exist;",
+        };
+
+        await purger.PurgeAsync(CancellationToken.None);
+
+        (await LockIsFreeAsync(s)).ShouldBeTrue();
+    }
+
+    [Fact]
+    public async Task Falsification_control_a_failing_release_with_a_no_op_ClearPool_leaves_the_applock_held()
+    {
+        fx.SkipIfUnavailable();
+        var (c, b, _, s) = Make();
+        await SeedAsync(b, s);
+        var purger = new SqlServerHistorianRetention(c, b, TimeSpan.FromHours(1), 30, 4000, time: new FixedTime(Now))
+        {
+            ReleaseSql = "EXEC sp_this_procedure_does_not_exist;",
+            ClearPool = _ => { },
+        };
+
+        await purger.PurgeAsync(CancellationToken.None);
+
+        try
+        {
+            // The pooled session still owns the Session-scoped lock: proves the fallback test above is not vacuous.
+            (await LockIsFreeAsync(s)).ShouldBeFalse();
+        }
+        finally
+        {
+            SqlConnection.ClearAllPools(); // drop the leaked session so the lock does not outlive the test
+        }
+    }
+
    [Fact]
    public async Task A_throwing_purge_releases_the_applock()
    {
@@ -0,0 +1,78 @@
+using System.Text.Json;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Host.Configuration;
+
+namespace ZB.MOM.WW.OtOpcUa.Host.Tests.Configuration;
+
+/// <summary>
+///     #678.6 — the per-role appsettings overlay name is built from plain roles only. A <c>cluster-*</c> role in the
+///     name produced a file that never exists, so no overlay ever loaded on a node with a cluster role.
+/// </summary>
+public sealed class RoleOverlayTests
+{
+    [Fact]
+    public void A_driver_site_node_loads_the_driver_overlay()
+        => RoleOverlay.Suffix(["driver", "cluster-SITE-A"]).ShouldBe("driver");
+
+    [Fact]
+    public void A_central_node_loads_the_admin_driver_overlay()
+        => RoleOverlay.Suffix(["admin", "driver", "cluster-MAIN"]).ShouldBe("admin-driver");
+
+    [Fact]
+    public void Plain_roles_are_ordinal_sorted_regardless_of_input_order()
+        => RoleOverlay.Suffix(["driver", "cluster-MAIN", "admin"]).ShouldBe("admin-driver");
+
+    [Fact]
+    public void Roles_without_a_cluster_role_keep_the_existing_names()
+    {
+        RoleOverlay.Suffix(["admin"]).ShouldBe("admin");
+        RoleOverlay.Suffix(["driver"]).ShouldBe("driver");
+        RoleOverlay.Suffix(["admin", "driver"]).ShouldBe("admin-driver");
+    }
+
+    [Fact]
+    public void A_cluster_role_prefix_is_matched_case_insensitively()
+        => RoleOverlay.Suffix(["driver", "CLUSTER-SITE-A"]).ShouldBe("driver");
+
+    [Fact]
+    public void No_plain_role_means_no_overlay()
+    {
+        RoleOverlay.Suffix([]).ShouldBeNull();
+        RoleOverlay.Suffix(["cluster-SITE-A"]).ShouldBeNull();
+    }
+
+    // ---- The shipped overlays: activating them on cluster-role nodes must not change production semantics. ----
+
+    private static string HostDir()
+    {
+        for (var dir = new DirectoryInfo(AppContext.BaseDirectory); dir is not null; dir = dir.Parent)
+        {
+            var candidate = Path.Combine(dir.FullName, "src", "Server", "ZB.MOM.WW.OtOpcUa.Host");
+            if (File.Exists(Path.Combine(candidate, "appsettings.json"))) return candidate;
+        }
+
+        throw new InvalidOperationException("Host project directory not found above " + AppContext.BaseDirectory);
+    }
+
+    [Theory]
+    [InlineData("admin")]
+    [InlineData("driver")]
+    [InlineData("admin-driver")]
+    public void Shipped_overlays_carry_only_the_audited_keys(string suffix)
+    {
+        using var doc = JsonDocument.Parse(File.ReadAllText(Path.Combine(HostDir(), $"appsettings.{suffix}.json")));
+        var root = doc.RootElement;
+
+        // Only Serilog + Security:Ldap, and in Security:Ldap only the two audited keys.
+        root.EnumerateObject().Select(p => p.Name).Order().ShouldBe(["Security", "Serilog"]);
+        var ldap = root.GetProperty("Security").GetProperty("Ldap");
+        ldap.EnumerateObject().Select(p => p.Name).Order().ShouldBe(["DevStubMode", "Transport"]);
+        ldap.GetProperty("DevStubMode").GetBoolean().ShouldBeFalse();
+
+        // No Debug-level (or lower) category override: activating an overlay must not raise log volume on a node.
+        foreach (var o in root.GetProperty("Serilog").GetProperty("MinimumLevel").GetProperty("Override").EnumerateObject())
+            o.Value.GetString().ShouldNotBeOneOf("Debug", "Verbose");
+        root.GetProperty("Serilog").GetProperty("MinimumLevel").GetProperty("Default").GetString().ShouldBe("Information");
+    }
+}
@@ -223,10 +223,30 @@ public sealed class DriverHostActorHistorizationRoleViewTests : RuntimeActorTest
        host.Tell(new ClusterEvent.UnreachableMember(MemberAt(Peer672)));
        AwaitAssert(() => hist.Published.ShouldBe([false, true]), duration: Timeout);

-        // And self "returning" does not close what the peer's loss opened.
+        // Self "returning" is ignored too, and it does not close what the peer's loss opened. Events are
+        // processed in order, so the peer's return (which MUST close the gate) is the only thing that may
+        // appear next: had the self event published anything it would sit between `true` and `false`.
        host.Tell(new ClusterEvent.ReachableMember(MemberAt(Self672)));
+        host.Tell(new ClusterEvent.ReachableMember(MemberAt(Peer672)));
+        AwaitAssert(() => hist.Published.ShouldBe([false, true, false]), duration: Timeout);
+    }
+
+    /// <summary>
+    ///     A host that starts while its peer is ALREADY unreachable (the <c>InitialStateAsEvents</c> replay
+    ///     delivers the <c>UnreachableMember</c> before the first redundancy snapshot) must still treat that
+    ///     peer as unreachable when the snapshot naming it Primary finally arrives.
+    /// </summary>
+    [Fact]
+    public void A_peer_unreachable_before_the_first_snapshot_still_opens_capture()
+    {
+        var hist = new RecordingHistorizationView();
+        IActorRef host = SpawnHost672(hist, others: null, captureWhilePrimaryUnreachable: true);
+
+        host.Tell(new ClusterEvent.UnreachableMember(MemberAt(Peer672)));
        host.Tell(Snapshot((Self672, RedundancyRole.Secondary), (Peer672, RedundancyRole.Primary)));
-        AwaitAssert(() => hist.Published.ShouldBe([false, true, true]), duration: Timeout);
+
+        // Exactly one publication, and it is `true`: no spurious `false` from treating the peer as reachable.
+        AwaitAssert(() => hist.Published.ShouldBe([true]), duration: Timeout);
    }

    /// <summary><c>CaptureWhilePrimaryUnreachable = false</c> restores the pre-#672 behaviour.</summary>