From df44b460bed2dd28f4ff422fba6677f4fa815183 Mon Sep 17 00:00:00 2001 From: Marc Schier Date: Wed, 2 Sep 2026 13:08:49 +0200 Subject: [PATCH] Complete CreateInstanceOf node lifecycle during registration (#4360) # Description Generated `CreateInstanceOf` factories build typed node graphs and assign per-instance NodeIds, but intentionally leave the node create lifecycle open. Hand-written callers commonly registered those graphs without calling `CreateAsPredefinedNode`, so `OnBeforeCreate`, `OnAfterCreate`, and change-mask clearing never ran. This change: - adds `NodeState.IsCreated` and makes lifecycle completion idempotent per node; - completes newly materialized children, including children created by `OnAfterCreate`; - bounds late-child lifecycle convergence and fails explicitly if callbacks keep adding uncreated nodes; - completes uncreated nodes automatically before predefined-node registration and behavior wiring in both async and legacy custom node managers; - assigns/rebases async-node-manager NodeIds before lifecycle callbacks; - emits a null-safe Debug diagnostic when registration performs completion; - documents the generated-factory lifecycle contract and migration behavior; - adds regressions for condition/alarm method binding, finite-state-machine namespaces, Fluent state-node rebasing, behavior replacement, cloning, deletion, late children, non-converging callbacks, and synchronous/async registration. The tracked `src/` and `samples/` call-site audit covered 2,953 C# files. No additional ordering-sensitive call sites require explicit completion beyond the three existing DI/Robotics sites. ## Validation - `dotnet build UA.slnx -c Release -p:CustomTestTarget=net10.0`: passed. - `Opc.Ua.Server.Tests` net10.0: 4,811 passed, 5 skipped. - `NodeStateTests` net10.0 and net48: 213 passed on each. - Node-manager lifecycle and null-logger tests net48: 459 passed. - Source-generation core tests net10.0 and net48: 3,802 passed, 8 skipped on each. - DI, Robotics, ISA95, Positioning, Vision, Vision.Intent, and XRegistry suites passed on net10.0 and net48. - Generated alarm lifecycle regression passed on net10.0 and net48. The solution build reports six pre-existing CA warnings in untouched files. The OpenUSD suite has one pre-existing locale-sensitive failure in `StructuredCartesianCoordinatesAreAccepted` (`1.5` expected versus `1,5` under `en-DE`); all other OpenUSD tests passed on both tested frameworks. ## Related Issues - Fixes #4320 ## Checklist - [ ] I have signed the [CLA](https://opcfoundation.org/license/cla/ContributorLicenseAgreementv1.0.pdf) and read the [CONTRIBUTING](https://github.com/OPCFoundation/UA-.NETStandard/blob/master/CONTRIBUTING.md) doc. - [x] I have added tests that prove my fix is effective or that my feature works and increased code coverage. - [x] I have added all necessary documentation. - [x] I have verified that my changes do not introduce (new) build or analyzer warnings. - [ ] I ran **all** tests locally using the **UA.slnx** solution against at least .net **framework** and .net **10**, and all passed. - [ ] I fixed **all** failing and flaky tests in the CI pipelines and **all** CodeQL warnings. - [x] I have addressed **all** PR feedback received. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 7ffdc400-0710-4a2e-a9d1-c0e1a9b3b81e --- .../references/stack-migration/node-states.md | 41 +++ docs/DeviceIntegration.md | 5 +- docs/NodeManagers.md | 23 ++ docs/Robotics.md | 3 +- docs/migrate/2.0.x/node-states.md | 42 ++- .../NodeManager/AsyncCustomNodeManager.cs | 43 ++- .../NodeManager/CustomNodeManager.cs | 26 +- src/Opc.Ua.Server/NodeManagerLog.cs | 9 + src/Opc.Ua.Types/State/NodeState.cs | 97 ++++-- src/Opc.Ua.Types/State/readme.md | 31 +- .../GeneratorProtectionAlarmNodeTests.cs | 23 ++ .../AsyncCustomNodeManagerTests.cs | 306 ++++++++++++++++++ .../State/NodeStateTests.cs | 176 +++++++++- .../Generators/NodeStateTemplates.cs | 21 ++ 14 files changed, 794 insertions(+), 52 deletions(-) diff --git a/.agents/skills/opcua-v20-migration/references/stack-migration/node-states.md b/.agents/skills/opcua-v20-migration/references/stack-migration/node-states.md index a67c079743..4efa56e351 100644 --- a/.agents/skills/opcua-v20-migration/references/stack-migration/node-states.md +++ b/.agents/skills/opcua-v20-migration/references/stack-migration/node-states.md @@ -113,6 +113,47 @@ protected override void OnAfterCreate(ISystemContext context, NodeState node, Ca } ``` +#### Generated instance factories and the create lifecycle + +Source-generated `CreateInstanceOf` factories materialise the typed node +graph and assign per-instance NodeIds, but intentionally leave the create +lifecycle open so callers can finish assembling and configuring the subtree. +In 2.0, registering the graph through `AddPredefinedNodeAsync`, +`AddNodeAsync`, or the synchronous predefined-node registration path +automatically runs `OnBeforeCreate`/`OnAfterCreate` and clears change masks +before indexing. The asynchronous paths assign or rebase NodeIds before the +callbacks, so they see the identifiers which enter the address space. +Synchronous predefined-node registration preserves the caller-assigned +NodeIds and expects them to be ready. `NodeState.IsCreated` exposes whether +an individual node has completed that lifecycle. + +`AddBehaviourToPredefinedNode` and its asynchronous equivalent receive the +created node. An override which replaces a passive node may therefore observe +the passive node's lifecycle before returning the active replacement; the +replacement is also completed before indexing. Keep external lifecycle side +effects idempotent when using that legacy replacement pattern. + +If pre-registration code depends on values established by `OnAfterCreate`, or +sets state or handlers which an `OnAfterCreate` override would replace, call +`CreateAsPredefinedNode` first: + +```csharp +MyMachineState machine = + context.CreateInstanceOfMyMachineType(parent, browseName); + +machine.CreateAsPredefinedNode(context); +SeedInitialState(machine); + +await AddPredefinedNodeAsync(context, machine, cancellationToken); +``` + +`CreateAsPredefinedNode` is idempotent per node and still completes newly +added children. It does not re-run `OnAfterCreate` on an ancestor which was +already created, so add children whose handlers are wired by a parent callback +before the parent's first completion, or wire those late children explicitly. +This prevents duplicate callback execution when an explicitly completed +subtree is later registered. + #### NodeState FindChild and CreateChild state NodeId assignment `NodeState.FindChild` and `NodeState.CreateChild` now take diff --git a/docs/DeviceIntegration.md b/docs/DeviceIntegration.md index a0bfe0920f..6807e56469 100644 --- a/docs/DeviceIntegration.md +++ b/docs/DeviceIntegration.md @@ -217,8 +217,9 @@ ITopologyElementBuilder TopologyElementByBrowseName( sets BrowseName/SymbolicName/DisplayName and stamps the `TypeDefinitionId`. 5. Calls the real `AsyncCustomNodeManager.AddPredefinedNodeAsync` so - subscription wiring, type-tree registration, and root-notifier - propagation all happen exactly as for nodes loaded from a NodeSet2. + the node create lifecycle, subscription wiring, type-tree registration, + and root-notifier propagation all happen exactly as for nodes loaded from + a NodeSet2. Machinery-aware subclasses can additionally reference managed machines from the OPC 40001 `Machines` entry point. `DiNodeManager`'s helper adds diff --git a/docs/NodeManagers.md b/docs/NodeManagers.md index c86d001441..5f0c0d41ca 100644 --- a/docs/NodeManagers.md +++ b/docs/NodeManagers.md @@ -1826,6 +1826,25 @@ pump.CreateChild(SystemContext, someBrowseName); // CreateOrReplace` rebases in a single pass afterwards. The same parameter is available to you if you need that behaviour. +* `CreateInstanceOf` assigns identity but does not call + `CreateAsPredefinedNode`. `AddPredefinedNodeAsync`, `AddNodeAsync`, and the + synchronous predefined-node registration paths complete the lifecycle + automatically. * Assignment only happens when the context carries an `ISystemContext.NodeIdFactory`. `AsyncCustomNodeManager` supplies one that allocates from the manager's namespace; override `New` to derive diff --git a/docs/Robotics.md b/docs/Robotics.md index 42c3d36353..995e0c70ab 100644 --- a/docs/Robotics.md +++ b/docs/Robotics.md @@ -301,7 +301,8 @@ Instances are always materialised through the generated `ISystemContext.CreateInstanceOf` factories (for example `CreateInstanceOfMotionDeviceSystemType`), so each instance carries the full companion-type structure and its per-instance NodeIds, rather than only a -type-definition reference on a bare `BaseObjectState`. +type-definition reference on a bare `BaseObjectState`. Registration completes +the node create lifecycle before the subtree enters the address space. ### Binding live values diff --git a/docs/migrate/2.0.x/node-states.md b/docs/migrate/2.0.x/node-states.md index b77c68499c..4513cbc25e 100644 --- a/docs/migrate/2.0.x/node-states.md +++ b/docs/migrate/2.0.x/node-states.md @@ -113,6 +113,47 @@ protected override void OnAfterCreate(ISystemContext context, NodeState node, Ca } ``` +#### Generated instance factories and the create lifecycle + +Source-generated `CreateInstanceOf` factories materialise the typed node +graph and assign per-instance NodeIds, but intentionally leave the create +lifecycle open so callers can finish assembling and configuring the subtree. +In 2.0, registering the graph through `AddPredefinedNodeAsync`, +`AddNodeAsync`, or the synchronous predefined-node registration path +automatically runs `OnBeforeCreate`/`OnAfterCreate` and clears change masks +before indexing. The asynchronous paths assign or rebase NodeIds before the +callbacks, so they see the identifiers which enter the address space. +Synchronous predefined-node registration preserves the caller-assigned +NodeIds and expects them to be ready. `NodeState.IsCreated` exposes whether +an individual node has completed that lifecycle. + +`AddBehaviourToPredefinedNode` and its asynchronous equivalent receive the +created node. An override which replaces a passive node may therefore observe +the passive node's lifecycle before returning the active replacement; the +replacement is also completed before indexing. Keep external lifecycle side +effects idempotent when using that legacy replacement pattern. + +If pre-registration code depends on values established by `OnAfterCreate`, or +sets state or handlers which an `OnAfterCreate` override would replace, call +`CreateAsPredefinedNode` first: + +```csharp +MyMachineState machine = + context.CreateInstanceOfMyMachineType(parent, browseName); + +machine.CreateAsPredefinedNode(context); +SeedInitialState(machine); + +await AddPredefinedNodeAsync(context, machine, cancellationToken); +``` + +`CreateAsPredefinedNode` is idempotent per node and still completes newly +added children. It does not re-run `OnAfterCreate` on an ancestor which was +already created, so add children whose handlers are wired by a parent callback +before the parent's first completion, or wire those late children explicitly. +This prevents duplicate callback execution when an explicitly completed +subtree is later registered. + #### NodeState FindChild and CreateChild state NodeId assignment `NodeState.FindChild` and `NodeState.CreateChild` now take @@ -323,4 +364,3 @@ bool isType = await cache.IsTypeOfAsync(sub, super); - Related: [types.md](types.md), [alarms-model-change.md](alarms-model-change.md), [sessions-subscriptions.md](sessions-subscriptions.md). - [2.0 migration index](README.md) — analyzer quick-start + symptom → sub-doc table. - [Migration Guide](../../MigrationGuide.md) — landing page across versions. - diff --git a/src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs b/src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs index 5485569cbf..69742c4072 100644 --- a/src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs +++ b/src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs @@ -970,8 +970,12 @@ namespace Opc.Ua.Server } /// - /// Add a created instance and its children to the NodeManagers AddressSpace. Assigns NodeIds if needed and fixes ReferenceTargets after assigning NodeIds. + /// Adds an instance and its children to the NodeManager address space. /// + /// + /// Assigns NodeIds if needed, updates reference targets, and completes + /// the create lifecycle before indexing the subtree. + /// /// The operation context. /// An optional parent identifier. /// The instance to create. @@ -1040,6 +1044,11 @@ namespace Opc.Ua.Server /// children are kept verbatim and used as PredefinedNodes keys. /// /// + /// Registration completes the create lifecycle for nodes which have + /// not already passed through + /// . + /// + /// /// The supplied should already be attached to /// its parent (via parent.AddChild) when invoked, if a parent /// linkage is required for browse references. @@ -2042,8 +2051,14 @@ namespace Opc.Ua.Server /// protected virtual async ValueTask AddPredefinedNodeAsync(ISystemContext context, NodeState node, CancellationToken cancellationToken = default) { + PrepareInstanceNodeIdsForRegistration(context, node); + CompleteCreateLifecycleForRegistration(context, node); NodeState activeNode = await AddBehaviourToPredefinedNodeAsync(context, node, cancellationToken).ConfigureAwait(false); - PrepareInstanceNodeIdsForRegistration(context, activeNode); + if (!ReferenceEquals(activeNode, node)) + { + PrepareInstanceNodeIdsForRegistration(context, activeNode); + CompleteCreateLifecycleForRegistration(context, activeNode); + } IndexPredefinedNode(activeNode); var children = new List(); @@ -2204,8 +2219,9 @@ namespace Opc.Ua.Server /// this path does not invoke /// — it is intended /// for plain instance nodes that carry no asynchronous behaviour - /// wiring. Registration is idempotent, so a subsequent tree - /// registration that includes the same node is safe. + /// wiring. It completes the create lifecycle before indexing. + /// Registration is idempotent, so a subsequent tree registration + /// that includes the same node is safe. /// /// The node subtree to register. /// @@ -2223,6 +2239,7 @@ namespace Opc.Ua.Server private void AddPredefinedNodeSynchronously(ISystemContext context, NodeState node) { + CompleteCreateLifecycleForRegistration(context, node); IndexPredefinedNode(node); var children = new List(); @@ -2240,6 +2257,24 @@ namespace Opc.Ua.Server } } + private void CompleteCreateLifecycleForRegistration( + ISystemContext context, + NodeState node) + { + if (node.IsCreated) + { + return; + } + + node.CreateAsPredefinedNode(context); + if (m_logger != null) + { + m_logger.PredefinedNodeLifecycleCompletedAtRegistration( + node.NodeId, + node.BrowseName); + } + } + /// /// Indexes a single (already behaviour-resolved) node into the /// table, the type tree, and the diff --git a/src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs b/src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs index a4fdaba3e8..f97fa17131 100644 --- a/src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs +++ b/src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs @@ -1095,7 +1095,8 @@ namespace Opc.Ua.Server } /// - /// Recursively indexes the node and its children. + /// Completes the create lifecycle and recursively indexes the node and + /// its children. /// protected virtual void AddPredefinedNode(ISystemContext context, NodeState node) { @@ -1110,7 +1111,12 @@ namespace Opc.Ua.Server Server.TypeTree); } + CompleteCreateLifecycleForRegistration(context, node); NodeState activeNode = AddBehaviourToPredefinedNode(context, node); + if (!ReferenceEquals(activeNode, node)) + { + CompleteCreateLifecycleForRegistration(context, activeNode); + } PredefinedNodes.AddOrUpdate(activeNode.NodeId, activeNode, (key, _) => activeNode); if (activeNode is BaseTypeState type) @@ -1173,6 +1179,24 @@ namespace Opc.Ua.Server } } + private void CompleteCreateLifecycleForRegistration( + ISystemContext context, + NodeState node) + { + if (node.IsCreated) + { + return; + } + + node.CreateAsPredefinedNode(context); + if (m_logger != null) + { + m_logger.PredefinedNodeLifecycleCompletedAtRegistration( + node.NodeId, + node.BrowseName); + } + } + /// /// Recursively indexes the node and its children. /// diff --git a/src/Opc.Ua.Server/NodeManagerLog.cs b/src/Opc.Ua.Server/NodeManagerLog.cs index 5ed474559f..653c402492 100644 --- a/src/Opc.Ua.Server/NodeManagerLog.cs +++ b/src/Opc.Ua.Server/NodeManagerLog.cs @@ -47,5 +47,14 @@ namespace Opc.Ua.Server [LoggerMessage(EventId = ServerEventIds.NodeManager + 2, Level = LogLevel.Debug, Message = "Cleared historical access advertisement for NodeId={NodeId} because no historian provider is wired.")] public static partial void HistoryAdvertisementCleared(this ILogger logger, NodeId nodeId); + + [LoggerMessage(EventId = ServerEventIds.NodeManager + 3, Level = LogLevel.Debug, + Message = "Completed the create lifecycle while registering NodeId={NodeId}, " + + "BrowseName={BrowseName}. Call CreateAsPredefinedNode before registration " + + "when configuration depends on OnAfterCreate.")] + public static partial void PredefinedNodeLifecycleCompletedAtRegistration( + this ILogger logger, + NodeId nodeId, + QualifiedName browseName); } } diff --git a/src/Opc.Ua.Types/State/NodeState.cs b/src/Opc.Ua.Types/State/NodeState.cs index decd940af9..4b70f1be59 100644 --- a/src/Opc.Ua.Types/State/NodeState.cs +++ b/src/Opc.Ua.Types/State/NodeState.cs @@ -2463,6 +2463,15 @@ namespace Opc.Ua /// public bool Initialized { get; set; } + /// + /// True if the node has completed its create lifecycle. + /// + /// + /// resets this state. Removing a node from a node + /// manager without deleting the instance does not. + /// + public bool IsCreated { get; private set; } + /// /// True if the node must be validated with the underlying system before use. /// @@ -3041,7 +3050,7 @@ namespace Opc.Ua Initialize(context); // Call OnBeforeCreate on all children. - CallOnBeforeCreate(context); + CallOnBeforeCreate(context, true); // override node id. if (!nodeId.IsNull) @@ -3063,13 +3072,16 @@ namespace Opc.Ua DisplayName = displayName; } - CreateInternal(context, assignNodeIds); + CreateInternal(context, assignNodeIds, true); } /// /// Internal create sequence without node assignments /// - private void CreateInternal(ISystemContext context, bool assignNodeIds) + private void CreateInternal( + ISystemContext context, + bool assignNodeIds, + bool forceCreateLifecycle) { // get all children. var children = new List(); @@ -3088,24 +3100,62 @@ namespace Opc.Ua UpdateReferenceTargets(context, children, mappingTable); } - CallOnAfterCreate(context, children); + CallOnAfterCreate(context, children, forceCreateLifecycle); + + const int maxLifecycleCompletionPasses = 100; + for (int pass = 0; HasUncreatedNodes(context); pass++) + { + if (pass >= maxLifecycleCompletionPasses) + { + throw new InvalidOperationException( + "The node create lifecycle did not converge because lifecycle callbacks " + + "kept adding uncreated nodes."); + } + + CallOnBeforeCreate(context, false); + CallOnAfterCreate(context, null, false); + } ClearChangeMasks(context, true); } - /// - /// Recusivesly calls OnBeforeCreate for the node and its children. - /// - private void CallOnBeforeCreate(ISystemContext context) + private bool HasUncreatedNodes(ISystemContext context) { - OnBeforeCreate(context, this); + if (!IsCreated) + { + return true; + } var children = new List(); GetChildren(context, children); for (int ii = 0; ii < children.Count; ii++) { - children[ii].CallOnBeforeCreate(context); + if (children[ii].HasUncreatedNodes(context)) + { + return true; + } + } + + return false; + } + + /// + /// Recusivesly calls OnBeforeCreate for the node and its children. + /// + private void CallOnBeforeCreate(ISystemContext context, bool force) + { + if (force || !IsCreated) + { + OnBeforeCreate(context, this); + } + + var children = new List(); + GetChildren(context, children); + + for (int ii = 0; ii < children.Count; ii++) + { + children[ii].CallOnBeforeCreate(context, force); } } @@ -3133,7 +3183,11 @@ namespace Opc.Ua /// /// Recusivesly calls OnAfterCreate for the node and its children. /// - private void CallOnAfterCreate(ISystemContext context, List? children, CancellationToken ct = default) + private void CallOnAfterCreate( + ISystemContext context, + List? children, + bool force, + CancellationToken ct = default) { if (children == null) { @@ -3143,10 +3197,14 @@ namespace Opc.Ua for (int ii = 0; ii < children.Count; ii++) { - children[ii].CallOnAfterCreate(context, null, ct); + children[ii].CallOnAfterCreate(context, null, force, ct); } - OnAfterCreate(context, this, ct); + if (force || !IsCreated) + { + OnAfterCreate(context, this, ct); + IsCreated = true; + } } /// @@ -3155,8 +3213,8 @@ namespace Opc.Ua public virtual void Create(ISystemContext context, NodeState source) { Initialize(context, source); - CallOnBeforeCreate(context); - CreateInternal(context, false); + CallOnBeforeCreate(context, true); + CreateInternal(context, false, true); } /// @@ -3176,18 +3234,19 @@ namespace Opc.Ua OnAfterDelete(context); + IsCreated = false; ChangeMasks = NodeStateChangeMasks.Deleted; ClearChangeMasks(context, false); } /// - /// Called when the predefined node was fully created. Called - /// by generated code after all children have been created. + /// Completes the create lifecycle for a predefined node and any + /// children which have not already completed it. /// public void CreateAsPredefinedNode(ISystemContext context) { - CallOnBeforeCreate(context); - CreateInternal(context, false); + CallOnBeforeCreate(context, false); + CreateInternal(context, false, false); } /// diff --git a/src/Opc.Ua.Types/State/readme.md b/src/Opc.Ua.Types/State/readme.md index efd3ec5699..74a62ae8c7 100644 --- a/src/Opc.Ua.Types/State/readme.md +++ b/src/Opc.Ua.Types/State/readme.md @@ -121,23 +121,24 @@ sequenceDiagram ## Usage Inside the Server -The generated Create and CreateInstanceOf methods (static extension -method to ISystemContext) create a new type or instance of a type (respectively) and returns -it with all mandatory children set (for the instantiation) and with all children set for the -type (but then the child will only define the "modelling rule" for the instance. +The generated `Create` and `CreateInstanceOf` +extension methods create a type declaration or a typed instance, +respectively. The instance factory materialises all mandatory children and +assigns per-instance NodeIds when a browse name and `NodeIdFactory` are +available. -CreateInstanceOf does call Initialize(ITelemetryContext) method. TODO: +The returned instance has not completed its create lifecycle. Node manager +registration completes `OnBeforeCreate`/`OnAfterCreate` and clears change +masks before indexing it. Asynchronous registration assigns any required +NodeIds first; synchronous predefined-node registration preserves +caller-assigned NodeIds. Call `CreateAsPredefinedNode` explicitly when code +must read lifecycle-established state, or set state or handlers which +`OnAfterCreate` would otherwise replace, before registration. -The caller can then add optional children and place holders as needed using "Add()" -and "Add(QualifiedName browseName)" methods (for optional children and placeholders -defined in the type definition). - -These internally call Create(this, forInstance: true) to create the optional -children of the instantiated type. - -No "user" initialization calls are needed after the Create method is called. -User can add additional references and children to all states using AddChild/AddReference -and so on. +The caller can add optional children and placeholders with the generated +`Add()` methods. These call +`Create(this, forInstance: true)` internally. Additional +references and children can be attached with `AddReference` and `AddChild`. ### Legacy behavior diff --git a/tests/Opc.Ua.OpenUsd.Tests/Generator/GeneratorProtectionAlarmNodeTests.cs b/tests/Opc.Ua.OpenUsd.Tests/Generator/GeneratorProtectionAlarmNodeTests.cs index ea47c8ae33..eb2f373c9d 100644 --- a/tests/Opc.Ua.OpenUsd.Tests/Generator/GeneratorProtectionAlarmNodeTests.cs +++ b/tests/Opc.Ua.OpenUsd.Tests/Generator/GeneratorProtectionAlarmNodeTests.cs @@ -81,6 +81,29 @@ namespace Opc.Ua.OpenUsd.Tests.Generator }); } + /// + /// Completing the generated alarm instance binds the standard condition + /// methods supplied by its base classes. + /// + [Test] + public void CompletingFactoryCreatedAlarmBindsConditionMethods() + { + SystemContext context = CreateContext(); + GeneratorModel.GeneratorProtectionAlarmState alarm = CreateAlarm(context); + + Assert.That(alarm.IsCreated, Is.False); + Assert.That(alarm.Enable!.OnCallMethod, Is.Null); + Assert.That(alarm.Disable!.OnCallMethod, Is.Null); + Assert.That(alarm.AddComment!.OnCall, Is.Null); + + alarm.CreateAsPredefinedNode(context); + + Assert.That(alarm.IsCreated, Is.True); + Assert.That(alarm.Enable.OnCallMethod, Is.Not.Null); + Assert.That(alarm.Disable.OnCallMethod, Is.Not.Null); + Assert.That(alarm.AddComment.OnCall, Is.Not.Null); + } + /// /// The production path gives every published member a reference a browse can /// follow. diff --git a/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs b/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs index 9d27718a05..1e50c8cf86 100644 --- a/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs +++ b/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs @@ -39,6 +39,7 @@ using System.Threading.Tasks; using Microsoft.Extensions.Logging; using Moq; using NUnit.Framework; +using Opc.Ua.Server.StateMachines; namespace Opc.Ua.Server.Tests { @@ -168,6 +169,229 @@ namespace Opc.Ua.Server.Tests } } + [Test] + public async Task AddPredefinedNodeCompletesCreateLifecycleAsync() + { + using ITestNodeManager manager = CreateManager(); + ushort namespaceIndex = manager.NamespaceIndexes[0]; + var node = new LifecycleProbeState(null) + { + NodeId = new NodeId("Lifecycle", namespaceIndex), + BrowseName = new QualifiedName("Lifecycle", namespaceIndex), + DisplayName = new LocalizedText("Lifecycle") + }; + node.UpdateChangeMasks(NodeStateChangeMasks.Children); + m_mockLogger + .Setup(logger => logger.IsEnabled(LogLevel.Debug)) + .Returns(true); + + await manager.AddPredefinedNodeAsync(manager.SystemContext, node).ConfigureAwait(false); + await manager.AddPredefinedNodeAsync(manager.SystemContext, node).ConfigureAwait(false); + + Assert.That(node.IsCreated, Is.True); + Assert.That(node.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(node.AfterCreateCount, Is.EqualTo(1)); + Assert.That(node.ChangeMasks, Is.EqualTo(NodeStateChangeMasks.None)); + Assert.That(manager.Find(node.NodeId), Is.SameAs(node)); +#pragma warning disable CA1873 // Expression tree in Moq Verify is not an executed logging call + m_mockLogger.Verify( + logger => logger.Log( + LogLevel.Debug, + It.Is(eventId => + eventId.Name == "PredefinedNodeLifecycleCompletedAtRegistration"), + It.IsAny(), + It.IsAny(), + (Func)It.IsAny()), + Times.Once); +#pragma warning restore CA1873 + } + + [Test] + public async Task AddNodeCompletesCreateLifecycleAsync() + { + using ITestNodeManager manager = CreateManager(); + ushort namespaceIndex = manager.NamespaceIndexes[0]; + var node = new LifecycleProbeState(null) + { + NodeId = new NodeId("RuntimeLifecycle", namespaceIndex), + BrowseName = new QualifiedName("RuntimeLifecycle", namespaceIndex), + DisplayName = new LocalizedText("RuntimeLifecycle") + }; + + await manager + .AddNodeAsync(manager.SystemContext, NodeId.Null, node) + .ConfigureAwait(false); + + Assert.That(node.IsCreated, Is.True); + Assert.That(node.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(node.AfterCreateCount, Is.EqualTo(1)); + } + + [Test] + public async Task PredefinedNodeIdIsAssignedBeforeCreateLifecycleAsync() + { + using ITestNodeManager manager = CreateManager(); + Assume.That(manager is TestableAsyncCustomNodeManager, + "Requires AsyncCustomNodeManager NodeId preparation"); + var node = new LifecycleProbeState(null) + { + BrowseName = new QualifiedName("AssignedBeforeLifecycle", manager.NamespaceIndexes[0]), + DisplayName = new LocalizedText("AssignedBeforeLifecycle") + }; + + await manager + .AddPredefinedNodeAsync(manager.SystemContext, node) + .ConfigureAwait(false); + + Assert.That(node.NodeId.IsNull, Is.False); + Assert.That(node.NodeIdAtAfterCreate, Is.EqualTo(node.NodeId)); + } + + [Test] + public async Task BehaviourReplacementIsCompletedAfterOriginalAsync() + { + using ITestNodeManager manager = CreateManager(); + ushort namespaceIndex = manager.NamespaceIndexes[0]; + var original = new LifecycleProbeState(null) + { + NodeId = new NodeId("BehaviourReplacement", namespaceIndex), + BrowseName = new QualifiedName("BehaviourReplacement", namespaceIndex), + DisplayName = new LocalizedText("BehaviourReplacement") + }; + var replacement = new LifecycleProbeState(null) + { + NodeId = original.NodeId, + BrowseName = original.BrowseName, + DisplayName = original.DisplayName + }; + bool hookSawCreatedNode = false; + manager.AddBehaviourCallback = node => + { + hookSawCreatedNode = node.IsCreated; + return replacement; + }; + + await manager + .AddPredefinedNodeAsync(manager.SystemContext, original) + .ConfigureAwait(false); + + Assert.That(hookSawCreatedNode, Is.True); + Assert.That(original.IsCreated, Is.True); + Assert.That(replacement.IsCreated, Is.True); + Assert.That(manager.Find(original.NodeId), Is.SameAs(replacement)); + } + + [Test] + public async Task RegisteringGeneratedConditionBindsMethodsAsync() + { + using ITestNodeManager manager = CreateManager(); + ushort namespaceIndex = manager.NamespaceIndexes[0]; + ConditionState condition = manager.SystemContext.CreateInstanceOfConditionType( + null, + new QualifiedName("Condition", namespaceIndex)); + + Assert.That(condition.IsCreated, Is.False); + Assert.That(condition.Enable!.OnCallMethod, Is.Null); + Assert.That(condition.Disable!.OnCallMethod, Is.Null); + Assert.That(condition.AddComment!.OnCall, Is.Null); + + await manager + .AddPredefinedNodeAsync(manager.SystemContext, condition) + .ConfigureAwait(false); + + Assert.That(condition.IsCreated, Is.True); + Assert.That(condition.Enable.OnCallMethod, Is.Not.Null); + Assert.That(condition.Disable.OnCallMethod, Is.Not.Null); + Assert.That(condition.AddComment.OnCall, Is.Not.Null); + } + + [Test] + public async Task RegisteringStateMachineResolvesElementNamespaceAsync() + { + using ITestNodeManager manager = CreateManager(); + ushort instanceNamespaceIndex = manager.NamespaceIndexes[0]; + ushort elementNamespaceIndex = (ushort)manager.SystemContext.NamespaceUris + .GetIndexOrAppend(NamespaceFiniteStateMachineState.ElementsNamespaceUri); + var machine = new NamespaceFiniteStateMachineState(null) + { + NodeId = new NodeId("StateMachine", instanceNamespaceIndex), + BrowseName = new QualifiedName("StateMachine", instanceNamespaceIndex), + DisplayName = new LocalizedText("StateMachine") + }; + + Assert.That(machine.GetStateNodeId(123), Is.EqualTo(new NodeId(123))); + + await manager + .AddPredefinedNodeAsync(manager.SystemContext, machine) + .ConfigureAwait(false); + + Assert.That(machine.IsCreated, Is.True); + Assert.That( + machine.GetStateNodeId(123), + Is.EqualTo(new NodeId(123, elementNamespaceIndex))); + } + + [Test] + public async Task RegistrationRebasesLifecycleMaterializedStateNodesAsync() + { + using ITestNodeManager manager = CreateManager(); + Assume.That(manager is TestableAsyncCustomNodeManager, + "Requires AsyncCustomNodeManager NodeId preparation"); + StateMachineDefinition definition = StateMachineBuilder + .Create( + null, + manager.SystemContext, + new NodeId("DefinitionSource", manager.NamespaceIndexes[0]), + new QualifiedName("DefinitionSource", manager.NamespaceIndexes[0])) + .AddState(1, "Off", isInitial: true) + .AddState(2, "On") + .StateMachine + .Definition; + var machine = new FluentFiniteStateMachineState(null, definition) + { + BrowseName = new QualifiedName("RuntimeMachine", manager.NamespaceIndexes[0]), + DisplayName = new LocalizedText("RuntimeMachine") + }; + + await manager + .AddPredefinedNodeAsync(manager.SystemContext, machine) + .ConfigureAwait(false); + + var children = new List(); + machine.GetChildren(manager.SystemContext, children); + BaseInstanceState off = children.Single(child => child.BrowseName.Name == "Off"); + + Assert.That(machine.NodeId.IsNull, Is.False); + Assert.That(off.NodeId.IsNull, Is.False); + Assert.That( + off.NodeId.IdentifierAsString, + Is.EqualTo($"{machine.NodeId}_Off")); + Assert.That(machine.GetStateNodeId(1), Is.EqualTo(off.NodeId)); + Assert.That(manager.Find(off.NodeId), Is.SameAs(off)); + } + + [Test] + public void SynchronousPredefinedNodeRegistrationCompletesCreateLifecycle() + { + using ITestNodeManager manager = CreateManager(); + Assume.That(manager is TestableAsyncCustomNodeManager, + "Requires AsyncCustomNodeManager synchronous registration"); + var asyncManager = (TestableAsyncCustomNodeManager)manager; + ushort namespaceIndex = manager.NamespaceIndexes[0]; + var node = new LifecycleProbeState(null) + { + NodeId = new NodeId("SynchronousLifecycle", namespaceIndex), + BrowseName = new QualifiedName("SynchronousLifecycle", namespaceIndex), + DisplayName = new LocalizedText("SynchronousLifecycle") + }; + + asyncManager.AddPredefinedNodeSynchronouslyPublic(node); + + Assert.That(node.IsCreated, Is.True); + Assert.That(node.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(node.AfterCreateCount, Is.EqualTo(1)); + } + [Test] public async Task FindPredefinedNode_ReturnsNodeOnlyWhenTypeMatchesAsync() { @@ -4868,6 +5092,50 @@ namespace Opc.Ua.Server.Tests disposable.Dispose(); } } + + private sealed class LifecycleProbeState : BaseObjectState + { + public LifecycleProbeState(NodeState parent) + : base(parent) + { + } + + public int BeforeCreateCount { get; private set; } + + public int AfterCreateCount { get; private set; } + + public NodeId NodeIdAtAfterCreate { get; private set; } + + protected override void OnBeforeCreate(ISystemContext context, NodeState node) + { + BeforeCreateCount++; + base.OnBeforeCreate(context, node); + } + + protected override void OnAfterCreate( + ISystemContext context, + NodeState node, + CancellationToken ct = default) + { + AfterCreateCount++; + NodeIdAtAfterCreate = NodeId; + base.OnAfterCreate(context, node, ct); + } + } + + private sealed class NamespaceFiniteStateMachineState : FiniteStateMachineState + { + public const string ElementsNamespaceUri = + "http://test.org/UA/FiniteStateMachineElements/"; + + public NamespaceFiniteStateMachineState(NodeState parent) + : base(parent) + { + } + + protected override string ElementNamespaceUri => + ElementsNamespaceUri; + } } public sealed class TestableAsyncCustomNodeManager : AsyncCustomNodeManager, ITestNodeManager @@ -4907,6 +5175,8 @@ namespace Opc.Ua.Server.Tests public Func NodeRemovedCallback { get; set; } = null!; + public Func AddBehaviourCallback { get; set; } = null!; + public Func IsNodeInViewOverride { get; set; } protected override bool IsNodeInView(ServerSystemContext context, NodeId viewId, NodeState node) @@ -4936,6 +5206,19 @@ namespace Opc.Ua.Server.Tests return NodeRemovedCallback?.Invoke(node, cancellationToken) ?? default; } + protected override ValueTask AddBehaviourToPredefinedNodeAsync( + ISystemContext context, + NodeState predefinedNode, + CancellationToken cancellationToken = default) + { + return AddBehaviourCallback != null + ? new ValueTask(AddBehaviourCallback(predefinedNode)) + : base.AddBehaviourToPredefinedNodeAsync( + context, + predefinedNode, + cancellationToken); + } + public ValueTask AddPredefinedNodeWithExternalReferencesAsync( ISystemContext context, NodeState node, @@ -5039,6 +5322,11 @@ namespace Opc.Ua.Server.Tests return AddPredefinedNodeAsync(context, node, cancellationToken); } + public void AddPredefinedNodeSynchronouslyPublic(NodeState node) + { + AddPredefinedNodeSynchronously(node); + } + ValueTask ITestNodeManager.AddPredefinedNodeAsync( ISystemContext context, NodeState node, @@ -5273,6 +5561,8 @@ namespace Opc.Ua.Server.Tests /// Func? IsNodeInViewOverride { get; set; } + Func? AddBehaviourCallback { get; set; } + /// /// Tests whether a NodeId belongs to a managed namespace. /// @@ -5398,12 +5688,22 @@ namespace Opc.Ua.Server.Tests public Func? IsNodeInViewOverride { get; set; } + public Func? AddBehaviourCallback { get; set; } + protected override bool IsNodeInView(ServerSystemContext context, NodeId viewId, NodeState node) { return IsNodeInViewOverride?.Invoke(context, viewId, node) ?? base.IsNodeInView(context, viewId, node); } + protected override NodeState AddBehaviourToPredefinedNode( + ISystemContext context, + NodeState predefinedNode) + { + return AddBehaviourCallback?.Invoke(predefinedNode) + ?? base.AddBehaviourToPredefinedNode(context, predefinedNode); + } + public void AddPredefinedNodePublic(ISystemContext context, NodeState node) { AddPredefinedNode(context, node); @@ -5575,6 +5875,12 @@ namespace Opc.Ua.Server.Tests set => m_cnm2.IsNodeInViewOverride = value; } + public Func? AddBehaviourCallback + { + get => m_cnm2.AddBehaviourCallback; + set => m_cnm2.AddBehaviourCallback = value; + } + public bool IsNodeIdInNamespacePublic(NodeId nodeId) { return m_cnm2.IsNodeIdInNamespacePublic(nodeId); diff --git a/tests/Opc.Ua.Types.Tests/State/NodeStateTests.cs b/tests/Opc.Ua.Types.Tests/State/NodeStateTests.cs index 63d405ee37..eee9dabe79 100644 --- a/tests/Opc.Ua.Types.Tests/State/NodeStateTests.cs +++ b/tests/Opc.Ua.Types.Tests/State/NodeStateTests.cs @@ -432,7 +432,8 @@ namespace Opc.Ua.Types.Tests.State } [Test] - public void AddChildSetsParentAndAddsToChildren() { + public void AddChildSetsParentAndAddsToChildren() + { BaseObjectState parent = CreateObjectNode(); PropertyState child = CreatePropertyChild(parent, "Child1"); parent.AddChild(child); @@ -2652,15 +2653,105 @@ namespace Opc.Ua.Types.Tests.State } [Test] - public void CreateAsPredefinedNodeSucceeds() + public void CreateAsPredefinedNodeCompletesLifecycleOnce() { - var node = new BaseObjectState(null) - { - NodeId = new NodeId(100, 0), - BrowseName = QualifiedName.From("Predefined"), - DisplayName = LocalizedText.From("Predefined") - }; - Assert.DoesNotThrow(() => node.CreateAsPredefinedNode(m_context)); + var node = new LifecycleProbeState(null); + + Assert.That(node.IsCreated, Is.False); + + node.CreateAsPredefinedNode(m_context); + node.CreateAsPredefinedNode(m_context); + + Assert.That(node.IsCreated, Is.True); + Assert.That(node.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(node.AfterCreateCount, Is.EqualTo(1)); + } + + [Test] + public void CreateAsPredefinedNodeCompletesLateChild() + { + var parent = new LifecycleProbeState(null); + parent.CreateAsPredefinedNode(m_context); + + var child = new LifecycleProbeState(parent); + parent.AddChild(child); + + parent.CreateAsPredefinedNode(m_context); + + Assert.That(parent.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(parent.AfterCreateCount, Is.EqualTo(1)); + Assert.That(child.IsCreated, Is.True); + Assert.That(child.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(child.AfterCreateCount, Is.EqualTo(1)); + } + + [Test] + public void CreateAsPredefinedNodeCompletesChildAddedByOnAfterCreate() + { + var parent = new ChildCreatingState(null); + + parent.CreateAsPredefinedNode(m_context); + + Assert.That(parent.AddedChild, Is.Not.Null); + Assert.That(parent.AddedChild.IsCreated, Is.True); + Assert.That(parent.AddedChild.BeforeCreateCount, Is.EqualTo(1)); + Assert.That(parent.AddedChild.AfterCreateCount, Is.EqualTo(1)); + } + + [Test] + public void CreateAsPredefinedNodeRejectsNonConvergingLifecycle() + { + var node = new NonConvergingState(null); + + Assert.That( + () => node.CreateAsPredefinedNode(m_context), + Throws.TypeOf() + .With.Message.Contains("did not converge")); + } + + [Test] + public void DeleteResetsCreatedState() + { + var node = new LifecycleProbeState(null); + node.CreateAsPredefinedNode(m_context); + + node.Delete(m_context); + + Assert.That(node.IsCreated, Is.False); + } + + [Test] + public void CloneDoesNotCopyCreatedState() + { + BaseObjectState node = CreateObjectNode(); + node.CreateAsPredefinedNode(m_context); + + var clone = (BaseObjectState)node.Clone(); + + Assert.That(node.IsCreated, Is.True); + Assert.That(clone.IsCreated, Is.False); + } + + [Test] + public void CreateAlwaysRunsLifecycle() + { + var node = new LifecycleProbeState(null); + + node.Create( + m_context, + NodeId.Null, + QualifiedName.Null, + LocalizedText.Null, + false); + node.Create( + m_context, + NodeId.Null, + QualifiedName.Null, + LocalizedText.Null, + false); + + Assert.That(node.BeforeCreateCount, Is.EqualTo(2)); + Assert.That(node.AfterCreateCount, Is.EqualTo(2)); } [Test] @@ -2712,5 +2803,72 @@ namespace Opc.Ua.Types.Tests.State return new NodeId((uint)(90000 + Count), 0); } } + + private sealed class LifecycleProbeState : BaseObjectState + { + public LifecycleProbeState(NodeState parent) + : base(parent) + { + } + + public int BeforeCreateCount { get; private set; } + + public int AfterCreateCount { get; private set; } + + protected override void OnBeforeCreate(ISystemContext context, NodeState node) + { + BeforeCreateCount++; + base.OnBeforeCreate(context, node); + } + + protected override void OnAfterCreate( + ISystemContext context, + NodeState node, + System.Threading.CancellationToken ct = default) + { + AfterCreateCount++; + base.OnAfterCreate(context, node, ct); + } + } + + private sealed class ChildCreatingState : BaseObjectState + { + public ChildCreatingState(NodeState parent) + : base(parent) + { + } + + public LifecycleProbeState AddedChild { get; private set; } = null!; + + protected override void OnAfterCreate( + ISystemContext context, + NodeState node, + System.Threading.CancellationToken ct = default) + { + base.OnAfterCreate(context, node, ct); + if (AddedChild == null) + { + AddedChild = new LifecycleProbeState(this); + AddChild(AddedChild); + } + } + } + + private sealed class NonConvergingState : BaseObjectState + { + public NonConvergingState(NodeState parent) + : base(parent) + { + } + + protected override void OnAfterCreate( + ISystemContext context, + NodeState node, + System.Threading.CancellationToken ct = default) + { + base.OnAfterCreate(context, node, ct); + AddChild(new NonConvergingState(this)); + } + } } } diff --git a/tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateTemplates.cs b/tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateTemplates.cs index 5659244d55..ee5fb9f8b8 100644 --- a/tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateTemplates.cs +++ b/tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateTemplates.cs @@ -1788,6 +1788,13 @@ namespace Opc.Ua.SourceGeneration /// /// Creates an instance of the {{Tokens.SymbolicName}} Method node state. /// + /// + /// The returned node graph has not completed its create lifecycle. + /// Node manager registration completes it automatically. Call + /// + /// before registration when configuration depends on + /// OnBeforeCreate or OnAfterCreate. + /// public static {{Tokens.StateClassName}} CreateInstanceOf{{Tokens.SymbolicId}}( this global::Opc.Ua.ISystemContext context, global::Opc.Ua.NodeState parent = null, @@ -1893,6 +1900,13 @@ namespace Opc.Ua.SourceGeneration /// /// Creates an instance of the {{Tokens.TypeName}} type. /// + /// + /// The returned node graph has not completed its create lifecycle. + /// Node manager registration completes it automatically. Call + /// + /// before registration when configuration depends on + /// OnBeforeCreate or OnAfterCreate. + /// public static {{Tokens.StateClassName}} Create{{Tokens.SymbolicId}}( this global::Opc.Ua.ISystemContext context, global::Opc.Ua.NodeState parent = null, @@ -1951,6 +1965,13 @@ namespace Opc.Ua.SourceGeneration /// /// Creates an instance of the {{Tokens.TypeName}} variable type. /// + /// + /// The returned node graph has not completed its create lifecycle. + /// Node manager registration completes it automatically. Call + /// + /// before registration when configuration depends on + /// OnBeforeCreate or OnAfterCreate. + /// public static {{Tokens.StateClassName}} Create{{Tokens.SymbolicId}}( this global::Opc.Ua.ISystemContext context, global::Opc.Ua.NodeState parent = null,