diff --git a/docs/ModelChangeTracking.md b/docs/ModelChangeTracking.md index ebc3d1ea1e..6f92c69775 100644 --- a/docs/ModelChangeTracking.md +++ b/docs/ModelChangeTracking.md @@ -122,7 +122,10 @@ is cancelled. A versioned descendant is reported even if its root is unversioned The deletion error is still returned; notification failures are not suppressed. For reference endpoints owned by another node manager, automatic reporting requires requested and successful target-reference cleanup because their remaining -references cannot be inspected locally. A successful `DeleteNodes` call with +references cannot be inspected locally. A bad per-reference cleanup result from +an owning node manager is propagated as a deletion failure, not treated as +successful cleanup. Committed local changes are still reported, but unconfirmed +foreign reference removals are not. A successful `DeleteNodes` call with `DeleteTargetReferences = false` does not imply that foreign references were removed. This reporting covers both directions of hierarchical and non-hierarchical references to other node managers in the same server. Absolute targets are diff --git a/src/Opc.Ua.Server/NodeManager/IMasterNodeManager.cs b/src/Opc.Ua.Server/NodeManager/IMasterNodeManager.cs index 85e64a7e51..ad7087ce64 100644 --- a/src/Opc.Ua.Server/NodeManager/IMasterNodeManager.cs +++ b/src/Opc.Ua.Server/NodeManager/IMasterNodeManager.cs @@ -311,6 +311,9 @@ namespace Opc.Ua.Server /// /// Deletes the specified references. /// + /// + /// An owning node manager returned a bad result while deleting a reference. + /// ValueTask RemoveReferencesAsync(List referencesToRemove, CancellationToken cancellationToken = default); /// diff --git a/src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs b/src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs index 5bdd17732b..54874f0e8a 100644 --- a/src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs +++ b/src/Opc.Ua.Server/NodeManager/MasterNodeManager.NodeManagement.cs @@ -140,7 +140,7 @@ namespace Opc.Ua.Server } // delete the reference. - await nodeManager!.DeleteReferenceAsync( + ServiceResult result = await nodeManager!.DeleteReferenceAsync( sourceHandle, reference.ReferenceTypeId, reference.IsInverse, @@ -148,6 +148,10 @@ namespace Opc.Ua.Server false, cancellationToken) .ConfigureAwait(false); + if (ServiceResult.IsBad(result)) + { + throw new ServiceResultException(result); + } } } diff --git a/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs b/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs index 44d38eac78..3d29c015a1 100644 --- a/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs +++ b/tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs @@ -1305,6 +1305,106 @@ namespace Opc.Ua.Server.Tests Assert.That(changes[0].Verb, Is.EqualTo((byte)Opc.Ua.Server.NodeManager.ModelChangeVerbs.NodeDeleted)); } + /// + /// Foreign cleanup status failures propagate without announcing retained references. + /// + [TestCase(false, 0)] + [TestCase(false, 1)] + [TestCase(false, 2)] + [TestCase(true, 0)] + [TestCase(true, 1)] + [TestCase(true, 2)] + public async Task DeleteNodeChecksForeignReferenceCleanupStatusAsync(bool service, int statusIndex) + { + StatusCode cleanupStatus = statusIndex switch + { + 0 => StatusCodes.Good, + 1 => StatusCodes.BadNotSupported, + _ => StatusCodes.BadUnexpectedError + }; + using ITestNodeManager manager = CreateManager(); + Assume.That(!service || manager is TestableAsyncCustomNodeManager, "Requires node-management services."); + var node = new BaseObjectState(null); + manager.EnableModelChangeTrackingFor(node); + NodeId id = await manager.CreateNodeAsync( + manager.SystemContext, default, ReferenceTypeIds.Organizes, + new QualifiedName("ForeignCleanupStatus", manager.NamespaceIndexes[0]), node).ConfigureAwait(false); + const string ForeignNamespaceUri = "urn:test:foreign-cleanup-status"; + var foreignTarget = new BaseObjectState(null) + { + NodeId = new NodeId("ForeignTarget", m_namespaceTable.GetIndexOrAppend(ForeignNamespaceUri)) + }; + node.AddReference(ReferenceTypeIds.HasCause, false, foreignTarget.NodeId); + foreignTarget.AddReference(ReferenceTypeIds.HasCause, true, id); + var foreignManager = new Mock(); + foreignManager.Setup(value => value.NamespaceUris).Returns([ForeignNamespaceUri]); + foreignManager.Setup(value => value.GetManagerHandleAsync( + foreignTarget.NodeId, It.IsAny())) + .Returns(new ValueTask(foreignTarget)); + foreignManager.Setup(value => value.DeleteReferenceAsync( + foreignTarget, ReferenceTypeIds.HasCause, true, id, false, It.IsAny())) + .Returns(() => + { + if (StatusCode.IsGood(cleanupStatus)) + { + foreignTarget.RemoveReference(ReferenceTypeIds.HasCause, true, id); + } + return new ValueTask(new ServiceResult(cleanupStatus)); + }); + var factory = new Mock(); + factory.Setup(value => value.CreateConfigurationNodeManager()) + .Returns(new Mock().Object); + factory.Setup(value => value.CreateCoreNodeManager(It.IsAny())) + .Returns(new Mock().Object); + m_mockServer.Setup(value => value.MainNodeManagerFactory).Returns(factory.Object); + using var master = new MasterNodeManager( + m_mockServer.Object, m_configuration, null, [foreignManager.Object]); + m_mockServer.Setup(value => value.NodeManager).Returns(master); + m_mockServer.Invocations.Clear(); + + if (StatusCode.IsGood(cleanupStatus)) + { + await DeleteTestNodeAsync(manager, id, service).ConfigureAwait(false); + } + else + { + await Assert.ThatAsync( + async () => await DeleteTestNodeAsync(manager, id, service).ConfigureAwait(false), + Throws.TypeOf() + .With.Property(nameof(ServiceResultException.StatusCode)).EqualTo(cleanupStatus)) + .ConfigureAwait(false); + } + + Assert.That(manager.Find(id), Is.Null); + bool cleanupSucceeded = StatusCode.IsGood(cleanupStatus); + Assert.That(foreignTarget.ReferenceExists(ReferenceTypeIds.HasCause, true, id), + Is.EqualTo(!cleanupSucceeded)); + foreignManager.Verify(value => value.DeleteReferenceAsync( + foreignTarget, ReferenceTypeIds.HasCause, true, id, false, It.IsAny()), + Times.Once()); + ArrayOf changes = GetReportedDeletionChanges(); + Assert.That(changes.Count, Is.EqualTo(cleanupSucceeded ? 2 : 1)); + bool deletionReported = false; + bool foreignReferenceReported = false; + foreach (ModelChangeStructureDataType change in changes) + { + if (change.Affected == id) + { + deletionReported = true; + Assert.That(change.Verb, + Is.EqualTo((byte)Opc.Ua.Server.NodeManager.ModelChangeVerbs.NodeDeleted)); + } + if (change.Affected == foreignTarget.NodeId) + { + foreignReferenceReported = true; + Assert.That(change.Verb, + Is.EqualTo((byte)Opc.Ua.Server.NodeManager.ModelChangeVerbs.ReferenceDeleted)); + } + } + Assert.That(deletionReported, Is.True); + Assert.That(foreignReferenceReported, Is.EqualTo(cleanupSucceeded)); + } + /// /// Internal and legacy deletion report foreign non-hierarchical endpoints after successful cleanup. ///