Preserve NodeVersion eligibility when deleting runtime nodes (#4777)

# Description Preserve a runtime node's NodeVersion eligibility before removing it from the address space. Previously, deletion removed the local node before model-change filtering, which then treated it as an unresolved external node and emitted a spurious NodeDeleted entry. This fixes unversioned runtime/session-diagnostics deletion in both AsyncCustomNodeManager and the legacy CustomNodeManager. Strict mode suppresses unversioned deletion entries; versioned nodes and the explicit compatibility mode keep their events. The regression matrix exercises all four eligibility combinations across both async configurations and the legacy adapter. The documentation now cites OPC UA Part 3 section 9.32.2, which requires NodeVersion/model-change correlation. This PR is intentionally limited to the original eligibility bug. Per-node monitoring completion and outcome-based model-change completion are separate changes tracked in #4768 and #4769, including broader subtree/service-deletion handling. ## Related Issues _Reference all GitHub issues this PR addresses. If there is no issue yet, open one and link it here._ _If this is a relatively large or complex change, a design must have been discussed in the related tracking issue and signed off (which becomes the Architectural Decision Record (ADR))._ - Fixes #4770 - Related: #4768, #4769 ## Checklist _Put an `x` in the boxes that apply. You can complete these step by step after opening the PR._ - [x] 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. - [ ] I have addressed **all** PR feedback received. ## Validation The server test project and its dependencies built successfully on net10.0 and net48 with no new build/analyzer warnings. Focused reruns: ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net10.0 -p:CustomTestTarget=net10.0 --filter "Name~DeleteNodePreservesModelChangeEligibilityAsync" ``` Result: **12 passed, 0 failed**. ```powershell dotnet test tests\Opc.Ua.Server.Tests\Opc.Ua.Server.Tests.csproj --no-build --no-restore -f net48 -p:CustomTestTarget=net48 --filter "Name~DeleteNodePreservesModelChangeEligibilityAsync" ``` Result: **12 passed, 0 failed**. Full-solution .NET 10 validation was attempted, but the unchanged AOT test projects reject the VSTest target with Microsoft.Testing.Platform 2.4.1 under SDK 10.0.401. The run was stopped after this blocker was confirmed. It is not reported as a passing suite. Exact isolated reproduction of the blocker: ```powershell dotnet test tests\Opc.Ua.Aot.Tests\Opc.Ua.Aot.Tests.csproj --no-build --no-restore -p:CustomTestTarget=net10.0 -p:NuGetAudit=false ``` Result: **failed** with "Testing with VSTest target is no longer supported by Microsoft.Testing.Platform on .NET 10 SDK and later." The full net48 solution was not run. Restore/build commands used `NuGetAudit=false` because NuGet's audit endpoint was unavailable; repository audit settings are unchanged. Published as a draft by request until the full validation/CI gates are satisfied. No release-version files are included; this branch starts from master. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
2026-10-16 21:02:51 +08:00 · 2026-10-14 08:44:23 +02:00
parent f1e51c19f1
commit 27c17fb410
4 changed files with 54 additions and 3 deletions
@@ -353,9 +353,15 @@ class MyInvalidatingCache : INodeCache

 ## NodeVersion correlation

-OPC UA Part 5 §9.32.2 ties `ModelChangeEvent` emission to the
+OPC UA Part 3 §9.32.2 ties `ModelChangeEvent` emission to the
 `NodeVersion` property:

+Automatic deletion captures a node's `NodeVersion` eligibility before removing
+it from the address space. A removed local node must not bypass this check as an
+unresolved external node. Unversioned session diagnostics therefore do not emit
+`NodeDeleted` events on session closure; versioned nodes and the explicit
+`RequireNodeVersionForModelChange = false` compatibility mode retain their events.
+
 > "Every time a `ModelChangeEvent` is issued for a Node, its
 > `NodeVersion` shall be changed, and every time the `NodeVersion` is
 > changed, a `ModelChangeEvent` shall be generated. A Server shall
@@ -1887,6 +1887,8 @@ namespace Opc.Ua.Server
            }

            NodeId? deletedTypeDefinition = (node as BaseInstanceState)?.TypeDefinitionId;
+            // The event filter cannot inspect this node after it has been removed.
+            bool emitNodeDeleted = !RequireNodeVersionForModelChange || node.HasNodeVersion();
            NodeId parentId = (node as BaseInstanceState)?.Parent?.NodeId ?? default;
            NodeId modelChangeParentId = GetModelChangeParentNodeId(contextToUse, node);
            List<NodeState> removedNotifiers = GetSubtreeNotifiers(contextToUse, node);
@@ -1932,7 +1934,10 @@ namespace Opc.Ua.Server

            if (ModelChangeEmissionEnabled)
            {
-                ModelChangeAggregator.RecordNodeDeleted(nodeId, deletedTypeDefinition);
+                if (emitNodeDeleted)
+                {
+                    ModelChangeAggregator.RecordNodeDeleted(nodeId, deletedTypeDefinition);
+                }
                if (!modelChangeParentId.IsNull)
                {
                    ModelChangeAggregator.RecordReferenceDeleted(modelChangeParentId);
@@ -969,6 +969,8 @@ namespace Opc.Ua.Server
            }

            NodeId? deletedTypeDefinition = (node as BaseInstanceState)?.TypeDefinitionId;
+            // The event filter cannot inspect this node after it has been removed.
+            bool emitNodeDeleted = !RequireNodeVersionForModelChange || node.HasNodeVersion();
            IReadOnlyList<IMonitoredItem> monitoredItems =
                GetMonitoredItemsForNodeSubtree(contextToUse, node);
            var detachedItems = new List<IMonitoredItem>(monitoredItems.Count);
@@ -1006,7 +1008,7 @@ namespace Opc.Ua.Server
                ((IDetachableMonitoredItem)monitoredItem).MarkNodeDeleted();
            }

-            if (ModelChangeEmissionEnabled)
+            if (ModelChangeEmissionEnabled && emitNodeDeleted)
            {
                ModelChangeAggregator.RecordNodeDeleted(nodeId, deletedTypeDefinition);
                EmitModelChange(contextToUse);
@@ -1147,6 +1147,44 @@ namespace Opc.Ua.Server.Tests
                Times.Once);
        }

+        /// <summary>
+        /// Deletion preserves the node's pre-removal model-change eligibility.
+        /// </summary>
+        [TestCase(false, true, false)]
+        [TestCase(true, true, true)]
+        [TestCase(false, false, true)]
+        [TestCase(true, false, true)]
+        public async Task DeleteNodePreservesModelChangeEligibilityAsync(
+            bool hasNodeVersion,
+            bool requireNodeVersion,
+            bool expectEvent)
+        {
+            using ITestNodeManager manager = CreateManager();
+            manager.RequireNodeVersionForModelChange = requireNodeVersion;
+            var node = new BaseObjectState(null);
+            if (hasNodeVersion)
+            {
+                manager.EnableModelChangeTrackingFor(node);
+            }
+
+            NodeId nodeId = await manager.CreateNodeAsync(
+                manager.SystemContext,
+                default,
+                ReferenceTypeIds.Organizes,
+                new QualifiedName("DeletedObject", manager.NamespaceIndexes[0]),
+                node).ConfigureAwait(false);
+            m_mockServer.Invocations.Clear();
+
+            bool deleted = await manager.DeleteNodeAsync(
+                manager.SystemContext,
+                nodeId).ConfigureAwait(false);
+
+            Assert.That(deleted, Is.True);
+            m_mockServer.Verify(
+                server => server.ReportEvent(It.Is<IFilterTarget>(target => target is GeneralModelChangeEventState)),
+                expectEvent ? Times.Once() : Times.Never());
+        }
+
        /// <summary>
        /// An external write to NodeVersion fires a BaseModelChangeEvent
        /// (Part 5 §9.32.5). The framework-driven bump from