diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs index 391bd4e3..d0b6aa7c 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/OpcUa/OpcUaPublishActor.cs @@ -157,6 +157,15 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers /// protecting. Distinguishes "the artifact went missing under a running server" (issue #485 — warn, and /// hold what is materialised) from "nothing has been deployed here yet" (a quiet no-op at boot). private bool _hasAppliedComposition; + /// + /// Gitea #601 review — was advanced by a DEGRADED rebuild (a rebuild advances + /// even with swallowed failures, see ). A re-deploy of the same revision + /// then diffs to an EMPTY plan; without this flag that empty plan reported clean, so the "re-deploy + /// re-applies" repair path sealed green over an address space still known to be partial. While set, an + /// empty plan re-materialises the whole composition (idempotent ensures) and reports clean only when + /// that pass is itself clean. + /// + private bool _lastAppliedDegraded; private DbHealthProbeActor.DbHealthStatus? _lastDbHealth; private RedundancyStateChanged? _lastSnapshot; private (bool Ok, DateTime At)? _probeAboutMe; @@ -565,6 +574,28 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers if (plan.IsEmpty) { + if (_lastAppliedDegraded) + { + // #601 review — the composition is unchanged but the last apply of it was a degraded + // rebuild: re-ensure every node (each Ensure* no-ops on a node that stands) so the + // re-deploy actually repairs what that rebuild could not place. + var repairFailed = _applier.MaterialiseHierarchy(composition) + + MaterialiseNodes(plan, composition, fullComposition: true); + if (repairFailed > 0) + { + OtOpcUaTelemetry.OpcUaApplyFailed.Add(1, new KeyValuePair("kind", "nodes")); + _log.Error( + "OpcUaPublish: re-apply of a degraded composition is STILL degraded (correlation={Correlation}, failedNodes={FailedNodes})", + msg.Correlation, repairFailed); + return (true, $"re-apply of a previously degraded address space is still degraded (failedNodes={repairFailed})"); + } + + _lastAppliedDegraded = false; + _log.Info("OpcUaPublish: re-apply repaired the previously degraded address space (correlation={Correlation})", + msg.Correlation); + return (false, null); + } + _log.Debug("OpcUaPublish: rebuild requested but plan is empty (correlation={Correlation})", msg.Correlation); return (false, null); @@ -610,6 +641,9 @@ public sealed class OpcUaPublishActor : ReceiveActor, IWithTimers { _lastApplied = composition; _hasAppliedComposition = true; + // #601 review — advanced despite failures only on a rebuild; remember it so an identical + // re-deploy repairs instead of short-circuiting clean on the empty plan. + _lastAppliedDegraded = outcome.RebuildFailed || failedNodes > 0; } else { diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyOutcomeTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyOutcomeTests.cs index 37d0773d..f3e0602e 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyOutcomeTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorApplyOutcomeTests.cs @@ -112,7 +112,49 @@ public sealed class OpcUaPublishActorApplyOutcomeTests : RuntimeActorTestBase ExpectNoMsg(TimeSpan.FromMilliseconds(200)); } - private static byte[] BuildEquipmentArtifact(string equipmentId) => + [Fact] + public void Redeploying_the_same_composition_after_a_degraded_REBUILD_repairs_it_instead_of_reporting_clean() + { + // A degraded REBUILD advances _lastApplied (deliberately — see HandleRebuildCore), so a re-deploy of + // the same revision diffs to an EMPTY plan. Reporting that empty plan as clean would let the + // "re-deploy re-applies" repair path (#601) seal green over an address space still known partial. + var sink = new FaultingSink(throwOnEnsureFolder: false); + var applier = new AddressSpaceApplier(sink, NullLogger.Instance); + var actor = Sys.ActorOf(OpcUaPublishActor.PropsForTests(sink: sink, applier: applier)); + + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace( + CorrelationId.NewId(), DeploymentId.NewId(), BuildEquipmentArtifact("eq-1", "First"), ReportOutcome: true)); + ExpectMsg(PresenceBudget).Degraded.ShouldBeFalse(); + + // A changed equipment row routes to a full rebuild; its re-materialise faults. + sink.ThrowOnEnsureFolder = true; + var changed = BuildEquipmentArtifact("eq-1", "Second"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), DeploymentId.NewId(), changed, ReportOutcome: true)); + var degraded = ExpectMsg(PresenceBudget); + degraded.Degraded.ShouldBeTrue(); + sink.RebuildCount.ShouldBe(1); + + // Same revision again, sink still faulting: must NOT claim clean. + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), DeploymentId.NewId(), changed, ReportOutcome: true)); + ExpectMsg(PresenceBudget).Degraded.ShouldBeTrue(); + + // Fault gone: the same revision re-materialises the whole composition and only THEN reports clean. + sink.ThrowOnEnsureFolder = false; + var foldersBefore = sink.EnsureFolderCount; + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), DeploymentId.NewId(), changed, ReportOutcome: true)); + ExpectMsg(PresenceBudget).Degraded.ShouldBeFalse(); + sink.EnsureFolderCount.ShouldBeGreaterThan(foldersBefore); + + // And now it is genuinely clean, so a further identical deploy is the ordinary empty no-op. + var foldersAfterRepair = sink.EnsureFolderCount; + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), DeploymentId.NewId(), changed, ReportOutcome: true)); + ExpectMsg(PresenceBudget).Degraded.ShouldBeFalse(); + sink.EnsureFolderCount.ShouldBe(foldersAfterRepair); + } + + private static byte[] BuildEquipmentArtifact(string equipmentId) => BuildEquipmentArtifact(equipmentId, equipmentId); + + private static byte[] BuildEquipmentArtifact(string equipmentId, string name) => JsonSerializer.SerializeToUtf8Bytes(new { UnsAreas = new[] { new { UnsAreaId = "area-1", ClusterId = "c1", Name = "Area" } }, @@ -124,7 +166,7 @@ public sealed class OpcUaPublishActorApplyOutcomeTests : RuntimeActorTestBase EquipmentId = equipmentId, DriverInstanceId = (string?)null, UnsLineId = "line-1", - Name = equipmentId, + Name = name, MachineCode = equipmentId.ToUpperInvariant(), }, }, @@ -137,16 +179,20 @@ public sealed class OpcUaPublishActorApplyOutcomeTests : RuntimeActorTestBase /// A sink that optionally throws from EnsureFolder (a swallowed per-node fault). private sealed class FaultingSink(bool throwOnEnsureFolder) : IOpcUaAddressSpaceSink { + public bool ThrowOnEnsureFolder { get; set; } = throwOnEnsureFolder; + public int EnsureFolderCount; + public int RebuildCount; public void WriteValue(string nodeId, object? value, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } public void WriteAlarmQuality(string alarmNodeId, uint statusCode, DateTime sourceTimestampUtc, AddressSpaceRealm realm) { } public void WriteAlarmCondition(string alarmNodeId, AlarmConditionSnapshot state, DateTime occurredUtc, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } public void MaterialiseAlarmCondition(string alarmNodeId, string equipmentNodeId, string displayName, string alarmType, int severity, AddressSpaceRealm realm, bool isNative = false) { } public void EnsureFolder(string folderNodeId, string? parentNodeId, string displayName, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { - if (throwOnEnsureFolder) throw new InvalidOperationException("simulated sink fault"); + EnsureFolderCount++; + if (ThrowOnEnsureFolder) throw new InvalidOperationException("simulated sink fault"); } public void EnsureVariable(string variableNodeId, string? parentFolderNodeId, string displayName, string dataType, bool writable, AddressSpaceRealm realm, string? historianTagname = null, bool isArray = false, uint? arrayLength = null, bool refuseArrayOnScalar = false) { } - public void RebuildAddressSpace() { } + public void RebuildAddressSpace() => RebuildCount++; public void RaiseNodesAddedModelChange(string affectedNodeId, AddressSpaceRealm realm = AddressSpaceRealm.Uns) { } public void WireAlarmNotifiers(string alarmNodeId, AddressSpaceRealm alarmRealm, IReadOnlyList notifierFolderNodeIds, AddressSpaceRealm notifierFolderRealm) { } public void AddReference(string sourceNodeId, AddressSpaceRealm sourceRealm, string targetNodeId, AddressSpaceRealm targetRealm, string referenceType = "Organizes") { } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs index 94453d9a..0d19d36a 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Runtime.Tests/OpcUa/OpcUaPublishActorRebuildTests.cs @@ -845,7 +845,10 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase /// _lastApplied. A rebuild already re-materialised the whole composition, and the applier's /// one-way rebuild ratchet is the resync mechanism for anything it could not place — holding the /// composition back here would make every subsequent deploy re-diff the entire address space as added. - /// Proven by the third apply of the same composition being an empty-plan no-op. + /// Proven by the third apply of the same composition NOT rebuilding again (it diffs to an empty plan). + /// Gitea #601 review: that empty plan is no longer a silent no-op, though — the previous apply was a + /// degraded rebuild, so the identical re-deploy re-ensures the whole composition (idempotently, no + /// rebuild) and thereby repairs the node the rebuild could not place. /// [Fact] public void Rebuild_with_failures_still_advances_last_applied() @@ -869,13 +872,24 @@ public sealed class OpcUaPublishActorRebuildTests : RuntimeActorTestBase var afterSecond = sink.Calls.Count; // Re-deploy the SAME composition. _lastApplied advanced despite the failure, so this diffs to an - // empty plan and adds no sink calls at all. + // empty plan — no second rebuild (an un-advanced composition would re-diff ChangedEquipment and + // rebuild again) — but the degraded rebuild is remembered, so the failed variable is re-ensured. sink.FailEnsureVariableFor = null; var dep3 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a"); actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep3))); - Thread.Sleep(300); + AwaitAssert( + () => sink.Calls.Skip(afterSecond).ShouldContain("EV:Plant/Modbus/dev1/b"), + duration: PresenceBudget); + sink.RebuildCalls.ShouldBe(1); - sink.Calls.Count.ShouldBe(afterSecond); + // Repaired ⇒ genuinely clean now, so a FOURTH identical deploy is the ordinary empty-plan no-op. + Thread.Sleep(200); + var afterRepair = sink.Calls.Count; + var dep4 = SeedUnsRawDeployment(db, equipmentName: "station1-renamed", referencedTag: "a"); + actor.Tell(new OpcUaPublishActor.RebuildAddressSpace(CorrelationId.NewId(), new DeploymentId(dep4))); + Thread.Sleep(300); + sink.Calls.Count.ShouldBe(afterRepair); + sink.RebuildCalls.ShouldBe(1); } /// Seal a deployment whose raw tree is Plant/Modbus/dev1 carrying one tag per name in