diff --git a/docs/AlarmTracking.md b/docs/AlarmTracking.md index 85085bc5..e4ba256b 100644 --- a/docs/AlarmTracking.md +++ b/docs/AlarmTracking.md @@ -135,9 +135,11 @@ them by type + browse path), and neither were the Part 9 methods — calling the its declaration id. ⚠️ The ShelvingState methods needed help here: their SDK declaration id is the `ShelvedStateMachineType`'s (`i=2948`…), and they were only ever found by the `AlarmConditionType_ShelvingState_*` id (`i=9212`…) because that WAS their NodeId. `New` adopts that original id as the method's -`MethodDeclarationId` (only when it is a real standard-namespace method node, never an SDK-generated -number), so OneShotShelve/TimedShelve/Unshelve by the `AlarmConditionType_ShelvingState_*` id keep working. -Calling them by the `ShelvedStateMachineType_*` id is not supported. +`MethodDeclarationId` (a fixed set of those three ids — no address-space lookup, which would take other +node managers' locks under ours), so OneShotShelve/TimedShelve/Unshelve by the +`AlarmConditionType_ShelvingState_*` id keep working, as do calls by the condition's own instance method +ids (`{ConditionId}/ShelvingState/TimedShelve`, on the condition or its ShelvingState). Calling them by the +`ShelvedStateMachineType_*` id returns `BadMethodInvalid` — a pinned trade-off. - **ACL:** the slash-joined id makes a child's ACL scope its condition's scope plus the browse path, so with `Security:Acl:Enabled` a child is governed exactly like its condition (a grant covering the diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs index 057b4f49..1db91982 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer/OtOpcUaNodeManager.cs @@ -2499,6 +2499,20 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 _ => new AlarmConditionState(parent), }; + /// + /// Gitea #629 — the AlarmConditionType_ShelvingState_* method instance declarations a client calls + /// the shelve methods by. records them as the methods' MethodDeclarationId + /// (the SDK's own is the ShelvedStateMachineType's), so calls by these ids keep resolving after + /// the methods move to instance-scoped NodeIds. Calls by the ShelvedStateMachineType_* ids are + /// not supported (pinned by ConditionChildNodeIdTests). + /// + private static readonly HashSet ShelvingMethodDeclarationIds = + [ + MethodIds.AlarmConditionType_ShelvingState_TimedShelve, + MethodIds.AlarmConditionType_ShelvingState_OneShotShelve, + MethodIds.AlarmConditionType_ShelvingState_Unshelve, + ]; + /// /// Gitea #629 — the hook NodeState.Create(…, assignNodeIds: true) calls /// for the node being created and then, top-down, for every child it built from the type's definition. @@ -2543,15 +2557,10 @@ public sealed class OtOpcUaNodeManager : CustomNodeManager2 // already, over an SDK-generated ns=0 NodeId. The ShelvingState methods do NOT: their declaration id // is the ShelvedStateMachineType's (i=2948), and they were found only because their NodeId WAS the // AlarmConditionType instance declaration every client uses. Adopt that id as the declaration before - // it is replaced — but only when it is a real node of the standard address space, never an - // SDK-generated number — or every shelve/unshelve call fails BadMethodInvalid. - if (node is MethodState method - && method.NodeId is { NamespaceIndex: 0 } declarationId - && declarationId != method.MethodDeclarationId - && FindNodeInAddressSpace(declarationId) is MethodState) - { - method.MethodDeclarationId = declarationId; - } + // it is replaced, or every shelve/unshelve call fails BadMethodInvalid. A fixed set, deliberately: a + // lookup in the standard address space would take other node managers' locks while we hold Lock. + if (node is MethodState method && ShelvingMethodDeclarationIds.Contains(method.NodeId)) + method.MethodDeclarationId = method.NodeId; return new NodeId($"{parentId.Identifier}/{node.BrowseName.Name}", parentId.NamespaceIndex); } diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionChildNodeIdTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionChildNodeIdTests.cs index d46ddf0e..78445392 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionChildNodeIdTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.OpcUaServer.IntegrationTests/ConditionChildNodeIdTests.cs @@ -246,8 +246,96 @@ public sealed class ConditionChildNodeIdTests finally { await rig.DisposeAsync(); } } + /// + /// The shelve methods resolve by the condition's OWN child method ids + /// ({condition}/ShelvingState/TimedShelve, …/Unshelve) — the ids a browse of the condition + /// returns — whether the call names the condition or its ShelvingState as the object, and each accepted + /// call reports its one condition event. + /// + [Theory] + [InlineData(false)] + [InlineData(true)] + public async Task Shelve_methods_called_by_their_instance_ids_succeed_and_report(bool objectIsShelvingState) + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync($"CondChildMethodIds{objectIsShelvingState}", encrypted: false, ct); + try + { + rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", AllMethods)); + rig.MaterialiseScripted(); + var current = ConditionMethodEventTests.PreState("TimedShelve", native: false) with { Message = "base" }; + ConditionMethodEventTests.WireSimulatedOwners(rig, native: false, () => current, s => current = s); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var events = new RecordedEvents(); + var subscription = await RecordedEvents.SubscribeAsync(session, events, ct); + var item = RecordedEvents.AddEventItem(subscription, ObjectIds.Server); + await subscription.ApplyChangesAsync(ct); + + rig.Project(false, current); + await RecordedEvents.WaitUntilAsync(() => events.Has(item, "base")); + + var condition = ConditionNodeId(session, native: false); + var ns = condition.NamespaceIndex; + var shelvingState = new NodeId($"{ScriptedAlarmId}/ShelvingState", ns); + var target = objectIsShelvingState ? shelvingState : condition; + + await session.CallAsync(target, new NodeId($"{ScriptedAlarmId}/ShelvingState/TimedShelve", ns), ct, 60_000d); + await rig.DrainProjectionsAsync(); + await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == 2); + events.ConditionEvents(item, "base")[1][RecordedEvents.ShelvingField].Value + .ShouldBe(ObjectIds.ShelvedStateMachineType_TimedShelved); + + await session.CallAsync(target, new NodeId($"{ScriptedAlarmId}/ShelvingState/Unshelve", ns), ct); + await rig.DrainProjectionsAsync(); + await RecordedEvents.WaitUntilAsync(() => events.ConditionEvents(item, "base").Count == 3); + events.ConditionEvents(item, "base")[2][RecordedEvents.ShelvingField].Value + .ShouldBe(ObjectIds.ShelvedStateMachineType_Unshelved); + + await subscription.DeleteAsync(true, ct); + } + finally { await rig.DisposeAsync(); } + } + + /// + /// Pins the #629 trade-off: the shelve methods resolve by the condition's instance ids and by the + /// AlarmConditionType_ShelvingState_* ids every client uses (the positive control), but NOT by + /// the ShelvedStateMachineType_* ids — those were the methods' SDK declarations, replaced by the + /// AlarmConditionType ones when the methods moved off their ns=0 NodeIds. Change this deliberately. + /// + [Fact] + public async Task A_shelve_call_by_the_state_machine_types_method_id_is_not_supported() + { + var ct = TestContext.Current.CancellationToken; + var rig = await StartAsync("CondChildMethodStateMachineId", encrypted: false, ct); + try + { + rig.Enforce(true, [Key()], ClusterGrant(KeyPrincipal, "acl-key", AllMethods)); + rig.MaterialiseScripted(); + var current = ConditionMethodEventTests.PreState("OneShotShelve", native: false); + ConditionMethodEventTests.WireSimulatedOwners(rig, native: false, () => current, s => current = s); + rig.Project(false, current); + + using var session = await rig.ConnectAsync(anonymous: false, ct); + var condition = ConditionNodeId(session, native: false); + + var refused = await Should.ThrowAsync( + session.CallAsync(condition, MethodIds.ShelvedStateMachineType_OneShotShelve, ct)); + refused.StatusCode.ShouldBe(StatusCodes.BadMethodInvalid); + + // Positive control: the AlarmConditionType declaration id resolves on the same condition. + await session.CallAsync(condition, MethodIds.AlarmConditionType_ShelvingState_OneShotShelve, ct); + } + finally { await rig.DisposeAsync(); } + } + // ───────────────────────────── helpers ───────────────────────────── + private const NodePermissions AllMethods = + NodePermissions.Browse | NodePermissions.Read | NodePermissions.Subscribe | NodePermissions.AlarmRead + | NodePermissions.AlarmAcknowledge | NodePermissions.AlarmConfirm | NodePermissions.AlarmShelve + | NodePermissions.MethodCall; + /// /// Native: active, unacked, "native-distinct". Scripted: active, unacked, "scripted-distinct", timed /// shelved for 60 s — none of which the condition type's instance declarations can hold.