diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs index b9786bfd..37f28d86 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Host/OpcUa/OtOpcUaServerHostedService.cs @@ -130,35 +130,23 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl } }); - // Wire the reverse-path inbound operator-write router: a client write to a writable equipment-tag + // Wire the reverse-path inbound operator-write gateway: a client write to a writable equipment-tag // node that passes the node manager's WriteOperate gate routes the write to the owning driver child - // (RouteNodeWrite → NodeWriteResult) via the local DriverHostActor. This dispatch is FIRE-AND-FORGET - // (just like the alarm router): the SDK's CustomNodeManager2.Write holds the node-manager Lock while - // invoking OnWriteValue, so a blocking Ask here would freeze ALL address-space operations (reads, - // subscription notifications, the publish path) for up to the Ask timeout. We kick off the Ask and - // log failures from a continuation; the write reaches the device asynchronously and the value - // self-corrects on the next driver poll (standard OPC UA optimistic-write semantics). The - // DriverHostActor ref is resolved LAZILY per write — this hosted service's StartAsync runs before - // the Akka DriverHostActor registers, so a one-shot resolve here would always miss and leave every - // write unavailable. By write time (long after startup) the registry has it; a node that genuinely - // has no driver-host (admin-only, no writable driver nodes materialised) logs + drops the write. - _server.SetNodeWriteRouter((nodeId, value) => - { - if (!_actorRegistry.TryGet(out var driverHost)) - { - _logger.LogWarning("Inbound write to {NodeId} dropped: no DriverHostActor registered", nodeId); - return; - } - driverHost.Ask( - new DriverHostActor.RouteNodeWrite(nodeId, value), TimeSpan.FromSeconds(10)) - .ContinueWith(t => - { - if (!t.IsCompletedSuccessfully) - _logger.LogWarning("Operator write to {NodeId} failed or timed out", nodeId); - else if (!t.Result.Success) - _logger.LogWarning("Operator write to {NodeId} rejected: {Reason}", nodeId, t.Result.Reason); - }, TaskScheduler.Default); - }); + // (RouteNodeWrite → NodeWriteResult) via the local DriverHostActor. The node manager calls the + // gateway's WriteAsync FIRE-AND-FORGET: the SDK's CustomNodeManager2.Write holds the node-manager + // Lock while invoking OnWriteValue, so a blocking Ask here would freeze ALL address-space operations + // (reads, subscription notifications, the publish path) for up to the Ask timeout. The gateway kicks + // off the Ask and resolves a NodeWriteOutcome; the node manager applies the client value optimistically + // and self-corrects (reverts to the pre-write value) when the device write comes back FAILED — but only + // while the node still holds the optimistic value, so a fresh driver poll is not clobbered. The + // DriverHostActor ref is resolved LAZILY per write (inside the gateway) — this hosted service's + // StartAsync runs before the Akka DriverHostActor registers, so a one-shot resolve here would always + // miss and leave every write unavailable. By write time (long after startup) the registry has it; a + // node that genuinely has no driver-host (admin-only, no writable driver nodes materialised) logs + + // resolves the write to "writes unavailable". + _server.SetNodeWriteGateway(new ActorNodeWriteGateway( + resolveDriverHost: () => _actorRegistry.TryGet(out var driverHost) ? driverHost : null, + logger: _loggerFactory.CreateLogger())); // ServiceLevel publisher needs IServerInternal — only available after Start. if (_server.CurrentInstance is { } serverInternal) @@ -181,8 +169,8 @@ public sealed class OtOpcUaServerHostedService : IHostedService, IAsyncDisposabl // half-disposed NodeManager. _deferredSink.SetSink(null); _deferredServiceLevel.SetInner(null); - // Drop the inbound-write router too so a late client write doesn't Ask a stopping DriverHostActor. - _server?.SetNodeWriteRouter(null); + // Restore the Null write gateway so a late client write doesn't Ask a stopping DriverHostActor. + _server?.SetNodeWriteGateway(null); return Task.CompletedTask; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index cfcebafe..08ebd83d 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -70,37 +70,40 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// public Action? AlarmCommandRouter { get; set; } - private volatile Action? _nodeWriteRouter; + private volatile IOpcUaNodeWriteGateway _nodeWriteGateway = NullOpcUaNodeWriteGateway.Instance; /// - /// Reverse-path sink for inbound OPC UA operator writes to a writable equipment-tag variable node. + /// Reverse-path gateway for inbound OPC UA operator writes to a writable equipment-tag variable node. /// When a client writes such a node, the node's /// handler (, attached by when the /// variable is writable) first gates on the caller's - /// role and, when allowed, invokes this delegate with the node's string id + the written value to - /// route the write to the backing driver. + /// role and, when allowed, calls with the node's + /// string id + the written value to route the write to the backing driver. /// - /// This is the write-side twin of — a plain - /// (no Akka / IActorRef / DI handle) so this assembly stays - /// Akka-free. The handler delegates run under the node-manager Lock (the OPC UA SDK's - /// CustomNodeManager2.Write holds Lock while invoking OnWriteValue), so this - /// dispatch MUST be non-blocking and fire-and-forget — exactly like the alarm router. The host - /// sets it at boot to a lambda that kicks off an unawaited bounded Ask of the local - /// DriverHostActor (RouteNodeWrite → NodeWriteResult) and logs failures from - /// a continuation; the write reaches the device asynchronously and the value self-corrects on the - /// next driver poll (standard OPC UA optimistic-write semantics). Null (the default) makes every - /// write resolve to a "writes unavailable" failure. + /// This is the write-side twin of ; the gateway abstraction keeps + /// this assembly Akka-free (the host wires an ActorNodeWriteGateway that Asks the local + /// DriverHostActor). The handler delegates run under the node-manager Lock (the OPC + /// UA SDK's CustomNodeManager2.Write holds Lock while invoking OnWriteValue), + /// so the dispatch is FIRE-AND-FORGET — the handler kicks off WriteAsync and returns + /// Good immediately so the SDK applies the client value optimistically; it MUST NOT block + /// on the device round-trip. When the asynchronous comes back + /// FAILED, an off-Lock continuation self-corrects: it re-takes Lock and reverts the node to + /// its real pre-write value — but only while the node still holds the optimistic value, so a fresh + /// driver poll that has already moved the node on is not clobbered (see + /// / ). /// /// - /// Backed by a volatile field (auto-properties can't be volatile) to make the + /// Set by the host at StartAsync; the default + /// (assigning null restores it) makes every write resolve to a "writes unavailable" + /// failure. Backed by a volatile field (auto-properties can't be volatile) to make the /// startup-write / SDK-thread-read explicit: the host assigns it once at boot on the start thread /// and the SDK reads it on Write request threads. /// /// - public Action? NodeWriteRouter + public IOpcUaNodeWriteGateway NodeWriteGateway { - get => _nodeWriteRouter; - set => _nodeWriteRouter = value; + get => _nodeWriteGateway; + set => _nodeWriteGateway = value ?? NullOpcUaNodeWriteGateway.Instance; } /// Look up a materialised Part 9 alarm-condition node by its alarm node id (the @@ -589,16 +592,27 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 /// (Task 11). The OPC UA SDK invokes it when a client writes the /// node's Value. It resolves the calling principal off the SDK the /// SAME way does, gates on the - /// role (fails closed: a missing identity or - /// missing role is denied), and on pass dispatches the value through . + /// role + the gateway being wired + /// (fails closed: a missing identity / missing role ⇒ BadUserAccessDenied; no gateway ⇒ + /// BadNotWritable) via the pure , and on pass dispatches + /// the value through . /// /// The dispatch is FIRE-AND-FORGET: the SDK's CustomNodeManager2.Write holds the node /// manager Lock while invoking this handler, so a blocking driver round-trip here would /// freeze every address-space operation (reads, subscription notifications, the publish path) for - /// the duration. The router only kicks off the asynchronous route. Returning - /// lets the SDK apply the written value optimistically; the next - /// driver poll republishes the confirmed register value over the optimistic one via the normal - /// path. + /// the duration. The gateway only kicks off the asynchronous route. Returning + /// lets the SDK apply the written value optimistically. + /// + /// + /// Write-outcome self-correction. Before returning Good (which makes the SDK overwrite the + /// node with ) we capture both the optimistic value AND the node's REAL + /// prior value/status — at handler entry the node still holds the prior value. An off-Lock + /// continuation on the then reverts the node to that prior + /// value/status on a FAILED outcome, but ONLY while the node still holds the optimistic value, so a + /// fresh driver poll that already republished the confirmed register value is not clobbered + /// ( / ). On success the + /// optimistic value stands and the next poll re-confirms it via the normal + /// path. /// /// private ServiceResult OnEquipmentTagWrite( @@ -606,36 +620,54 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 ref object value, ref StatusCode statusCode, ref DateTime timestamp) { var identity = (context as ISessionOperationContext)?.UserIdentity as RoleCarryingUserIdentity; - // Capture the value into a local so the route thunk (a lambda) can close over it — a ref parameter - // can't be captured by a lambda. The handler does not mutate the ref value: returning Good lets the - // SDK apply the client's value as-is. - var writtenValue = value; + var gateway = _nodeWriteGateway; + var gate = EvaluateEquipmentWriteGate(identity, gateway is not NullOpcUaNodeWriteGateway); + if (gate is not null) return gate; + + // Capture the optimistic value + the REAL prior value/status BEFORE the SDK applies the write + // (at handler entry the node still holds the prior value; returning Good makes the SDK apply `value`). + var optimisticValue = value; var nodeKey = node.NodeId.Identifier?.ToString() ?? string.Empty; - var router = _nodeWriteRouter; - Action? route = router is { } r ? () => r(nodeKey, writtenValue) : null; - return EvaluateEquipmentWrite(identity, route); + object? priorValue = null; + StatusCode priorStatus = StatusCodes.Good; + if (node is BaseDataVariableState variable) + { + priorValue = variable.Value; + priorStatus = variable.StatusCode; + } + + // Fire-and-forget — MUST NOT block under Lock. On a FAILED outcome, compare-and-revert (off-Lock + // continuation). A faulted/cancelled WriteAsync is treated as a failure so the optimistic value never + // sticks when the route never resolved a real outcome. + _ = gateway.WriteAsync(nodeKey, optimisticValue, CancellationToken.None) + .ContinueWith( + t => + { + var outcome = t.IsCompletedSuccessfully ? t.Result : new NodeWriteOutcome(false, "write dispatch faulted"); + RevertOptimisticWriteIfNeeded(nodeKey, outcome, optimisticValue, priorValue, priorStatus); + }, + CancellationToken.None, TaskContinuationOptions.None, TaskScheduler.Default); + + return ServiceResult.Good; } /// - /// Pure decision for an inbound equipment-tag write: the - /// role gate + the fire-and-forget dispatch, extracted off so it is - /// unit-testable without booting an SDK server. The gate fails closed (null identity or missing role - /// ⇒ BadUserAccessDenied, NOT invoked). When the gate passes but no - /// router is wired ( is null), the write resolves to BadNotWritable - /// ("writes unavailable"). Otherwise it invokes exactly once (fire-and-forget - /// — the actual driver round-trip happens asynchronously off the router lambda) and returns - /// so the SDK applies the client value optimistically. Role - /// comparison is case-insensitive (the role set is built with - /// ), matching the alarm gate. + /// Pure role + availability gate for an inbound equipment-tag write, extracted off + /// so it is unit-testable without booting an SDK server. Fails closed: + /// a null identity or an identity missing the role ⇒ + /// BadUserAccessDenied. When the gate passes but no real gateway is wired + /// ( is false) ⇒ BadNotWritable ("writes unavailable"). A + /// null return means "proceed" (the caller dispatches + returns Good). Role comparison is + /// case-insensitive (the role set is built with ), + /// matching the alarm gate. /// /// The role-carrying identity extracted off the SDK context, or null when the /// session is anonymous / carries no role-carrying identity. - /// A thunk that kicks off the asynchronous route of the write to the driver; invoked - /// only when the gate passes. Null when no router is wired (e.g. admin-only nodes). - /// on an allowed write (dispatch started); BadUserAccessDenied - /// when the gate vetoes; BadNotWritable ("writes unavailable") when no router is wired. - internal static ServiceResult EvaluateEquipmentWrite( - RoleCarryingUserIdentity? identity, Action? route) + /// True when a non-Null is wired; false + /// for the Null default (no route — e.g. admin-only nodes / pre-boot). + /// null to proceed (gate passed); otherwise the veto + /// (BadUserAccessDenied on a failed role gate, BadNotWritable when no gateway is wired). + internal static ServiceResult? EvaluateEquipmentWriteGate(RoleCarryingUserIdentity? identity, bool gatewayWired) { if (identity is null || !identity.Roles.Contains(OpcUaDataPlaneRoles.WriteOperate, StringComparer.OrdinalIgnoreCase)) { @@ -644,16 +676,56 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 return new ServiceResult(StatusCodes.BadUserAccessDenied); } - if (route is null) + if (!gatewayWired) { - // Gate passed but no router wired (admin-only nodes / pre-boot) ⇒ writes unavailable. + // Gate passed but no gateway wired (admin-only nodes / pre-boot) ⇒ writes unavailable. return new ServiceResult(StatusCodes.BadNotWritable, "writes unavailable"); } - // Fire-and-forget: kick off the asynchronous route and return Good immediately. The SDK holds the - // node-manager Lock here, so we must NOT block on the driver round-trip. - route(); - return ServiceResult.Good; + return null; + } + + /// + /// Pure decision for the write-outcome self-correction: revert the node to its pre-write value ONLY on + /// a FAILED outcome AND only while the node still holds the optimistic value. The + /// still-holds-the-optimistic-value check is what stops a revert from clobbering a fresh driver poll + /// that already republished the confirmed register value over the optimistic write. Pure (value + /// comparison via ) so it is unit-testable without an SDK + /// server. + /// + /// The device-write outcome routed back by the gateway. + /// The node's current Value at revert time. + /// The value the SDK optimistically applied on the write. + /// true to revert (failed outcome and node unchanged since the optimistic write); + /// false on success, or when a poll has already moved the node off the optimistic value. + internal static bool ShouldRevert(NodeWriteOutcome outcome, object? currentNodeValue, object? optimisticValue) => + !outcome.Success && Equals(currentNodeValue, optimisticValue); + + /// + /// Off-Lock continuation body for the write-outcome self-correction: re-takes Lock and, when + /// says so, reverts the node's Value + StatusCode to the captured pre-write + /// value/status and notifies subscribers (same node-update shape as ). A + /// no-op when the node was rebuilt/removed in the interim, when the outcome succeeded, or when a fresh + /// poll already moved the node off the optimistic value. Silent — this node manager carries no logger; + /// the gateway logs the underlying write failure. + /// + /// The string id of the written variable node. + /// The device-write outcome routed back by the gateway. + /// The value the SDK optimistically applied on the write. + /// The node's real value captured before the optimistic write. + /// The node's real status captured before the optimistic write. + private void RevertOptimisticWriteIfNeeded( + string nodeId, NodeWriteOutcome outcome, object? optimisticValue, object? priorValue, StatusCode priorStatus) + { + lock (Lock) + { + if (!_variables.TryGetValue(nodeId, out var variable)) return; // rebuilt/removed ⇒ no-op + if (!ShouldRevert(outcome, variable.Value, optimisticValue)) return; // success, or poll moved it on + variable.Value = priorValue; + variable.StatusCode = priorStatus; + variable.Timestamp = DateTime.UtcNow; + variable.ClearChangeMasks(SystemContext, includeChildren: false); + } } /// Map our domain AlarmType string to the matching SDK condition subtype. Script diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs index 978a44cf..b9275efb 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaSdkServer.cs @@ -37,22 +37,25 @@ public sealed class OtOpcUaSdkServer : StandardServer } /// - /// Wire the reverse-path sink for inbound operator writes to writable equipment-tag nodes onto the - /// created . The host calls this after start with a fire-and-forget - /// lambda that kicks off a bounded Ask of the local DriverHostActor - /// (RouteNodeWrite) and logs failures from a continuation — it must NOT block, because the - /// handler runs under the node-manager Lock (mirrors ). - /// No-op (returns false) when the node manager has not been created yet, so the caller can - /// detect a too-early call. + /// Wire the reverse-path gateway for inbound operator writes to writable equipment-tag nodes onto the + /// created . The host calls this after start with an + /// ActorNodeWriteGateway whose WriteAsync kicks off a bounded Ask of the local + /// DriverHostActor (RouteNodeWrite); the node manager calls it fire-and-forget and uses + /// the resolved NodeWriteOutcome to self-correct the node on a failed device write (mirrors + /// ). Passing null restores the + /// NullOpcUaNodeWriteGateway default (writes unavailable). No-op (returns false) when the + /// node manager has not been created yet, so the caller can detect a too-early call. /// - /// The router invoked by the writable node's OnWriteValue handler once the - /// WriteOperate gate passes; may be null to clear it. - /// true when the router was set on a live node manager; false when no node + /// The gateway invoked by the writable node's OnWriteValue handler once the + /// WriteOperate gate passes; may be null to restore the Null default. + /// true when the gateway was set on a live node manager; false when no node /// manager exists yet. - public bool SetNodeWriteRouter(Action? router) + public bool SetNodeWriteGateway(IOpcUaNodeWriteGateway? gateway) { if (_otOpcUaNodeManager is null) return false; - _otOpcUaNodeManager.NodeWriteRouter = router; + // The NodeWriteGateway setter null-coalesces to the Null default, so a null gateway is intentional + // (restores "writes unavailable"); forgive the nullable-in here. + _otOpcUaNodeManager.NodeWriteGateway = gateway!; return true; } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs index a334bdb1..06270e36 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests/EquipmentWriteGateTests.cs @@ -1,83 +1,111 @@ using Opc.Ua; using Shouldly; using Xunit; +using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; using ZB.MOM.WW.OtOpcUa.OpcUaServer.Security; namespace ZB.MOM.WW.OtOpcUa.OpcUaServer.Tests; /// -/// Task 11 — the inbound operator-write authz gate + fire-and-forget dispatch. The OnWriteValue handler -/// on a writable equipment-tag node extracts the caller's , gates -/// on the role (deny otherwise), and on pass kicks off the -/// fire-and-forget route through the and returns -/// Good (optimistic write). The pure decision is extracted into -/// so the gate + dispatch are unit-testable -/// without booting an SDK server: the handler just supplies the extracted identity and a thunk that -/// starts the router (or null when no router is wired). +/// Task 11 + write-outcome self-correction — the inbound operator-write authz/availability gate and the +/// compare-and-revert decision. The OnWriteValue handler on a writable equipment-tag node extracts the +/// caller's , gates on the +/// role (deny otherwise) AND on the gateway being wired +/// (BadNotWritable otherwise), then returns Good (optimistic write) and dispatches the write +/// fire-and-forget through the . When the async device +/// outcome comes back FAILED, the node self-corrects back to its real pre-write value — but only while the +/// node still holds the optimistic value (so a fresh poll isn't clobbered). +/// +/// Both decisions are extracted as pure statics so they're unit-testable without booting an SDK server: +/// (role + availability) and +/// (compare-and-revert). /// public sealed class EquipmentWriteGateTests { /// (a) A null identity (anonymous / no role-carrying identity on the context) is denied with - /// BadUserAccessDenied and the route thunk is NEVER invoked — the gate fails closed. + /// BadUserAccessDenied — the gate fails closed. [Fact] - public void Null_identity_is_denied_and_does_not_route() + public void Null_identity_is_denied() { - var routed = false; - var result = OtOpcUaNodeManager.EvaluateEquipmentWrite( + var result = OtOpcUaNodeManager.EvaluateEquipmentWriteGate( identity: null, - route: () => routed = true); + gatewayWired: true); - result.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); - routed.ShouldBeFalse(); + result.ShouldNotBeNull(); + result!.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); } /// (b) An identity WITHOUT the WriteOperate role is denied with - /// BadUserAccessDenied and the route thunk is NEVER invoked. + /// BadUserAccessDenied. [Fact] - public void Identity_without_WriteOperate_is_denied_and_does_not_route() + public void Identity_without_WriteOperate_is_denied() { - var routed = false; var identity = IdentityWith("ReadOnly", OpcUaDataPlaneRoles.AlarmAck); // no WriteOperate - var result = OtOpcUaNodeManager.EvaluateEquipmentWrite( + var result = OtOpcUaNodeManager.EvaluateEquipmentWriteGate( identity, - route: () => routed = true); + gatewayWired: true); - result.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); - routed.ShouldBeFalse(); + result.ShouldNotBeNull(); + result!.StatusCode.Code.ShouldBe(StatusCodes.BadUserAccessDenied); } - /// (c) An identity WITH the WriteOperate role and a non-null route invokes the route - /// thunk (fire-and-forget) and returns ServiceResult.Good so the SDK applies the value - /// optimistically. The role match is case-insensitive (the role set + gate both use + /// (c) An identity WITH the WriteOperate role and a wired gateway passes the gate + /// (returns null — proceed). The role match is case-insensitive (the role set + gate both use /// OrdinalIgnoreCase). [Fact] - public void Identity_with_WriteOperate_routes_and_returns_good() + public void Identity_with_WriteOperate_and_gateway_passes() { - var routed = false; var identity = IdentityWith("readonly", "writeoperate"); // lower-cased: case-insensitive match - var result = OtOpcUaNodeManager.EvaluateEquipmentWrite( + var result = OtOpcUaNodeManager.EvaluateEquipmentWriteGate( identity, - route: () => routed = true); + gatewayWired: true); - routed.ShouldBeTrue(); - result.ShouldBe(ServiceResult.Good); + result.ShouldBeNull(); } - /// (d) An identity WITH the WriteOperate role but a null route (no router wired — e.g. - /// admin-only nodes) maps to BadNotWritable ("writes unavailable") — the gate passes but there is - /// nowhere to route the write. + /// (d) An identity WITH the WriteOperate role but no gateway wired maps to + /// BadNotWritable ("writes unavailable") — the gate passes but there is nowhere to route. [Fact] - public void Identity_with_WriteOperate_and_null_route_maps_to_bad_not_writable() + public void Identity_with_WriteOperate_and_no_gateway_maps_to_bad_not_writable() { var identity = IdentityWith(OpcUaDataPlaneRoles.WriteOperate); - var result = OtOpcUaNodeManager.EvaluateEquipmentWrite( + var result = OtOpcUaNodeManager.EvaluateEquipmentWriteGate( identity, - route: null); + gatewayWired: false); - result.StatusCode.Code.ShouldBe(StatusCodes.BadNotWritable); + result.ShouldNotBeNull(); + result!.StatusCode.Code.ShouldBe(StatusCodes.BadNotWritable); result.LocalizedText.Text.ShouldContain("writes unavailable"); } + /// + /// decision table. Revert ONLY on a failed outcome AND + /// only while the node still holds the optimistic value (a fresh poll that moved the node on must not + /// be clobbered). The first bool is the outcome's Success flag. + /// + [Theory] + [InlineData(true, "v", "v", false)] // success → never revert (even though still-optimistic) + [InlineData(false, "v", "v", true)] // fail + node still holds optimistic → revert + [InlineData(false, "w", "v", false)] // fail + a poll moved the node on → no revert + [InlineData(true, "w", "v", false)] // success + moved on → no revert + public void ShouldRevert_decision_table(bool success, object currentNodeValue, object optimisticValue, bool expected) + { + var outcome = new NodeWriteOutcome(success, success ? null : "device rejected"); + + OtOpcUaNodeManager.ShouldRevert(outcome, currentNodeValue, optimisticValue).ShouldBe(expected); + } + + /// Null-value edges: a failed write whose node still holds the optimistic null reverts; + /// a failed write whose node moved off null does not. + [Fact] + public void ShouldRevert_null_value_edges() + { + var fail = new NodeWriteOutcome(false, "device rejected"); + + OtOpcUaNodeManager.ShouldRevert(fail, currentNodeValue: null, optimisticValue: null).ShouldBeTrue(); + OtOpcUaNodeManager.ShouldRevert(fail, currentNodeValue: null, optimisticValue: "v").ShouldBeFalse(); + } + private static RoleCarryingUserIdentity IdentityWith(params string[] roles) => new(new UserNameIdentityToken { UserName = "op" }, roles); }