Commit Graph

2 Commits

Author SHA1 Message Date
Marc Schier 736b92ac7e Fix #3768: stop emitting unimplemented Optional children on singleton instances (#3828)
# Description

Singleton instances declared in `StandardTypes.xml` and
`Opc.Ua.NodeSet2.xml` (`Server`, `Server.ServerConfiguration`,
`HistoryServerCapabilities`, `OPCUANamespaceMetadata`, the well-known
role objects, the Part 17 `Aliases` category, etc.) were emitting
Optional Variable/Method children that the SDK does not implement — for
example `ServerConfiguration.ApplicationUri`, `ProductUri`,
`ApplicationType`, `HasSecureElement`, `CancelChanges`,
`ResetToServerDefaults`, `Server.UrisVersion`,
`Server.EstimatedReturnTime`, `Server.LocalTime`,
`Server.ServerCapabilities.ConformanceUnits` and friends.

The cause was the source generator promoting every type-definition
child's modelling rule into the singleton-instance factory whenever
`UseTypeDefinitionModellingRules` was enabled (#3799), which made every
Optional child unconditionally appear in the singleton's `CreateXxx`
body.

This PR fixes the regression at the generator level (without touching
`StandardTypes.xml`) and lets the SDK opt back in to just the Optional
children it actually implements.


## New: chainable `Add{Child}` API with configure-callback overloads

Per review feedback ("the existing predefined node is used. A user adds
optional children using `.AddXY(NodeId? nodeId = default)`. The Add
calls should be possible to chain. … Instead of AddAndGet, emit an Add
overload with a callback to configure the created node state further,
add another Add to take a condition predicate that decides whether the
Create is executed or not"), the source generator now emits **five**
chainable `Add{Child}` overloads per Optional child of every typed state
class. All return the **owner** (`this`) so multi-child wiring collapses
into a single fluent chain:

```csharp
// (1) base — idempotent ensure-child
public TOwner Add{Child}(ISystemContext context, NodeId? nodeId = default);

// (2) Action — always run, mutate the just-created child in-line
public TOwner Add{Child}(ISystemContext context, Action<TChild> configure, NodeId? nodeId = default);

// (3) Action — conditional; when false: no Add, no callback
public TOwner Add{Child}(ISystemContext context, bool condition, Action<TChild> configure, NodeId? nodeId = default);

// (4) Func — always run, transform/replace the typed slot
public TOwner Add{Child}(ISystemContext context, Func<TChild, TChild> configure, NodeId? nodeId = default);

// (5) Func — conditional
public TOwner Add{Child}(ISystemContext context, bool condition, Func<TChild, TChild> configure, NodeId? nodeId = default);
```

The previous `AddAndGet{Child}(context, NodeId?)` helper is **removed**;
every call site is migrated to one of the new overloads. The Action
overloads cover the common "set Value / wire OnCallAsync" case in-line;
the Func overloads allow callers to replace the typed slot with a
wrapper or subclass instance. The boolean overloads short-circuit when
the condition is `false`.

When `nodeId` is `null`: if `context.NodeIdFactory` is non-null the
helper mints a fresh NodeId via `context.NodeIdFactory.New(context,
child)`; otherwise it leaves the type-level NodeId assigned by the
generated factory unchanged (the caller can patch later).

`NodeState.Create(context, source)` stays authoritative (the loaded
NodeSet XML / passive source is the source of truth for what children
exist). The new chainable `Add{Child}` overloads are the
runtime-extension API for SDK code and downstream users that want to add
Optional children programmatically after load.

A new `Tokens.OwnerClassName` carries the owning typed state class name
through the per-child template loop so `Add{Child}` can return `this`
typed as the owner; the template engine's outer-scope lookup cascades
the value from `AddNodeStateClassTypeReplacements` /
`AddNodeStateClassMethodTypeReplacements` into the per-child template
scope.

### SDK migration in `Libraries/Opc.Ua.Server`

- `DiagnosticsNodeManager` add-back helpers collapse into single fluent
chains; the `SetSubscriptionDurable` conditional folds into the Server
chain via the bool-condition overload
(`AddSetSubscriptionDurable(context, m_durableSubscriptionsEnabled, _ =>
{ }, MethodIds.Server_SetSubscriptionDurable)`); the `OperationLimits`
add-back inlines via
`.AddOperationLimits(...).OperationLimits!.AddMax...()`.
- `ConfigurationNodeManager` chains
`AddGetCertificates(context).AddCreateSelfSignedCertificate(context)`
before `Create(context, passiveNode)`.
- `HistoricalDataConfigurationInstaller.PopulateProperties` is now a
single fluent chain using the conditional Action overload —
`.AddMaxTimeInterval(context, capabilities.MaxTimeInterval > 0, c =>
c.Value = capabilities.MaxTimeInterval)` replaces the `if (cond) {
config.AddAndGetMaxTimeInterval(context).Value = ...; }` ceremony.
- `KeyCredentialPushSubject` folds `OnCall = null; OnCallAsync = ...`
into Action callbacks on the chain: `state.AddUpdateCredential(context,
c => { c.OnCall = null; c.OnCallAsync = OnUpdateCredentialAsync;
}).AddDeleteCredential(...)`.
- `WotConnectivityNodeManager.WireManagementMethods` chains
`.AddDiscoverAssets(ctx, c => c.OnCallAsync =
...).AddCreateAssetForEndpoint(...).AddConnectionTest(...).AddSupportedWoTBindings(...)`.
- `AssetRegistry` and `AlarmConditionTypeHolder` updated from `state.X
??= state.AddX(context)` to plain `state.AddX(context)` + null-forgiving
access on the typed slot (idempotent `Add{Child}` makes the `??=`
ceremony obsolete).

## Generator fix

`Tools/Opc.Ua.SourceGeneration.Core/Generators/NodeStateGenerator.cs`:

`NodeToGenerate` gains a structural `IsUnderSingletonInstance` flag, set
`true` on the singleton-instance root in `CollectNodesToGenerate` and
propagated to every descendant in `GetChildren`. Mirrors the existing
`RootIsTypeDefinition` propagation pattern; the gate becomes an O(1)
lookup.

```csharp
bool isSingletonInstanceContext =
    m_context.Options.UseTypeDefinitionModellingRules &&
    node.IsUnderSingletonInstance &&
    node.Parent != null &&
    instance is VariableDesign or MethodDesign;

ModellingRule effectiveRule = isSingletonInstanceContext
    ? GetEffectiveModellingRule(node, instance)
    : instance.ModellingRule;
```

The `topLevelInstanceException` is preserved for **TYPE-DEF** roots —
its `UseTypeDefinitionModellingRules` gate only fires when the parent is
an `InstanceDesign` singleton. Without this, the type-def factories
(e.g. `CreateNamespaceMetadataType`) stop emitting their Optional
children which breaks deep code paths that rely on the type-def factory
shape (e.g.
`ConfigurationNodeManager.CreateNamespaceMetadataStateAsync`).

### Why the gate is transitive but narrowed to Variables/Methods

The gate applies **transitively** to every Variable/Method descendant of
a singleton-instance subtree. Deep Optional Variable/Method descendants
such as `Server.ServerCapabilities.MaxArrayLength`,
`Server.ServerCapabilities.OperationLimits.MaxNodesPerRead`,
`Server.ServerRedundancy.RedundantServerArray`, `Server.UrisVersion`,
`Server.EstimatedReturnTime`, `Server.LocalTime`,
`PublishSubscribe.AddConnection` / `RemoveConnection`, the Optional
methods on every well-known role, the OPCUANamespaceMetadata Default*
properties, and `Aliases.LastChange` are now all suppressed by the
generator.

Optional **Object** instances under singletons are intentionally exempt:
they are still emitted by the singleton-specific factory with their
correct instance-level NodeIds. Their subtrees use well-known descendant
NodeIds (e.g.
`ServerConfiguration.CertificateGroups.DefaultHttpsGroup.TrustList.Open
= NodeId 14095`) that the SDK and `ConfigurationNodeManager` bind
against directly. Optional Variable/Method children of those Objects are
still gated transitively (e.g. `OperationLimits.MaxNodesPerRead` is
suppressed even though `OperationLimits` itself stays emitted).

### EURange regression analysis

The earlier attempt at this fix regressed `AnalogItemType.EURange`
(which is `Mandatory` on `AnalogItemType` per the OPC UA spec — a
type-level promotion of `BaseAnalogType.EURange`).
`IsUnderSingletonInstance` is `false` on type-def and synthetic
type-instance roots, so the gate cannot fire on either type-template
tree. Combined with the topLevelInstanceException preserved for type-def
roots, every type-def factory continues to emit its full child set.

## SDK programmatic Add* (Opc.Ua.Server)

`DiagnosticsNodeManager.AddSdkImplementedOptionalChildren` now
dispatches per loaded predefined node type. Each lazy-add call patches
the resulting NodeId (and Method `InputArguments` / `OutputArguments`
NodeIds where applicable) to the well-known singleton-instance value via
`ObjectIds.*` / `MethodIds.*` / `VariableIds.*`. The generator-emitted
`Add{Child}` extension uses the type-level factory which assigns the
type NodeId; the SDK overrides each NodeId after construction.

- **`Server` (ServerObjectState)**: `GetMonitoredItems`, `ResendData`,
`SetSubscriptionDurable` (when enabled), `Namespaces`, `UrisVersion`,
`EstimatedReturnTime`, `LocalTime`.
- **`Server.ServerCapabilities`** (14 Properties): `MaxArrayLength`,
`MaxStringLength`, `MaxByteStringLength`, `MaxSessions`,
`MaxSubscriptions`, `MaxMonitoredItems`, `MaxSubscriptionsPerSession`,
`MaxMonitoredItemsPerSubscription`, `MaxSelectClauseParameters`,
`MaxWhereClauseParameters`, `MaxMonitoredItemsQueueSize`,
`ConformanceUnits`, `RoleSet`, `OperationLimits`. Required to keep
`ServerInternalData.CreateServerObjectAsync`'s non-null assertions
working and to keep `Session.FetchOperationLimitsAsync` round-trip
values matching master.
- **`Server.ServerCapabilities.OperationLimits`** (12 Properties): every
`MaxNodesPerXxx` and `MaxMonitoredItemsPerCall`.
- **`Server.ServerRedundancy.RedundantServerArray`**: re-added for
`DefaultServerRedundancyHandler` (client-side) compatibility.
- **`HistoryServerCapabilities.ServerTimestampSupported`**.
- **6 modifiable well-known roles** (Observer, Operator, Engineer,
Supervisor, ConfigureAdmin, SecurityAdmin) × 11 RoleType children
(`ApplicationsExclude`, `Applications`, `EndpointsExclude`, `Endpoints`,
`CustomConfiguration`, `AddIdentity`, `RemoveIdentity`,
`AddApplication`, `RemoveApplication`, `AddEndpoint`, `RemoveEndpoint`).
The three immutable roles (Anonymous, AuthenticatedUser,
TrustedApplication) have no well-known instance NodeIds for these
methods in the spec and are left untouched.
- **`OPCUANamespaceMetadata`** (i=15957, standard Part 6 namespace
metadata loaded from `Opc.Ua.NodeSet2.xml`): `DefaultRolePermissions`,
`DefaultUserRolePermissions`, `DefaultAccessRestrictions`.
- **Part 17 `Aliases`** (i=23470): `LastChange`. Required by
`AliasNameResolverRefreshMode.AutoOnLastChangeMonitoredItem` in
`Opc.Ua.Client` which subscribes to LastChange to invalidate its cache.

`ConfigurationNodeManager` continues to assign
`CreateSelfSignedCertificate = new
CreateSelfSignedCertificateMethodState(activeNode)` before `Create(...)`
(it is no longer emitted by the singleton factory).

## Related Issues

- Fixes #3768
- Follow-up to #3799

## Verification

- **Generated code inspection**: `CreateServerConfiguration` does
**not** emit `ApplicationUri` / `ProductUri` / `ApplicationType` /
`HasSecureElement` / `CancelChanges` / `ResetToServerDefaults` at
`forInstance=true`. `CreateServer_ServerCapabilities` no longer emits
the 14 SDK-handled Optional Properties at `forInstance=true`; the SDK
re-adds them with the correct instance NodeIds.
`CreateServerConfiguration_CertificateGroups` still emits
`DefaultHttpsGroup` and `DefaultUserTokenGroup` (Object exemption)
preserving the subtree NodeIds that `ConfigurationNodeManager` binds
against. `CreateNamespaceMetadataType` (the type-def factory) emits all
Optional Properties unconditionally so
`ConfigurationNodeManager.CreateNamespaceMetadataStateAsync` keeps
working for runtime-created metadata instances.
- **Strengthened regression test**
`ServerConfiguration_OptionalUnimplementedChildren_NotInAddressSpaceAsync`:
- Negative: 8 expected-suppressed Variable/Method NodeIds
(ServerConfiguration + PublishSubscribe) are absent.
- Positive: all SDK-added children resolve at their well-known
singleton-instance NodeIds — 14 ServerCapabilities Properties + 12
OperationLimits Properties + RedundantServerArray + 3 Server Optional
Variables + the existing Server.Namespaces / GetMonitoredItems /
ResendData / HistoryServerCapabilities.ServerTimestampSupported set.
- Regression guard:
`Server_ServerConfiguration_CertificateGroups_DefaultApplicationGroup_CertificateTypes`
resolves at its well-known NodeId, and the Optional `DefaultHttpsGroup`
/ `DefaultUserTokenGroup` roots remain emitted (Object exemption).
- WellKnownRole regression guard: one method per modifiable role
resolves at its well-known NodeId (Observer.AddIdentity,
Observer.ApplicationsExclude, Observer.AddApplication,
Operator.AddEndpoint, Engineer.RemoveIdentity,
Supervisor.RemoveApplication, ConfigureAdmin.RemoveEndpoint,
SecurityAdmin.AddIdentity).
- `Opc.Ua.Server.Tests`: **1683 / 1683** pass on net10 (5 pre-existing
skips).
- `Opc.Ua.Core.Security.Tests`: **547 / 547** pass on net10 (68
pre-existing skips). Includes the RoleManagementTests,
SecurityRoleServerBase2Tests, and SecurityRoleServerDefaultPermsTests
fixtures.
- `Opc.Ua.Features.Tests`: **128 / 128** pass on net10 (22 pre-existing
skips). Includes the RoleManagementClient and
MonitoredItemAliasNameRefreshStrategySignAndEncryptTests fixtures.
- `Opc.Ua.InformationModel.Tests`: **1052 / 1052** pass on net10 (24
pre-existing skips).
- `Opc.Ua.Gds.Tests`: **875 / 875** pass on net10 (29 pre-existing
skips).
- `Opc.Ua.SourceGeneration.Core.Tests`: **3514 / 3514** pass on net10 (8
long-running compile-cycle integration tests skipped by the framework).
- No `AnalogItemType.EURange` regression.

## 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.
- [ ] I have added all necessary documentation.
- [x] I have verified that my changes do not introduce (new) build or
analyzer warnings.
- [x] 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.
2026-06-12 15:13:41 +02:00
Marc Schier f66ceffd75 Immutable built in types and code generation (#3534)
Generate stack and application code using source generators
Change built in types to readonly structs
  Update localized text implementation to use JSON text serializer
  Status code with symbolic names
  Immutable node ids
  No boxing for all built in types <= 8 bytes in Variant.
Split user identity token and token handling via token handler
Add migration guide for breaking changes
2026-02-12 16:27:07 +01:00