From a568a7e54cd938f4e85ea12e9ab5db0a90542286 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alexander=20K=C3=B6pke?= Date: Tue, 21 Jul 2026 20:54:19 +0200 Subject: [PATCH] Set ContinuationPoint (when browse is finished) to `null` instead of empty array, align with existing 1.5.xxx behavior (#4057) This pull request standardizes the handling of the `ContinuationPoint` property in `BrowseResult` objects, ensuring it is always explicitly set to `null` (or its equivalent) when no continuation point is present. This change improves clarity and consistency in both the implementation and the tests, replacing checks for an "empty" continuation point with checks for a "null" continuation point. **Codebase consistency and correctness:** * All `BrowseResult` objects now explicitly set `ContinuationPoint = default` when no continuation point is to be returned, ensuring consistent initialization. (`src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs`) [[1]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L1900-R1904) [[2]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L2068-R2076) [[3]](diffhunk://#diff-851e5925c677925c42bd70bce11ea1a75a5f5cbdb810ac851351385e16f54a71L2078-R2090) **Test updates for new semantics:** * All test assertions now use `ContinuationPoint.IsNull` instead of `IsEmpty` to check for the absence of a continuation point, reflecting the new contract. (`tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs`, `tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs`) [[1]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L121-R122) [[2]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L394-R394) [[3]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L435-R435) [[4]](diffhunk://#diff-904275d6b4bee35e6884f38e0db5cbc955216eaf2aa7f468ba285133470e1596L634-R634) [[5]](diffhunk://#diff-bda1da5dc01533ffe61d1c65b6e968b6462e11a63758ac95ded000ffc885345eL420-R420) * Additional test cases have been added to verify that completed browse operations and error scenarios always return a null continuation point. (`tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs`) [[1]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR203-R230) [[2]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR327) [[3]](diffhunk://#diff-3d9efc81935ec60d7513334833e9eab033925194f5a0d11105668204cc60f54aR349)# Description _Describe the changes here to communicate to the maintainers why they should accept this pull request. By default - this will become the Commit message after merging and thus define history._ ## 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 #github-issue-number, ... ## 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. - [ ] 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. --------- Co-authored-by: Marc Schier Co-authored-by: Marc Schier Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../PumpDeviceIntegrationServer/Dockerfile | 12 +++++--- .../NodeManager/MasterNodeManager.cs | 18 +++++++++-- .../BrowseContinuationPointTests.cs | 10 +++---- .../BrowseTests.cs | 2 +- .../MasterNodeManagerDeterministicTests.cs | 30 +++++++++++++++++++ 5 files changed, 59 insertions(+), 13 deletions(-) diff --git a/samples/PumpDeviceIntegrationServer/Dockerfile b/samples/PumpDeviceIntegrationServer/Dockerfile index becd489807..90e3dd1bf6 100644 --- a/samples/PumpDeviceIntegrationServer/Dockerfile +++ b/samples/PumpDeviceIntegrationServer/Dockerfile @@ -78,10 +78,14 @@ COPY --from=build /app/publish . # The .NET base images ship a predefined non-root user `app` (UID 1654, # exposed via the APP_UID env var). The server creates its -# certificate/PKI store under /app at runtime, so make it writable by -# that user. -ENV HOME=/app -RUN mkdir -p /app/pki && chown -R $APP_UID:$APP_UID /app +# certificate/PKI store under /app at runtime. The shared /diag +# directory carries the diagnostic socket and collected artifacts for +# the companion Kubernetes diagnostics container. +ENV HOME=/app \ + DOTNET_DiagnosticPorts=/diag/dotnet-diagnostic.sock,suspend=n,listen +RUN mkdir -p /app/pki /diag \ + && chown -R $APP_UID:0 /app /diag \ + && chmod -R g=u /app /diag USER $APP_UID diff --git a/src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs b/src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs index 13b3fe683e..943f70d8ab 100644 --- a/src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs +++ b/src/Opc.Ua.Server/NodeManager/MasterNodeManager.cs @@ -1897,7 +1897,11 @@ namespace Opc.Ua.Server BrowseDescription nodeToBrowse = nodesToBrowse[ii]; // initialize result. - var result = new BrowseResult { StatusCode = StatusCodes.Good }; + var result = new BrowseResult + { + StatusCode = StatusCodes.Good, + ContinuationPoint = default + }; results.Add(result); ServiceResult error; @@ -2065,7 +2069,11 @@ namespace Opc.Ua.Server .ConfigureAwait(false); if (ServiceResult.IsBad(validationResult)) { - var badResult = new BrowseResult { StatusCode = validationResult.Code }; + var badResult = new BrowseResult + { + StatusCode = validationResult.Code, + ContinuationPoint = default + }; results.Add(badResult); // put placeholder for diagnostics @@ -2075,7 +2083,11 @@ namespace Opc.Ua.Server } // initialize result. - var result = new BrowseResult { StatusCode = StatusCodes.Good }; + var result = new BrowseResult + { + StatusCode = StatusCodes.Good, + ContinuationPoint = default + }; results.Add(result); // check if simply releasing the continuation point. diff --git a/tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs b/tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs index d81d0d8c4f..b28908bdc6 100644 --- a/tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs +++ b/tests/Opc.Ua.InformationModel.Tests/BrowseContinuationPointTests.cs @@ -118,8 +118,8 @@ namespace Opc.Ua.InformationModel.Tests cp = nextResp.Results[0].ContinuationPoint; } - Assert.That(cp.IsEmpty, Is.True, - "Continuation point should be empty after full traversal."); + Assert.That(cp.IsNull, Is.True, + "Continuation point should be null after full traversal."); Assert.That(totalRefs, Is.GreaterThan(1), "Server node should have more than one reference."); } @@ -391,7 +391,7 @@ namespace Opc.Ua.InformationModel.Tests Assert.That(response.Results.Count, Is.EqualTo(1)); Assert.That( StatusCode.IsGood(response.Results[0].StatusCode), Is.True); - Assert.That(response.Results[0].ContinuationPoint.IsEmpty, Is.True, + Assert.That(response.Results[0].ContinuationPoint.IsNull, Is.True, "Views folder has few children; no continuation point " + "should be returned with MaxRefs=100."); } @@ -432,7 +432,7 @@ namespace Opc.Ua.InformationModel.Tests Is.True, "Release should return Good status."); Assert.That( - releaseResp.Results[0].ContinuationPoint.IsEmpty, Is.True, + releaseResp.Results[0].ContinuationPoint.IsNull, Is.True, "No continuation point should remain after release."); } @@ -631,7 +631,7 @@ namespace Opc.Ua.InformationModel.Tests Assert.That(response.Results[0].References.Count, Is.GreaterThan(0), "Server node should have at least one reference."); - Assert.That(response.Results[0].ContinuationPoint.IsEmpty, Is.True, + Assert.That(response.Results[0].ContinuationPoint.IsNull, Is.True, "MaxReferencesPerNode=0 should return all references " + "without a continuation point."); } diff --git a/tests/Opc.Ua.InformationModel.Tests/BrowseTests.cs b/tests/Opc.Ua.InformationModel.Tests/BrowseTests.cs index 505d8e9c26..57f989b10a 100644 --- a/tests/Opc.Ua.InformationModel.Tests/BrowseTests.cs +++ b/tests/Opc.Ua.InformationModel.Tests/BrowseTests.cs @@ -417,7 +417,7 @@ namespace Opc.Ua.InformationModel.Tests Assert.That(releaseResponse.Results.Count, Is.EqualTo(1)); Assert.That(StatusCode.IsGood(releaseResponse.Results[0].StatusCode), Is.True, "Releasing a continuation point should return Good."); - Assert.That(releaseResponse.Results[0].ContinuationPoint.IsEmpty, Is.True, + Assert.That(releaseResponse.Results[0].ContinuationPoint.IsNull, Is.True, "No continuation point should remain after release."); } diff --git a/tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs b/tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs index 66adc086cf..1924b8a279 100644 --- a/tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs +++ b/tests/Opc.Ua.Server.Tests/NodeManager/MasterNodeManagerDeterministicTests.cs @@ -200,6 +200,34 @@ namespace Opc.Ua.Server.Tests Assert.That(results.Count, Is.EqualTo(1)); Assert.That(results[0].StatusCode, Is.EqualTo(StatusCodes.BadNodeIdUnknown)); + Assert.That(results[0].ContinuationPoint.IsNull, Is.True); + } + + [Test] + public async Task BrowseAsyncCompletedBrowseReturnsNullContinuationPointAsync() + { + IMasterNodeManager sut = m_server.CurrentInstance.NodeManager; + OperationContext ctx = CreateContext(); + + var nodeToBrowse = new BrowseDescription + { + NodeId = ObjectIds.ViewsFolder, + BrowseDirection = BrowseDirection.Forward, + ReferenceTypeId = ReferenceTypeIds.HierarchicalReferences, + IncludeSubtypes = true, + ResultMask = (uint)BrowseResultMask.All + }; + + (ArrayOf results, _) = await sut.BrowseAsync( + ctx, + new ViewDescription(), + 0u, + new BrowseDescription[] { nodeToBrowse }.ToArrayOf(), + CancellationToken.None).ConfigureAwait(false); + + Assert.That(results.Count, Is.EqualTo(1)); + Assert.That(results[0].StatusCode, Is.EqualTo(StatusCodes.Good)); + Assert.That(results[0].ContinuationPoint.IsNull, Is.True); } [Test] @@ -330,6 +358,7 @@ namespace Opc.Ua.Server.Tests Assert.That(results.Count, Is.EqualTo(1)); Assert.That(results[0].StatusCode, Is.EqualTo(StatusCodes.BadContinuationPointInvalid)); + Assert.That(results[0].ContinuationPoint.IsNull, Is.True); } [Test] @@ -351,6 +380,7 @@ namespace Opc.Ua.Server.Tests Assert.That(results.Count, Is.EqualTo(1)); Assert.That(results[0].StatusCode, Is.EqualTo(StatusCodes.Good)); + Assert.That(results[0].ContinuationPoint.IsNull, Is.True); } [Test]