fix(mesh-phase4): OpcUaPublish diff-applies from in-hand bytes with no ConfigDb (no wipe)
A driver-only node has a null dbFactory but a wired applier; the old combined guard routed it to the raw-sink fallback that wipes the address space and never re-materialises. Split the guard so in-hand artifact bytes drive the real diff-and-apply; a missing artifact abandons the rebuild (keep last-known-good, #485) instead of wiping. Claude-Session: https://claude.ai/code/session_01GASWkNEi68FSCtvr6rLoEW
This commit is contained in:
Joseph Doherty
@@ -359,10 +359,15 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
using var span = OtOpcUaTelemetry.StartAddressSpaceRebuildSpan();
|
||||
span?.SetTag("otopcua.correlation_id", msg.Correlation.ToString());
|
||||
|
||||
// Two modes: when dbFactory + applier are wired, do a real diff-and-apply pass against
|
||||
// the latest deployment artifact. Without them, fall back to a raw sink rebuild — the
|
||||
// F10b/dev path before the integration completes.
|
||||
if (_dbFactory is null || _applier is null)
|
||||
// Two modes. With an applier wired, do a REAL diff-and-apply pass — driven either by the
|
||||
// artifact bytes already in hand (a per-cluster-mesh driver-only / FetchAndCache node, whose
|
||||
// ConfigDb is admin-only now so _dbFactory is null, hands its fetched/cached bytes in
|
||||
// msg.Artifact) or, when no bytes are carried, by loading the artifact from the ConfigDb (the
|
||||
// Direct path, which requires _dbFactory). Only a node with NO applier at all (the F10b/dev
|
||||
// seam) falls back to the raw-sink rebuild — and that fallback WIPES the served address space
|
||||
// without re-materialising, so it must NEVER be reached by a real driver node that has an
|
||||
// applier but simply has no ConfigDb (that node diff-and-applies from its in-hand bytes below).
|
||||
if (_applier is null)
|
||||
{
|
||||
try
|
||||
{
|
||||
@@ -379,15 +384,20 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers
|
||||
|
||||
try
|
||||
{
|
||||
// An in-hand artifact (boot-from-cache) wins: the host already has the cached bytes, and
|
||||
// the ConfigDb read the DeploymentId path would do is exactly what is unreachable during
|
||||
// the outage this exists to survive. Otherwise prefer the artifact of the deployment the
|
||||
// host just applied — at apply time it is not yet Sealed, so LoadLatestArtifact would
|
||||
// return the PREVIOUS revision and materialise a stale composition (variables that don't
|
||||
// match the SubscribeBulk refs). Fall back to latest-sealed only for legacy callers that
|
||||
// carry neither.
|
||||
// An in-hand artifact (boot-from-cache / FetchAndCache) wins: the host already has the
|
||||
// cached/fetched bytes, and the ConfigDb read the DeploymentId path would do is exactly what
|
||||
// is unreachable during the outage this exists to survive — and is simply absent on a
|
||||
// driver-only node with no ConfigDb. Otherwise, when a ConfigDb is present (Direct mode),
|
||||
// prefer the artifact of the deployment the host just applied — at apply time it is not yet
|
||||
// Sealed, so LoadLatestArtifact would return the PREVIOUS revision and materialise a stale
|
||||
// composition (variables that don't match the SubscribeBulk refs); fall back to latest-sealed
|
||||
// only for legacy callers that carry neither. With NO bytes AND no ConfigDb there is nothing
|
||||
// to load, so yield Array.Empty<byte>() — the #485 guard below reads that as "no answer" and
|
||||
// abandons the rebuild, holding last-known-good rather than wiping the address space.
|
||||
var artifact = msg.Artifact
|
||||
?? (msg.DeploymentId is { } depId ? LoadArtifact(depId) : LoadLatestArtifact());
|
||||
?? (_dbFactory is not null
|
||||
? (msg.DeploymentId is { } depId ? LoadArtifact(depId) : LoadLatestArtifact())
|
||||
: Array.Empty<byte>());
|
||||
|
||||
// Issue #485 — an artifact we could not obtain is NOT an empty configuration. Parsing zero
|
||||
// bytes yields an empty composition, which the planner diffs against the live one as a
|
||||
|
||||
+106
-8
@@ -110,6 +110,97 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// mesh-phase4 — the wipe-prevention test. A driver-only node has a NULL dbFactory (ConfigDb is
|
||||
/// admin-only now) but a WIRED applier. A RebuildAddressSpace carrying the artifact bytes in hand
|
||||
/// (from the fetch/cache) must drive the REAL diff-and-apply (the equipment folder is materialised)
|
||||
/// and must NOT take the raw-sink <c>RebuildAddressSpace()</c> fallback, which wipes the address
|
||||
/// space without re-materialising. Before the guard split, a null dbFactory routed straight to that
|
||||
/// fallback and the applier never ran — a FetchAndCache node deployed green but served EMPTY.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_with_null_factory_but_wired_applier_diff_applies_from_in_hand_bytes()
|
||||
{
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var artifact = BuildNamedEquipmentArtifact(("eq-1", "Pump-1"));
|
||||
|
||||
// Driver-only node: null dbFactory, wired applier.
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(
|
||||
sink: sink, dbFactory: null, applier: applier));
|
||||
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), Artifact: artifact));
|
||||
|
||||
// The in-hand bytes drove the real diff-and-apply — the equipment folder was materialised…
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2));
|
||||
// …and the address-space-wiping raw-sink fallback was NOT taken.
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// mesh-phase4 / #485 negative — a driver-only node (null dbFactory, wired applier) that gets a
|
||||
/// RebuildAddressSpace with NO artifact bytes has no way to obtain a config (there is no ConfigDb to
|
||||
/// read). The artifact source yields <c>Array.Empty<byte>()</c>, which the #485 zero-length
|
||||
/// guard treats as "no answer": the rebuild is abandoned WITHOUT touching the served address space —
|
||||
/// the applier never runs and the raw-sink wipe fallback is never taken. Last-known-good stands.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_with_null_factory_and_no_artifact_abandons_without_wiping()
|
||||
{
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(
|
||||
sink: sink, dbFactory: null, applier: applier));
|
||||
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId()));
|
||||
Thread.Sleep(200);
|
||||
|
||||
sink.RebuildCalls.ShouldBe(0); // the raw-sink wipe fallback was NOT taken
|
||||
sink.Calls.ShouldBeEmpty(); // the applier's diff-and-apply never ran
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// mesh-phase4 regression (dev seam) — a node with NO applier at all still falls back to the raw-sink
|
||||
/// rebuild, and now the fallback is keyed on the applier ALONE: even WITH a dbFactory wired, a null
|
||||
/// applier takes the F10b/dev fallback. Proves the guard split did not change the no-applier seam.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_with_null_applier_falls_back_to_raw_sink_even_with_dbFactory()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var sink = new RecordingSink();
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(
|
||||
sink: sink, dbFactory: db, applier: null));
|
||||
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId()));
|
||||
|
||||
AwaitAssert(() => sink.RebuildCalls.ShouldBe(1), duration: TimeSpan.FromMilliseconds(500));
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// mesh-phase4 regression (Direct mode) — a DB-backed node (dbFactory wired, applier wired) with no
|
||||
/// in-hand bytes but a DeploymentId still loads the artifact from the ConfigDb via <c>LoadArtifact</c>
|
||||
/// and drives the applier. The guard split must leave the Direct path byte-identical.
|
||||
/// </summary>
|
||||
[Fact]
|
||||
public void Rebuild_direct_mode_with_dbFactory_and_deploymentId_still_loads_and_applies()
|
||||
{
|
||||
var db = NewInMemoryDbFactory();
|
||||
var sink = new RecordingSink();
|
||||
var applier = new AddressSpaceApplier(sink, NullLogger<AddressSpaceApplier>.Instance);
|
||||
var dep = SeedNamedEquipmentDeployment(db, ("eq-1", "Pump-1"));
|
||||
|
||||
var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(
|
||||
sink: sink, dbFactory: db, applier: applier));
|
||||
|
||||
actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep)));
|
||||
|
||||
AwaitAssert(() => sink.Calls.ShouldContain("EF:eq-1"), duration: TimeSpan.FromSeconds(2));
|
||||
sink.RebuildCalls.ShouldBe(0);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Wiring proof for per-ClusterId scoping (Task 4): a multi-cluster artifact must
|
||||
/// materialise ONLY the local node's cluster slice. Mirrors the multi-cluster artifact
|
||||
@@ -358,14 +449,12 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
: inner.CreateDbContext();
|
||||
}
|
||||
|
||||
/// <summary>Seal a deployment carrying the given (equipmentId, name) rows under a shared line, returning
|
||||
/// the new DeploymentId so a test can target THAT artifact. Renaming an equipment across two such
|
||||
/// deployments produces a ChangedEquipment delta (Rebuild), while adding a row is a PureAdd.</summary>
|
||||
private static Guid SeedNamedEquipmentDeployment(
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory,
|
||||
params (string Id, string Name)[] equipment)
|
||||
{
|
||||
var artifact = JsonSerializer.SerializeToUtf8Bytes(new
|
||||
/// <summary>Build the artifact bytes for the given (equipmentId, name) rows under a shared line —
|
||||
/// WITHOUT sealing them into a deployment. This is what a driver-only / FetchAndCache node carries
|
||||
/// in <c>RebuildAddressSpace.Artifact</c> (no ConfigDb read), so tests can drive the in-hand-bytes
|
||||
/// path directly.</summary>
|
||||
private static byte[] BuildNamedEquipmentArtifact(params (string Id, string Name)[] equipment) =>
|
||||
JsonSerializer.SerializeToUtf8Bytes(new
|
||||
{
|
||||
UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } },
|
||||
UnsLines = new[] { new { UnsLineId = "line-1", UnsAreaId = "area-1", Name = "Line" } },
|
||||
@@ -383,6 +472,15 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase
|
||||
ScriptedAlarms = Array.Empty<object>(),
|
||||
});
|
||||
|
||||
/// <summary>Seal a deployment carrying the given (equipmentId, name) rows under a shared line, returning
|
||||
/// the new DeploymentId so a test can target THAT artifact. Renaming an equipment across two such
|
||||
/// deployments produces a ChangedEquipment delta (Rebuild), while adding a row is a PureAdd.</summary>
|
||||
private static Guid SeedNamedEquipmentDeployment(
|
||||
IDbContextFactory<OtOpcUaConfigDbContext> dbFactory,
|
||||
params (string Id, string Name)[] equipment)
|
||||
{
|
||||
var artifact = BuildNamedEquipmentArtifact(equipment);
|
||||
|
||||
var id = Guid.NewGuid();
|
||||
using var ctx = dbFactory.CreateDbContext();
|
||||
ctx.Deployments.Add(new Deployment
|
||||
|
||||
Reference in New Issue
Block a user