Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) by romanett · Pull Request #4331 · OPCFoundation/UA-.NETStandard

romanett · 2026-08-28T17:06:11Z

Description

A node created inside the fluent Configure hook (source-generated [NodeManager] partial, or the hosting AddNodeManager(uri, build) route) had no way to appear below the Objects folder — or below any node owned by another node manager. AsyncCustomNodeManager.AddReverseReferencesAsync mirrors inverse references into the externalReferences dictionary before the Configure partials run, and the fluent surface never sees the dictionary, so a configure-created node with an inverse Organizes reference to ObjectsFolder existed and simulated but never showed up under Objects.

This implements the three coordinated pieces proposed in #4329, plus the companion namespace gap:

1. Idempotent mirroring pass (independently worthwhile hardening)

AddExternalReference in AsyncCustomNodeManager and CustomNodeManager2 now has if-missing semantics (linear scan of the per-source list).
TypeTable.AddEncoding no longer appends a duplicate encoding id; re-registering the same encoding is a succeeding no-op. (AddRootNotifierAsync and the node-side AddReferenceIfMissing branch were already idempotent.)

2. Re-run the pass after Configure

New protected FluentNodeManagerBase.CompleteConfigureAsync(externalReferences, ct) wraps AddReverseReferencesAsync so hand-written managers using CreateFluentBuilder benefit too.
The generator-emitted CreateAddressSpaceAsync and the hosting FluentNodeManager invoke it once between the Configure callbacks and Seal(). Timing is safe: the master distributes externalReferences only after every manager's CreateAddressSpaceAsync returns. Side benefit: inverse HasNotifier references on configure-created event sources get root-notifier registration for free.
The hosting FluentNodeManager's hand-rolled externalReferences workaround for its root folder is deleted — the root's inverse Organizes reference now flows through the shared pass (mirrored exactly once, covered by a test).

3. Discoverable fluent sugar

OrganizedBy(parentId) / UnderObjectsFolder() on INodeBuilder write the inverse Organizes reference; with piece 2, placement is just "write the inverse reference".
A parentless CreateInstance<TState> on INodeManagerBuilder takes a constructor-style factory (p => new BoilerState(p), keeping the surface reflection-free and AOT-safe), materializes the subtree from the type model via NodeState.Create, rebases all NodeIds through the manager's INodeIdFactory (AssignInstanceNodeId/AssignInstanceChildNodeIds — the same pair the generated factories use, so declaration-id children cannot collide with the type model), and registers via AddPredefinedNodeSynchronously. The Boiler Initial commit. #2 shape becomes fully fluent:

builder.CreateInstance(
        new QualifiedName("Boiler #2", NamespaceIndexes[1]),
        p => new BoilerState(p))
    .Configure(n => n.UnderObjectsFolder());

Companion gap, same theme: [NodeManager] grows AdditionalNamespaceUris, flowing through attribute discovery → binding → generator so the generated constructor reports a second (instance) namespace at construction and the generated factory advertises it in NamespacesUris — today SetNamespaces after construction is not enough because MasterNodeManager builds its namespace routing from what the manager reported when it was built.

Scope boundary (as discussed in the issue): startup-time configuration only; for nodes created after startup the correct primitive remains IMasterNodeManager.AddReferencesAsync.

Reviewer notes

The generated-code change was validated against the real pipeline: a forced fresh compile of Opc.Ua.Server.Tests (whose CoverageNodeSet managers are [NodeManager]-generated) emits await CompleteConfigureAsync(externalReferences, cancellationToken) between Configure and Seal() and compiles against the real base class.
The distribution consumer (AddReferencesAsync) already used AddReferenceIfMissing, so the whole chain is defense-in-depth idempotent.
15 new tests: double-run idempotence, second-pass pickup of late-registered nodes, the new reference sugar, root-instance creation incl. child-id rebasing and reference remapping, hosting end-to-end Objects-folder placement, emitted-sequence and AdditionalNamespaceUris generator tests, and a full Roslyn-pipeline attribute test. Docs updated in docs/NodeManagers.md (new "Creating nodes under other managers' nodes" section, attribute docs, fluent-surface sections).
Verified locally: all touched projects build with 0 warnings; full Opc.Ua.Server.Tests (4,725 passed / 0 failed / 5 skipped, net9.0), TypeTableTests (113), Opc.Ua.SourceGeneration.Core.Tests (41), Opc.Ua.SourceGeneration.Tests (132), and the Quickstarts.Servers sample all pass.

Related Issues

Checklist

I have signed the CLA and read the CONTRIBUTING doc.
I have added tests that prove my fix is effective or that my feature works and increased code coverage.
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.

🤖 Generated with Claude Code

…ger nodes A node created inside the fluent Configure hook (source-generated [NodeManager] partial, or the hosting AddNodeManager build callback) had no way to appear below the Objects folder or any other node owned by another node manager: AddReverseReferencesAsync mirrors inverse references into the externalReferences dictionary before Configure runs, and the fluent surface never sees the dictionary. Implements the three coordinated pieces from #4329: - Make the mirroring pass idempotent: AddExternalReference (async and sync managers) gets if-missing semantics and TypeTable.AddEncoding no longer appends duplicate encodings, so the pass can safely run twice. - Re-run the pass after Configure: new protected FluentNodeManagerBase.CompleteConfigureAsync wraps AddReverseReferencesAsync; the generator-emitted CreateAddressSpaceAsync and the hosting FluentNodeManager invoke it between the Configure callbacks and Seal(). The hosting manager's hand-rolled externalReferences workaround for its root folder is replaced by the shared pass. - Discoverable fluent sugar: OrganizedBy(parentId) / UnderObjectsFolder() on INodeBuilder write the inverse Organizes reference, and a parentless CreateInstance<TState> on INodeManagerBuilder materializes a root-level instance from its type model, mints per-instance NodeIds through the manager's INodeIdFactory, and registers it with the manager. Companion gap, same theme: [NodeManager] grows AdditionalNamespaceUris so the generated constructor reports a second (instance) namespace at construction and the generated factory advertises it in NamespacesUris, removing the manual RegisterNamespaceManager workaround. Fixes #4329 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot

Pull request overview

This PR fixes a fluent NodeManager startup-time gap where nodes created during fluent Configure could not publish cross-manager references (e.g., appear under the ns=0 Objects folder), by re-running the reverse-reference mirroring pass after Configure and making that pass idempotent. It also extends [NodeManager] source generation to support additional namespace URIs that must be reported at construction time for correct namespace routing.

Changes:

Make external-reference mirroring idempotent (AddExternalReference and TypeTable.AddEncoding) so the reverse-reference pass can safely run multiple times.
Re-run reverse-reference collection after fluent Configure via FluentNodeManagerBase.CompleteConfigureAsync, wired into generated and hosting node managers.
Add fluent convenience APIs for Objects-folder placement (OrganizedBy / UnderObjectsFolder) and a manager-scoped root CreateInstance overload; extend [NodeManager] with AdditionalNamespaceUris and wire through generator + tests + docs.

Reviewed changes

Copilot reviewed 24 out of 24 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/Opc.Ua.SourceGeneration/NodeManagerAttributeDiscovery.cs	Discovers `AdditionalNamespaceUris` from `[NodeManager]` attributes.
tools/Opc.Ua.SourceGeneration/Extensions.cs	Adds `GetStringArray` helper for attribute named-argument parsing.
tools/Opc.Ua.SourceGeneration.Core/Templating/Tokens.cs	Adds template token for `AdditionalNamespaceUris`.
tools/Opc.Ua.SourceGeneration.Core/NodeManagerAttributeBinding.cs	Carries discovered `AdditionalNamespaceUris` in binding record.
tools/Opc.Ua.SourceGeneration.Core/Generators/NodeManagerTemplates.cs	Emits base ctor + factory `NamespacesUris` with additional namespaces; emits `CompleteConfigureAsync` call.
tools/Opc.Ua.SourceGeneration.Core/Generators/NodeManagerGenerator.cs	Formats and injects additional namespaces into generated templates.
tools/Opc.Ua.SourceGeneration.Core/Generators.cs	Plumbs additional namespaces from discovery/binding into generator options.
tools/Opc.Ua.SourceGeneration.Core/DesignFile.cs	Adds design option for node-manager additional namespace URIs.
src/Opc.Ua.Server/Fluent/NodeManagerAttribute.cs	Adds `[NodeManager].AdditionalNamespaceUris` public surface.
src/Opc.Ua.Server/Fluent/FluentNodeManagerBase.cs	Adds `CompleteConfigureAsync` wrapper to re-run reverse-reference pass post-`Configure`.
src/Opc.Ua.Server/Hosting/FluentNodeManagerFactory.cs	Removes bespoke external-references workaround and uses `CompleteConfigureAsync` instead.
src/Opc.Ua.Server/Fluent/ReferenceBuilderExtensions.cs	Adds `OrganizedBy` / `UnderObjectsFolder` fluent sugar (inverse `Organizes`).
src/Opc.Ua.Server/Fluent/InstanceCreationBuilderExtensions.cs	Adds manager-scoped root `CreateInstance` and related builder plumbing/docs.
src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs	Makes `AddExternalReference` idempotent for multi-pass mirroring.
src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs	Makes `AddExternalReference` idempotent for multi-pass mirroring.
src/Opc.Ua.Types/Nodes/TypeTable.cs	Makes `AddEncoding` idempotent when re-registering the same encoding.
tests/Opc.Ua.Server.Tests/AsyncCustomNodeManagerTests.cs	Adds tests for double-run idempotence and second-pass pickup of new nodes.
tests/Opc.Ua.Server.Tests/Hosting/FluentNodeManagerFactoryCoverageTests.cs	Adds hosting end-to-end coverage for Objects-folder mirroring.
tests/Opc.Ua.Server.Tests/Fluent/ReferenceBuilderExtensionsTests.cs	Adds tests for new reference sugar methods.
tests/Opc.Ua.Server.Tests/Fluent/InstanceCreationBuilderExtensionsTests.cs	Adds tests for manager-level root instance creation and rebasing behavior.
tests/Opc.Ua.Types.Tests/Nodes/TypeTableTests.cs	Adds test asserting `AddEncoding` idempotence.
tests/Opc.Ua.SourceGeneration.Tests/ModelGeneratorTests.cs	Adds generator test for `AdditionalNamespaceUris` emission in ctor + factory.
tests/Opc.Ua.SourceGeneration.Core.Tests/Generators/NodeManagerGeneratorTests.cs	Extends generator structural tests for `CompleteConfigureAsync` order and additional namespaces.
docs/NodeManagers.md	Documents cross-manager placement, fluent sugar, and `[NodeManager]` additional namespaces.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

github-actions · 2026-08-28T17:41:46Z

Code coverage

✅ Coverage gate passed.

Check	Result	Threshold
✅ Project line rate	86.18% (233689/271171 lines)	>= 70.00%
✅ Project branch rate	75.63%	>= 60.00%
✅ Patch coverage	98.85% (86/87 changed lines)	>= 60.00% (<= 100 changed lines, advisory)
ℹ️ Baseline delta (advisory)	+12.58 pp	73.60% recorded

Uncovered changed lines

tools/Opc.Ua.SourceGeneration/Extensions.cs: 351

Coverage is above the recorded baseline - consider ratcheting coverage-thresholds.json.

_{Thresholds live in coverage-thresholds.json. Whole report before exclusions: line 85.54%, branch 75.07%.}

codecov · 2026-08-28T17:42:04Z

Codecov Report

❌ Patch coverage is 96.55172% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.33%. Comparing base (ca9cfcc) to head (16464da).

Files with missing lines	Patch %	Lines
tools/Opc.Ua.SourceGeneration/Extensions.cs	72.72%	3 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #4331      +/-   ##
==========================================
+ Coverage   80.31%   80.33%   +0.01%     
==========================================
  Files        1927     1927              
  Lines      263700   263779      +79     
  Branches    46129    46146      +17     
==========================================
+ Hits       211799   211903     +104     
+ Misses      35649    35624      -25     
  Partials    16252    16252

Flag	Coverage Δ
actions	`80.33% <96.55%> (+0.01%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
src/Opc.Ua.Server/Fluent/FluentNodeManagerBase.cs	`81.25% <100.00%> (+0.39%)`	⬆️
...Server/Fluent/InstanceCreationBuilderExtensions.cs	`59.84% <100.00%> (+10.32%)`	⬆️
...Opc.Ua.Server/Fluent/ReferenceBuilderExtensions.cs	`79.62% <100.00%> (+0.38%)`	⬆️
.../Opc.Ua.Server/Hosting/FluentNodeManagerFactory.cs	`86.84% <100.00%> (+46.84%)`	⬆️
...pc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs	`82.93% <100.00%> (-0.03%)`	⬇️
src/Opc.Ua.Server/NodeManager/CustomNodeManager.cs	`63.75% <100.00%> (+0.18%)`	⬆️
src/Opc.Ua.Types/Nodes/TypeTable.cs	`98.57% <100.00%> (+<0.01%)`	⬆️
tools/Opc.Ua.SourceGeneration.Core/DesignFile.cs	`96.07% <ø> (ø)`
tools/Opc.Ua.SourceGeneration.Core/Generators.cs	`88.82% <100.00%> (+0.04%)`	⬆️
...Generation.Core/Generators/NodeManagerGenerator.cs	`98.43% <100.00%> (+0.39%)`	⬆️
... and 4 more

... and 23 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Address Copilot review on #4331: NodeId follows the stack's INullable pattern, so the placement test now uses a non-nullable NodeId with NodeId.Null as the sentinel and checks .IsNull instead of HasValue/.Value. Also cover the root InstanceBuilder null-argument guards flagged as uncovered by the coverage report. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>

Copilot AI lite review requested due to automatic review settings August 28, 2026 17:06

Copilot started reviewing on behalf of romanett August 28, 2026 17:06 View session

Copilot AI reviewed Aug 28, 2026

View reviewed changes

Comment thread tests/Opc.Ua.Server.Tests/Hosting/FluentNodeManagerFactoryCoverageTests.cs Outdated

marcschier approved these changes Aug 29, 2026

View reviewed changes

marcschier merged commit 41db5ac into master Aug 29, 2026
259 checks passed

marcschier deleted the romanett/ua-net-standard-4329-d819da branch August 29, 2026 08:39

romanett mentioned this pull request Sep 2, 2026

Hosting AddNodeManager(namespaceUri, build) always creates a root folder named "ReferenceServer" #4388

Closed

Navigation Menu

Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) - #4331

Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement)#4331
marcschier merged 2 commits into
masterfrom
romanett/ua-net-standard-4329-d819da

romanett commented Aug 28, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

codecov Bot commented Aug 28, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

romanett commented Aug 28, 2026

Description

Related Issues

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

github-actions Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code coverage

Uh oh!

codecov Bot commented Aug 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

github-actions Bot commented Aug 28, 2026 •

edited

Loading

codecov Bot commented Aug 28, 2026 •

edited

Loading