GithubStar/UA-.NETStandard
1
0
Fork 0
mirror of https://github.com/OPCFoundation/UA-.NETStandard.git synced 2026-10-22 09:37:12 +08:00
Code Issues Packages Projects Releases Wiki Activity
2,739 Commits 116 Branches 105 Tags
fix/encoder-stack-depth-json-flake
Commit Graph

5 Commits

Author SHA1 Message Date
romanett 442889cba5 Fix source generator audit findings; conform structure-field matrices and type metadata to OPC UA 1.05 (#4569) 
# Description

This PR fixes a bug audit of the source generation projects
(`tools/Opc.Ua.SourceGeneration*`: model loading, NodeSet import, all
emitters, the Roslyn hosts and packaging). It also fixes a max-effort
code review of those fixes, and a check of every spec-relevant change
against the OPC UA 1.05 text.

- **Scope:** about 85 audit findings, 48 review findings and 32
spec-grounding items were addressed. Each fix has a regression test that
fails without it.
- **Wire change:** it includes one deliberate, stack-wide wire change:
**structure-field matrices now follow OPC 10000-6 5.2.5 Table 28**.

Most changes are internal correctness fixes. The sections below list
every change a user can observe: wire formats, generated code and API,
generated address-space content, diagnostics and runtime behaviour.

## Behaviour / user-facing changes

### Wire format (encoders)
- **Binary inline matrices** (OPC 10000-6 5.2.5 Table 28).
- **Layout:** a multi-dimensional *structure field* is encoded as
`Int32[] Dimensions`, then the values **without a length prefix**, with
at least 2 dimensions.
- **Empty and null:** any dimension <= 0 means no values follow, and a
null matrix is `-1`. An empty matrix is written as `[0,0]`.
- **Compatibility:** this is **not compatible with earlier builds of
this SDK** for matrix fields. It applies to generated types,
`[DataType]` types, `Structure`, client ComplexTypes and
`WriteEncodeableMatrix`.
  - **Unchanged:** Variant matrices (5.2.2.16) keep their old encoding.
- **Declared rank decides matrix vs array.** A 1-D `MatrixOf<T>` in an
array field or a Variable value is written as an array again.
Matrix-declared fields go through the new public
`EncoderExtensions.WriteInlineMatrixValue`, which generated code also
uses. A non-empty 1-D value in a matrix field throws `BadEncodingError`.
- **Decoder hardening:**
- Before allocating, the decoder bounds all matrix dimensions against
overflow, `MaxArrayLength` and the remaining message length.
  - The decoder rejects:
    - a matrix with fewer than 2 dimensions;
- a populated matrix whose rank differs from the field's declared rank;
    - a union SwitchField greater than the number of fields;
    - an EncodingMask with unassigned bits set.
- **Encoder `MaxArrayLength`:** the encoder enforces `MaxArrayLength`
for inline matrices, as it already did for arrays.
- **XML structure fields** (5.3.1, 5.3.4, 5.3.5).
- **Fields:** `Structure` and client ComplexTypes write fields like
typed fields (`<A>1</A>`, array elements directly) instead of a Variant
body.
- **Matrix fields:** `Dimensions` and `Elements` go directly inside the
field element, with no `<Matrix>` wrapper.
- **Empty matrices:** written as nil, because XML dimensions must be
greater than 0.
  - **Compatibility:** decoders still accept the old shapes.
- **Enum fixes:** enum array elements with value 0 are no longer
dropped, and enum array element names are fixed.
- **Other encoding fixes:**
- A null `DiagnosticInfo` structure field writes `0x00` instead of
nothing.
- A null scalar field in a runtime `Structure` is written as its default
instead of zero bytes, which used to shift every following field.
- A Variant holding an empty matrix encodes as an empty array instead of
throwing.
  - `Variant.Copy` keeps the matrix identity of null and empty matrices.
- **NodeSet export and the DataContract surrogate** write Variable
values with the Variant rules again. A null or empty `MatrixOf` variable
no longer produces invalid XML or unreadable NodeSets.
- **`JsonEncoder.CanOmitFields`** is `true` only for Compact encoding.
Verbose and RawData JSON include default-valued fields.

### `[DataType]` source-generated types
- **Default-value handling** (OPC 10000-6 5.4.1, Table 1).
- **Encode:** a field is omitted only when its value *and* its declared
default are the type's default (0, false, null). Fields with a
non-default initializer or constructor default, and empty strings, are
always written, so configuration files get slightly larger.
- **JSON decode:** a field missing from JSON decodes to the type
default.
- **XML decode:** a field missing from XML keeps its declared default,
which older configuration files rely on.
- **Why:** this fixes silent round-trip loss, for example
`SamplingInterval = 0` read back as `-1`.
- **Nested `[DataType]` types:** they are now generated inside their
containing types. Without an explicit id, their default TypeId and
XmlName are qualified (e.g. `s=Outer.Foo`); **wire identity change** for
nested types. Colliding activator names get a `_2` suffix.
- **Activators:** they are `internal` when the type, or any containing
type, is not public (was CS0060).
- **Matrix properties:**
  - `MatrixOf<T>` properties use the inline matrix encoding.
- Matrices of non-sealed structures, or with
`StructureHandling.ExtensionObject`, are written as ExtensionObjects.
- Their published ValueRank is 2, because the rank can't be known from
`MatrixOf<T>`.
- **Published StructureDefinitions:**
  - They include base-class fields.
  - Nullable properties are no longer marked optional.
  - Inline structure fields name their concrete type.
- **`[Flags]` enums:** they are published as plain enumerations with
their member values, which matches how they are encoded.
- **Other generated code:**
- `Clone` deep-copies arrays and matrices of structures, Variants,
ExtensionObjects and DataValues.
- `IsEqual` and `GetHashCode` are consistent: EncodingMask, active union
member and NaN handling agree.
- Pooled `Reuse()` restores the freshly constructed state through the
new `protected virtual ResetForReuse()`, and returns an instance to its
runtime type's pool.
- **Enum capture:** enum member values are captured culture-invariantly.
Negative values were lost on cultures that use U+2212 as the minus sign.

### Model-driven generator: generated code, API and file names
- **Removed API:** generated `*_Placeholder` proxy accessors (e.g.
`PowerTrainTypeClient.GetMotorIdentifier_PlaceholderAsync`) and fluent
`X_Placeholder()` accessors are removed; they could never resolve
anything.
- Proxy accessors now look children up by BrowseName with the right
namespace.
- `RoboticsClient` finds motors and gears by type and drives via
`IsDrivenBy`. Its topology snapshots used to be empty.
- **Renamed files:** generated files for a bound `[NodeManager]` are now
named `{Namespace}.{Class}.NodeManager.g.cs` (also `.Factory` and
`.FluentBuilders`).
- **Generated NodeState code:**
- Attributes are set through `baseState`/`nodeState` locals, so children
named like base attributes (e.g. `DataType`, `EventNotifier`) compile.
- Children that hide base members are declared `public new`, using names
reflected from the runtime.
  - `global::` qualification is used throughout.
- **Proxies and fluent builders:** a proxy whose supertype is excluded
(e.g. Draft) derives from the nearest emitted ancestor. Fluent wrappers
and NodeState factories fall back the same way; they used to fail with
CS0246.
- **Derived structures:**
  - The optional-field `EncodingMask` continues across inheritance.
- A derived structure over a base type from another model no longer
relies on `base.ResetForReuse`.
- **Default values:**
- NodeId, QualifiedName and ExpandedNodeId defaults resolve by namespace
URI (`context.NamespaceUris.GetIndexOrAppend`) and server URI.
- StatusCode, negative enum, `double`/`float` (shortest round-trip
literal) and escaped string defaults now compile.
- **Event records:**
- Vendor event records have `Decoder.GetStandardFields(NamespaceTable)`,
and their browse paths use the model's namespace.
- A field whose namespace is unknown to the session no longer matches a
namespace-0 field.
  - Companion `StandardFields` is `[Obsolete]`.
- Vendor simple subtypes such as `Percent : Double` map to their
built-in type.
- **Other fixes:**
- `MethodState.CallAsync` passes the real `objectId` to synchronous
`OnCallMethod2` handlers; it used to pass `NodeId.Null`.
- Enumeration fields with an explicit `Identifier` keep it, instead of
taking a trailing `_<digits>`.
  - Explicit NumericIds are no longer reused by automatic numbering.
  - Dependencies are loaded in topological order.

### Generated address-space content
- **Arguments:**
- Methods without arguments no longer get empty
`InputArguments`/`OutputArguments` properties (OPC 10000-3 5.7).
- About 2,660 automatically assigned namespace-0 NodeIds of 15000 and
above shift. Every NodeId in `StandardTypes.csv` is unchanged.
- **Permissions:** type nodes and method declarations on types now carry
the model's `RolePermissions`/`AccessRestrictions`.
- **Restored nodes:** NodeSet import keeps nodes that were silently
dropped before:
- `NamespaceMetadata` objects and the NodeSet's own TypeDictionary
variables;
  - ISA95 placeholders linked only non-hierarchically;
  - instances whose parent is in another namespace.
- **Import details:**
  - ValueRank > 1 without ArrayDimensions keeps its rank.
  - `svr=` indexes map correctly.
  - Invalid MinimumSamplingInterval values import as -1, not 0.
  - Localized texts prefer the invariant or English translation.
- **StructureDefinitions:**
- They include inherited fields, including for subtypes without their
own fields (e.g. `AnonymousIdentityToken` now publishes PolicyId).
  - The StructureType kind covers inherited AllowSubTypes fields.
  - Unions ignore IsOptional.
- StructureFields never publish ValueRank 0; a OneOrMoreDimensions field
without dimensions becomes rank 2.
- Fields with ValueRank -2/-3 are published as scalar BaseDataType,
which is what is on the wire.
- **OptionSets:**
  - Implicit fields get the next free bit.
- `OptionSetValues` uses null for undefined bits instead of "Reserved",
and structure OptionSets publish all their bits (no 32/96-bit cap).
- Numeric OptionSets are limited to their base type's width, and UInt64
bit 63 works.
- **Schemas:**
- Structures with inline-matrix fields are left out of the `.bsd`
(5.2.5).
- In the BSD, Variant-encoded and non-structure subtyped fields are
described as `ua:Variant`.
- In the `.xsd`, matrix fields are typed `ua:Matrix` with
`Dimensions`/`Elements`, and `EncodingMask` is required.

### NodeSet / model input handling
- **Model selection:**
- The newest NodeSet of a model is chosen by `ModelVersion` (SemVer 2.0
precedence), then PublicationDate. The `Version` attribute, which Annex
F says is not for comparison, is only a tie-break.
- The order is total and does not depend on input order; referenced
assemblies use the same rule.
- **Default names:**
- Invalid default Name/Prefix values are repaired (`201301ISA95` becomes
`_201301ISA95`, `class` becomes `class_`).
  - Valid names are unchanged.
  - Sanitising no longer depends on the OS.
- **`ModelUri` metadata** that doesn't match the NodeSet raises a
warning instead of silently losing the model.
- **Input detection:**
- Only XML files whose root is `ModelDesign` or `UANodeSet` are treated
as models.
- A malformed NodeSet stays a NodeSet input, so it no longer breaks the
ModelDesign pass.
- **`IdentifierFile` paths:** they are normalized (`./`, `../`), and the
suffix match requires a folder boundary.
- **Dependency payloads** (`ModelDependencyV1`) carry field IsOptional,
AllowSubTypes, ArrayDimensions and the producer's namespace tables in
skippable trailers, so old and new readers stay compatible.

### Roslyn host, diagnostics, packaging
- **New diagnostics:**
- MODELGEN036: `[NodeManager]` target not supported (nested, generic,
file-local or global namespace).
- MODELGEN037: `[DataType]` target not supported (struct, generic,
abstract, file-local or global namespace).
- MODELGEN038: the definition of an encodeable base type can't be found.
- **New validator errors:**
- a structure with both optional and subtyped fields (OPC 10000-6 F.13);
  - multi-bit or duplicate OptionSet masks.
- **Diagnostic locations:** `[DataType]`/`[NodeManager]` diagnostics
point at real source locations again, so `#pragma` and `.editorconfig`
apply. MODELGEN010 is the exception.
- **Incremental caching:** pipeline values compare by value, so
unrelated edits no longer regenerate the whole model or all
`[DataType]`s.
- **`ModelSourceGeneratorIgnore`** on a ModelDesign input is honoured;
the design still resolves as a dependency.
- **Cancellation:** a cancelled generation run propagates
`OperationCanceledException` instead of reporting a MODELGEN003 error.
- **Stack package:** the `Stack` NuGet package now ships
`build/OPCFoundation.NetStandard.Opc.Ua.SourceGeneration.Stack.props`,
so `StackSourceGeneratorMode` and `StackSourceGeneratorExclude` reach
the compiler.

## Related Issues

- No tracking issue. The audit, review and spec-grounding reports are
available on request. Contains the matrix wire-format change discussed
for D-5 (OPC 10000-6 5.2.5 Table 28).

## 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.
(`docs/SourceGeneratedDataTypes.md`, `docs/NodeManagers.md`, generator
readme)
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings. (`UA.slnx` 0 warnings / 0 errors on net48 and
net10.0)
- [ ] I ran **all** tests locally using the **UA.slnx** solution against
at least .net **framework** and .net **10**, and all passed. The
affected suites were run on net10.0: SourceGeneration Core/Roslyn/Stack,
Types, Core.Encoders, Encoders.Fuzz, Client, Client.ComplexTypes,
PubSub, Core, Server, GDS, Configuration, Di, Robotics, ISA95,
Positioning, OpenUsd, XRegistry. net48 was built; its test legs, and the
post-rebase verification, run in CI.
- [ ] 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](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
 2026-09-29 00:47:40 +02:00
romanett ececea15c8 Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager (#4434) 
# Description

Builds on #4428, which is now merged; this branch is rebased onto
master.

The GDS `ApplicationsNodeManager` becomes a source-generated
`[NodeManager]`: the companion model, its loader and the fluent plumbing
are emitted, and what stays hand-written is the behaviour. Getting there
needed three things from the shared surface, which are the first two
commits.

## Commits

**1. Add the fluent hooks a hand-written manager needs at startup**

- `OnReadRolePermissions` / `OnReadUserRolePermissions` on the node
builder. Every other `NodeState` `On*` hook was already fluent; these
were not, and a server that grants access on something the static model
cannot express has to compute the permission set per request.
- `Node<TState>(TState)` returns a builder over a node the caller
already holds, for nodes materialised after `Configure` resolved its
graph.
- `OnAddressSpaceReadyAsync` on `FluentNodeManagerBase` — the one seam
between the address space existing and the wiring being applied, for
managers whose wiring depends on asynchronous setup. `Configure` is a
`partial void` and cannot await. `DiNodeManager` had already invented
the hook privately, so its declaration becomes an `override`.

**2. Let a `[NodeManager]` bind to a model a referenced assembly
supplies**

A model belongs to the assembly that emits its types, which leaves a
node manager for it homeless whenever that assembly cannot reference
`Opc.Ua.Server` — a model-only package shared with clients, for
instance. Binding was rejected twice over: the design was skipped before
the binding was matched (`MODELGEN010`), and fluent-accessors-only mode
refused to run alongside manager generation (`MODELGEN014`). The binding
is now matched before the skip decision, and a matched binding puts that
design into accessors-only mode. A design nothing binds to is still
skipped, so this cannot start duplicating types by accident.

The emitted manager also gains what a real manager needs:
`DefaultNamespaceUris()` plus a protected constructor taking a
replacement set; `GenerateDefaultConstructor=false` to suppress the
two-argument form; its own logger category; and the call to
`OnAddressSpaceReadyAsync`.

Generated fluent accessors also stop re-enabling nullable warnings — a
bare `#nullable enable` undid the shared header's `disable warnings`, so
any model with a structure-typed method argument reported CS8600 on
every generated `out T` in a project that builds warnings as errors.

**3. Reimplement the GDS `ApplicationsNodeManager` as a generated node
manager**

The design stays owned by `Opc.Ua.Gds.Common`; the server assembly only
binds a manager to it.

**NodeIds do not move.** `NamespaceIndexes[0]` must stay the application
record namespace — it is the one the application database, the
certificate request store and the base allocator mint ids in — so the
constructor passes the namespace order explicitly rather than adopting
the generated model-first default. A test pins both ends against a live
server.

Four overrides go, leaving `OnAddressSpaceReadyAsync` and
`Dispose(bool)` — no address-space overrides at all:

| Removed | Why it was unnecessary |
| --- | --- |
| `New` | The base allocator already mints into `NamespaceIndexes[0]`,
with a thread-safe counter. The override existed only because
`Create(assignNodeIds: true)` asks the factory with the declaration id
still set; `AssignInstanceNodeId` nulls it and re-asks, documented as
being there for exactly this allocator. |
| `AddBehaviourToPredefinedNodeAsync` | Its `AuthorizationServiceType`
branch re-wired a node that was already wired, and its
`KeyCredentialServiceType` branch had nothing to wire — the model ships
that folder empty. |
| `GetManagerHandleAsync`, `ValidateNodeAsync` | Verbatim
reimplementations of the base that also blocked the fluent base's
virtual-node support. |
| `DeleteAddressSpaceAsync` | A `// TBD` passthrough. |

Everything else — all sixteen directory methods with their
self-administration permissions, the certificate groups' concrete types
and trust-list flags, and the authorization service (created through
this PR's `Add` surface and wired in the same place) — is one
`Configure` pass.

**4. Bind the certificate groups from the `Configure` pass**

Acquiring a certificate group and binding it to its nodes are different
kinds of work: the first is I/O (`InitAsync` opens certificate stores
and creates CA certificates), the second only touches the address space.
Each now runs in the phase it belongs to, which puts the whole of the
wiring in one place.

Ownership no longer rides on the binding. A group joins
`m_ownedCertificateGroups` the moment it is constructed, before
`InitAsync` can throw, so a group that fails half way through is still
disposed — which the previous code could not manage, keying ownership
off a dictionary it filled only after initialization succeeded.

> Commit 3's message says this move is impossible and blames `PushTest`.
**That was a wrong diagnosis, and this commit supersedes it.** Two real
defects were hiding behind those failures:

- **`Create(..., NodeId.Null, ..., assignNodeIds: false)` does not null
the NodeId.** `NodeState.Create` overrides it only when handed a
non-null one, so a custom group node kept the namespace-0 id of its type
declaration. `AssignInstanceNodeId` hid this by re-asking the factory
with the id nulled; the builder validates instead, and rejected it. The
node is now created with an id in the manager's own namespace, as the
`Default` authorization service already was.
- **The custom certificate group node was not browseable.** It answered
`GetCertificateGroups` and `ReadNode`, but a type-filtered recursive
browse from the Objects folder never returned it — which OPC 10000-12
§7.8.2 requires of a certificate group. Staging the node through the
builder gives it the reference it was missing.

### Two `PushTest` methods change — please review these closely

The second defect had been masking what those tests actually did.
Selecting the group positionally, `groups[3]` resolved to the server's
**own `ServerConfiguration` group** while the custom one stayed
invisible, so `UpdateTrustListOfCustomGroupAsync` and
`AddRemoveCertOfCustomGroupAsync` were exercising a *transactional*
trust list under a custom-group name.

Browse results, before and after:

| # | Before (baseline) | After |
| --- | --- | --- |
| 0-2 | GDS Directory groups (`ns=3`) | GDS Directory groups (`ns=3`) |
| 3 | **`DefaultApplicationGroup` `i=14156`** — the server's own |
**`MyCustomGroup` `ns=2`** |
| 4 | — | `DefaultApplicationGroup` `i=14156` |

They now resolve the group by browse name — which asserts it is
browseable at all — and expect the immediate-apply semantics the GDS
actually gives the groups of its own `Directory`: `CloseAndUpdate`
writes the stores at once and reports `applyChangesRequired == false`,
so the `ApplyChanges` calls go away (they would return
`Bad_NothingToDo`). The read-back assertions are what prove the writes
landed, and they are unchanged.

This is a genuine loss of the *transactional* trust-list coverage those
two methods were accidentally providing — the server's own group is
still covered by the other `PushTest` methods, which target it
deliberately.

**5. Select `GetCertificates` results by type instead of by position**

`GetCertificatesAsync` checked entry zero of a parallel-array result.
Which entry that is depends on the order the server reports its
certificate types in, an empty result throws `IndexOutOfRange` rather
than failing an assertion, and a second type going missing or
unparseable would not be noticed. Every returned pair is now checked,
and a failure names the certificate type.

**6. Drop the `.Common` suffix from the three GDS projects**

Every other companion specification ships as `<Family>` /
`<Family>.Client` / `<Family>.Server` — Di, ISA95, Positioning,
Robotics, Vision, WotCon, XRegistry. GDS was the exception, with a
`.Common` suffix that named nothing.

| Before | After |
| --- | --- |
| `src/Opc.Ua.Gds.Common` | `src/Opc.Ua.Gds` |
| `src/Opc.Ua.Gds.Client.Common` | `src/Opc.Ua.Gds.Client` |
| `src/Opc.Ua.Gds.Server.Common` | `src/Opc.Ua.Gds.Server` |

**No source changes were needed.** `RootNamespace` was already the final
name in all three projects — the namespaces have always been
`Opc.Ua.Gds`, `Opc.Ua.Gds.Client` and `Opc.Ua.Gds.Server`, and only the
assembly and package ids disagreed. Nothing compiling against these
assemblies changes a `using`.

The package ids do change, which is consumer-visible, so
`docs/migrate/2.0.x/packages.md` gains a renamed-packages section — it
is the document that undertakes to cover NuGet renames — and the
migration skill's package table gains the two rows a 1.5.378 consumer
needs.

Build infrastructure that names the projects or their output moves with
them: both solution files, `expected-packages.txt` (re-sorted — it is
maintained alphabetically), both signing manifests, the reference-server
Dockerfile, the metapackage nuspecs, and the analyzer's package
dependencies. `preview-pack.slnx` is derived from `UA.slnx` and needs no
edit. The migration analyzer's shim tree mirrors the source layout by
its own documented convention, so `Gds.Client.Common/` becomes
`Gds.Client/`.

Prose naming past work is left alone — plan 23 still records that its
deferral happened during the "GDS Client.Common modernization", which is
the name that effort had.

## Validation

`net10.0`, on master + these four commits, from a clean GDS PKI:

- `UA.slnx`: 0 errors, 0 warnings in changed files (the solution's 30
pre-existing warning lines are CA1861/CA1850/CA1307 in `XRegistry`,
`Types.Tests` and `WotCon.Tests` — same rules and projects as the base).
- `Opc.Ua.Gds.Tests`: 1,117 passed, 0 failed, 54 skipped — identical to
the base branch.
- `Opc.Ua.Server.Tests`: 4,961 passed, 0 failed, 5 skipped (measured
before commit 4, which touches only the GDS assembly and its tests).
- `Opc.Ua.SourceGeneration.Tests`: 169 passed.
- `Opc.Ua.SourceGeneration.Core.Tests`, NodeManager/Generators/Fluent:
84 passed; `GenerateCode`: 21 passed, 4 skipped.
- `Opc.Ua.MigrationAnalyzer.Core.Tests`: 10 passed, 3 skipped.
- After the rename: assemblies build as `Opc.Ua.Gds.dll` / `.Client.dll`
/ `.Server.dll`, with no `*.Common.dll` produced anywhere.
- Commits 1-3 build standalone.
- `Opc.Ua.Gds.Server.Common` also builds for `net48` and
`netstandard2.1` (checked before the rebase onto #4428).

Not yet run: the full `Opc.Ua.SourceGeneration.Core.Tests` suite
(~3,800), and the older target frameworks since the rebase.

## Checklist

- [x] Existing tests pass
- [x] New tests cover the new surface (role permissions, resolved-node
builder, referenced-model binding, constructor/namespace control, GDS
namespace invariant, custom-group node namespace)
- [ ] Full Core.Tests sweep and net48/netstandard2.1 re-run after the
rebase

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
 2026-09-09 10:41:06 +02:00
Marc Schier 02ea934a31 Complete fluent node manager extensibility (#4418) 
# Description

Complete the fluent node-manager extensibility surface needed by dynamic
and externally backed address spaces.

This change:

- adds virtual node families with cheap NodeId ownership checks, async
materialization, per-operation caching, custom browsing, and
family-level callbacks;
- adds monitored-item pre-creation decisions, complete lifecycle hooks,
async batch hooks, stack-managed custom monitored items, and
subscription-gated polling;
- exposes the startup `ApplicationConfiguration` to generated manager
partials through `FluentNodeManagerBase`;
- fixes typed builder proxy support, concrete object wrapper types, and
fully-qualified generated model composition;
- reports unresolved `[NodeManager]` namespace expressions as
`MODELGEN035` instead of silently dropping them;
- documents the new APIs and compatibility behavior.

## Related Issues

- Fixes #4397
- Fixes #4398
- Fixes #4399
- Fixes #4403
- Fixes #4404
- Fixes #4405

## Validation

- `Opc.Ua.Server.Tests` on `net10.0`: 4,886 passed, 5 skipped.
- `Opc.Ua.SourceGeneration.Core.Tests` on `net10.0`: 3,807 passed, 8
skipped.
- `Opc.Ua.SourceGeneration.Tests` on `net10.0`: 164 passed.
- Runtime and generator regression selections passed on `net48`.
- Review-feedback validation: 376 fluent tests passed on `net10.0`; 22
focused tests passed on `net48`.
- The CI timing regression was repeated five times on `net48` and once
on `net10.0`.
- All 266 applicable GitHub Actions and Azure Pipelines checks,
including CodeQL and coverage, pass on `83c571423`.
- AppVeyor reports an immediate zero-duration no-run failure because
`appveyor.yml` whitelists only the `appveyor` branch; it did not execute
this PR.
- `UA.slnx` builds successfully for `net10.0`; affected project builds
are warning-free.
- The repository-wide `dotnet test UA.slnx` invocation is currently
blocked by the solution's mixed VSTest/Microsoft.Testing.Platform
configuration under the .NET 10 SDK, so the affected test projects were
run individually.

## 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.
- [x] I fixed **all** failing and flaky tests in the CI pipelines and
**all** CodeQL warnings.
- [x] I have addressed **all** PR feedback received.

---------

Copilot-Session: afaa67c9-fc97-4185-a785-edd54a937d95
 2026-09-05 09:25:49 +02:00
romanett 41db5aca60 Fluent: publish Configure-created nodes' references on foreign-manager nodes (Objects folder placement) (#4331) 
# 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 #2 shape becomes fully fluent:

```csharp
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

- Fixes #4329
- Motivating case: OPCFoundation/UA-.NETStandard-Samples#756 /
OPCFoundation/UA-.NETStandard-Samples#795 (Workshop/Boiler migration)

## Checklist

- [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.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

---------

Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
 2026-08-29 10:38:56 +02:00
Marc Schier 7cec2d614d Move repository files to the consolidated layout 2026-07-16 06:25:54 +02:00