## What this is Design analysis for `Opc.Ua.Server`, the two rounds of design exploration behind it, and the first two implementation chunks it recommends. Draft: the analyses are ready to read, but the *conclusions* deserve discussion before the larger phases proceed. Later chunks stack onto this PR as they land. | File | Kind | Lines | |---|---|---:| | `plans/NodeManagerAnalysis.md` | Evidence — the node-manager plugin surface | 488 | | `plans/ServerRuntimeAnalysis.md` | Evidence — the server runtime cluster | 576 | | `plans/ServerDesignExploration.md` | Design round 2 — delivery strategy | 1,394 | | `plans/ServerDesignExplorationAlt.md` | Design round 1 — four interface designs | 2,119 | | `src/Opc.Ua.Types/State/NodeState.cs` | **Code** — node self-guards attributes | | | `src/Opc.Ua.Types/State/BaseVariableState.cs` | **Code** — value accessors take the lock | | | `src/Opc.Ua.Server/NodeManager/AsyncCustomNodeManager.cs` | **Code** — 6 external locks removed | | | `tests/…/NodeStateAttributeConcurrencyTests.cs` | **Code** — concurrency safety net | 449 | `docs/` is untouched — it holds shipped feature documentation, and none of this is shipped API. ## The analyses Counted mechanically over `src`, `samples` and `tests`; **every figure is reproducible** from commands printed in each document. **Node manager surface.** `AsyncCustomNodeManager` declares **93 `virtual`/`abstract` members** (80 distinct names). Across the repository: **112 override sites, 25 distinct names ever overridden, 55 of 80 never**. Six carry nearly all real use. For scale, `ReferenceNodeManager` is 6,373 lines and overrides **8**. **Server runtime.** Seven findings, **two of which recommend no change**. Findings 1–5 are filed as #4183–#4187. The headline is an `object`-typed sync root published on three public interfaces and taken in **88 `lock` statements across 7 files**, including a shipped sample. **What is deliberately not flagged.** `IStandardServer` — 9 members over 5,575 lines, 16 of 37 virtuals used — is identified as the reference shape to preserve. `IApplicationInstance` and the already-`[Obsolete]` `Stop()` are examined and left alone. ## The design exploration **Round 1** explored four alternative interfaces in parallel under the current back-compat constraint. Its value is the convergence: four designers who never saw each other's work independently arrived at a 6–9 member `IServerContext` replacing the 57-member locator, removal of the `object` node-manager handle, elimination of the `Processed` protocol, and `IHistorianProvider` as the pattern to copy. **Round 2** re-runs it with back-compat lifted and asks how to deliver: full rewrite versus side-by-side fork. **Both option designers independently declined to recommend their own assignment.** Three structural facts were verified: 1. **The fork line already sits at `IAsyncNodeManager`** — old and new node managers can coexist with zero runtime changes, so a fork duplicates ~10% of the assembly, not all of it. 2. **`[OpcUaShim]` cannot shim an inheritance surface** — honest auto-fix ceiling for the authoring surface is **~10%**, against ~70% for the shipped analyzers. 3. **A second lock leak exists** — `NodeState` takes `lock (this)` under a `CA2002` suppression whose own comment documents the contract. Recommendation: **neither pure option**, but a staged path reaching the rewrite end state incrementally, gated on migrating the five framework-internal node managers that today make it impossible for the old base to ever freeze. ## Implementation: NodeState now guards its own attributes This removes the prose-only contract in which `NodeState` took `lock(this)` and every external caller was required to take `lock(source)` on the node instance. The code already carried a TODO describing exactly this fix and noting it required updating every external site. - The node self-guards rather than relying on callers. `BaseVariableState` gains a **fully private** `System.Threading.Lock` covering its value fields; `NodeState`'s other collections keep their existing private locks. - All **6** `lock(this)` sites and both `CA2002`/`RCS1059` suppressions are gone. - `Value`, `Timestamp` and `StatusCode` on `BaseVariableState` take the lock too — without that, a direct property write raced an attribute read once the external lock was gone. - **6 of 9** external sites removed. Five were already redundant (`GetReferences`, `GetChildren`, `ReferenceExists` and `SetAreEventsMonitored` guard themselves; `FindMethod` reaches its state via `GetChildren`); the sixth was an attribute read now covered by the node's own lock. - Attribute read/write deliberately take **no** node-wide lock: both dispatch to caller-supplied handlers (`OnReadValue` and siblings) that may take locks of their own, and holding a lock on the node across one gives the node no fixed place in the lock order. - **3 deliberately retained and annotated in code**: two `CreateBrowser` calls, which are not internally synchronized (tracked in #4216), and one in `CustomNodeManager`, whose class is slated for deletion so refactoring its lock would be wasted work. ### No lock is exposed anywhere, to anyone An earlier revision put the attribute lock on `NodeState` as `private protected`, which still published a lock-sharing contract on the inheritance surface. Review pushed back and it is gone: `BaseVariableState` was the only user, so the lock moved there as a plain `private readonly Lock`. There is no member for a derived type to call, in this assembly or any other. `BaseVariableValue` was the last public leak — it handed out the lock guarding its value through a `public Lock` property, so any caller could hold it across a call back into the stack. That property is removed: - Derived value classes (what the source generator emits for every structure variable) synchronize through a `protected EnterLock()` / `ExitLock()` pair. Callback helpers were the first choice, but the generated bodies pass `ref Variant` / `ref StatusCode` / `ref DateTimeUtc`, and `ref` arguments cannot be captured by a lambda. The lock *object* stays private either way. - `NodeStateTemplates` emits the new form. - `ServerInternalData`, the one cross-assembly caller, was reading the lock back off a value it had itself constructed with that lock. It now takes `m_diagnosticsLock` directly. `NodeBrowser.DataLock` remains `protected` and is tracked in #4215: a subclass extending `Next()` legitimately needs its enumeration state atomic with the base browser's, so the fix needs a base-owned acquisition rather than a deletion. ### Migration coverage for the removed lock members None of the removed lock members get an `[Obsolete]` shim, and the migration guide now says why: a shim would have to be `object`-typed to keep old call sites compiling, and on .NET 9+ `lock (o)` on a statically-`object` reference is `Monitor.Enter`, which does **not** interlock with `Lock.Enter()`. It would compile, run, and silently synchronize nothing — strictly worse than a compile error. Re-adding a member to a downstream-implemented interface is also a break in its own right on `net48` / `netstandard2.0`, where there are no default interface methods. Instead, every removed member has a named replacement on the migration path: | Rule | Covers | Replacement it names | |---|---|---| | `UA0024` | `IServerInternal` / `ISession` / `ISubscription` `DiagnosticsLock`, `DiagnosticsWriteLock` | `UpdateServerDiagnostics(...)` / `UpdateDiagnostics(...)`, `ReadServerDiagnostics` / `ReadDiagnostics` | | `UA0025` | `ILocalNode.DataLock`, `Node.DataLock` | a lock the caller owns; the node guards its own state | | `UA0026` | `BaseVariableValue.Lock` | a lock passed to the `BaseVariableValue` constructor | Each rule fires on sources still written against 1.5.378, where the compiler only says a member is missing. `docs/MigrationGuide.md` gains a section per removal with before/after. ### The tests earned their keep The four concurrency tests were landed **first**, against the unmodified code, because removing a synchronization contract cannot be verified by the compiler and the existing `NodeStateHandlerConcurrencyTests` covers a different concern (racing a handler assignment against a write, not reader/writer exclusion). They now run with **no external locking**, which is what demonstrates the node guards itself. They caught two real problems, and one of them was mine: - The `Value`/`StatusCode`/`Timestamp` setters bypassed the new lock — a genuine race. - One test asserted an invariant the design does not offer. Two property setters are two lock acquisitions, so that pair was **never** atomic; it only passed originally because the test held a lock across both. It now uses a single attribute write carrying a whole `DataValue`, which genuinely is atomic and is the path the server uses. A third issue was found and fixed before it shipped: the reader loops gated on `!writer.IsCompleted`, so a reader scheduled after the writer finished performed zero reads — failing two runs in three on `net48`. Converted to `do…while`; now five consecutive green runs. **Verified:** `Stack.State` 116 passed on `net10.0` *and* `net48`; the concurrency category green 5/5 consecutive on `net48`; `Opc.Ua.Types.Tests` 8,456 passed; the server node-manager suite 1,205 passed, including `NodeManagerLifecycleTests` and `AsyncCustomNodeManagerTests` which exercise the changed paths. ## Two corrections to my own published figures Both were found while starting implementation, and both are recorded on #4183. **`ForceDiagnosticsScan()` was overstated.** The analysis claimed the `DiagnosticsWriteLock` getter *"triggers a full address-space diagnostics rescan"*. It is in fact a single flag assignment (`DiagnosticsNodeManager.cs:700-703`) consumed by the read paths and the periodic timer. The defect stands — a side-effecting property getter with a racy write on a synchronisation path — but its cost was misstated. **The contract was 15 sites, not 19.** A naive search matched ten comment and `#pragma` lines that *mention* `lock(source)` while explaining it. The documents now carry a counting command that excludes comment matches. A third finding expanded scope the other way: the `object`-typed lock leak is **repo-wide**, with ten such members across `Opc.Ua.Core`, `Opc.Ua.Types` and `Opc.Ua.Server` — including `Node.DataLock => this` and `ApplicationConfiguration.PropertiesLock => m_properties`, the same defect class as `Subscription.DiagnosticsLock`. ## Caveats stated in the documents - **Nothing in the designs is implemented or benchmarked.** Every performance claim across all seven designs is a design intention, not a measurement. - The larger phases are a **major undertaking against a live plugin API**; the documents say so rather than letting the neatness of the proposals imply otherwise. - The recommended sequencing puts low-risk, self-contained steps first and notes they are **worth doing regardless** of which strategy is eventually chosen. - Round 2 notes the two option designers' cost estimates are **not independent of their assignments**. ## Review guidance The two analyses make factual claims checkable by running the included commands. The design explorations are exploratory and need not be agreed with to merge. The code change is small, test-backed, and independent of which delivery strategy is eventually chosen. Reasonable outcomes include merging as-is, merging only the analyses and the lock fix, or disagreeing with the recommendation while keeping the evidence. --- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: e9c13dd0-a6e3-4ad2-8ad6-1c3fe6323041 Copilot-Session: d18f87e9-2484-44ce-98ce-463ba6cf5f63
24 KiB
Node Manager Seam Analysis
Status: design analysis / proposal — not shipped API. This document records an evidence-based assessment of the node-manager authoring interface in
Opc.Ua.Server. It describes what the surface looks like today and proposes a narrower shape. Nothing here has been implemented. For the current, shipped node-manager documentation see NodeManagers.md.
Analysis performed against commit e73e71184 on master.
Table of contents
- Purpose
- Vocabulary
- Method
- Finding 1: the assembly is bimodal
- Finding 2: the node-manager seam is the outlier
- Finding 3: the interface seam and the real seam are in different places
- The deletion test
- Full classification of the 93 virtual members
- Dependency category and testing strategy
- Proposed target shape
- Constraints and risks
- Open questions
- Reproducing the evidence
Purpose
Opc.Ua.Server is roughly 145,000 lines. Most of it is organised into modules with
small interfaces and large implementations. The node-manager authoring surface is not:
it presents 93 extension points to buy roughly 5 that callers use.
This document quantifies that gap so the decision to re-cut the seam can be made on evidence rather than impression. It deliberately does not propose moving implementation code. The implementation is good and earns its keep; only the interface is mis-shaped.
Vocabulary
The analysis uses deep-module design terms consistently:
| Term | Meaning here |
|---|---|
| Module | Anything with an interface and an implementation — a class, an interface, a package. |
| Interface | Everything a caller must know to use a module correctly: the type signature plus invariants, ordering constraints, error modes, and required cooperation protocols. |
| Implementation | The body of code inside the module. |
| Depth | Leverage at the interface — how much behaviour a caller gets per unit of interface they must learn. |
| Seam | The place where a module's interface lives; where behaviour can be altered without editing in that place. |
| Adapter | A concrete thing satisfying an interface at a seam. |
Two rules are applied throughout:
- The deletion test. Imagine deleting the module. If complexity vanishes, it was a pass-through. If complexity reappears across N callers, it was earning its keep.
- One adapter means a hypothetical seam. Two adapters means a real one.
Method
Evidence was gathered mechanically over src, samples, and tests, excluding
obj and bin:
- Every
public/protectedvirtual/abstractdeclaration inAsyncCustomNodeManager.cswas enumerated. - Every
overridesite in any class deriving fromAsyncCustomNodeManager,FluentNodeManagerBase, orCoreNodeManagerwas counted and attributed to one of four consumer categories. The scan is textual, so its output is reviewed for synchronous-twin members and regex false positives before use. - Each virtual member was cross-referenced against the interface declarations in
INodeManager.csto separate interface implementations from pure extension hooks.
Consumer categories:
| Category | Meaning |
|---|---|
core |
Inside src/Opc.Ua.Server — framework-internal, an internal seam |
sib |
Other src libraries (GDS, PubSub, WotCon, XRegistry, Positioning) — first-party callers |
sample |
samples/ — representative of external authors |
test |
tests/ — may indicate testing past the interface |
Commands are listed under Reproducing the evidence.
Finding 1: the assembly is bimodal
Opc.Ua.Server already contains several genuinely deep modules. These are the
reference shape and should be left alone:
| Module | Interface size | Behind the seam |
|---|---|---|
IHistorianProvider |
2 members + narrow capability interfaces | ~5,900 lines; HistorianDispatcher (2,370 lines) routes |
ILocalAddressSpace |
7 members, invariants documented in XML docs | node graph; two real adapters (PredefinedNodes, dictionary-backed test double) |
IMonitoredItemManager |
7 members | sampling groups, queues, queue handlers |
IFileSystemProvider |
11 members | 4,145 lines |
IHistorianProvider is worth reading as the model: an umbrella of two members
(IsHistorizingAsync, GetCapabilitiesAsync) with providers opting into Part 11
features through narrow capability interfaces (IHistorianDataProvider,
IHistorianAtTimeProvider, IHistorianEventProvider, and so on). See
HistoricalAccess.md.
Finding 2: the node-manager seam is the outlier
Interface inventory
NodeManager/INodeManager.cs is 1,158 lines and declares 22 public interfaces:
| Interface | Members | Notes |
|---|---|---|
INodeManager |
23 | Mandatory core |
INodeManager2 |
+3 | Version-suffixed extension |
INodeManager3 |
+4 | Version-suffixed extension |
IAsyncNodeManager |
~20 | Parallel async family |
INodeManagerFactory / IAsyncNodeManagerFactory |
2 each | Construction |
15 × I*AsyncNodeManager |
1–5 each | Opt-in capability interfaces — the correct pattern |
The seam has been widened twice by version suffix (2, 3) rather than re-cut.
Base-class surface
| Type | Lines | public | protected | virtual/abstract |
|---|---|---|---|---|
AsyncCustomNodeManager |
8,028 | 59 | 92 | 93 |
CustomNodeManager2 |
6,302 | 52 | 86 | 93 |
MasterNodeManager |
7,601 | 86 | 21 | 39 |
The 93 virtual/abstract declarations resolve to 80 distinct names (the remainder are overloads). That figure — 93 extension points — is the effective interface presented to the 33 classes deriving from these bases.
What callers actually use
Across src, samples, and tests there are 112 override sites. The raw scan
reports 30 distinct names; five are discounted — LoadPredefinedNodes,
OnSubscribeToEvents, and OnNodeRemoved are the synchronous twins declared on
CustomNodeManager2 rather than AsyncCustomNodeManager, and if plus
CreateSessionServerSignature are regex false positives. That leaves 25 distinct
names of the 80. 55 of the 80 names are never overridden anywhere.
Full override census, all consumer categories:
| Member | Total | core | sib | sample | test |
|---|---|---|---|---|---|
CreateAddressSpaceAsync |
21 | 5 | 9 | 5 | 2 |
Dispose |
14 | 4 | 5 | 4 | 1 |
New |
12 | 3 | 5 | 3 | 1 |
LoadPredefinedNodesAsync |
11 | 1 | 8 | 1 | 1 |
DeleteAddressSpaceAsync |
8 | 2 | 4 | 1 | 1 |
AddBehaviourToPredefinedNodeAsync |
5 | 1 | 3 | 1 | 0 |
OnMonitoredItemCreated |
4 | 2 | 1 | 1 | 0 |
OnMonitoredItemDeletedAsync |
3 | 1 | 1 | 1 | 0 |
GetHistorianProvider |
3 | 0 | 0 | 2 | 1 |
GetManagerHandleAsync |
3 | 1 | 1 | 1 | 0 |
ValidateNodeAsync |
3 | 1 | 1 | 1 | 0 |
OnMonitoringModeChangedAsync |
3 | 1 | 1 | 1 | 0 |
OnSubscribeToEventsAsync |
2 | 1 | 0 | 0 | 1 |
ConditionRefreshAsync |
2 | 0 | 0 | 1 | 1 |
OnMonitoredItemModifiedAsync |
2 | 0 | 1 | 1 | 0 |
OnNodeRemovedAsync |
2 | 1 | 0 | 0 | 1 |
SubscribeToAllEventsAsync |
1 | 0 | 0 | 0 | 1 |
SessionClosingAsync |
1 | 1 | 0 | 0 | 0 |
SessionActivatedAsync |
1 | 0 | 0 | 0 | 1 |
ReadAsync |
1 | 0 | 0 | 0 | 1 |
AddPredefinedNodeAsync |
1 | 1 | 0 | 0 | 0 |
AddReferencesAsync |
1 | 1 | 0 | 0 | 0 |
DeleteReferenceAsync |
1 | 1 | 0 | 0 | 0 |
CallAsync |
1 | 0 | 0 | 1 | 0 |
ValidateViewDescription |
1 | 1 | 0 | 0 | 0 |
Representative consumers:
| Consumer | Lines | Overrides |
|---|---|---|
ReferenceNodeManager (sample) |
6,373 | 8 |
FluentNodeManagerBase |
280 | 4 |
Six members account for the overwhelming majority of all override sites:
CreateAddressSpaceAsync, Dispose, New, LoadPredefinedNodesAsync,
DeleteAddressSpaceAsync, AddBehaviourToPredefinedNodeAsync.
This is the depth deficit. The implementation is genuinely deep — roughly 22,000 lines of browse, read, write, call, history, and monitored-item machinery across the three base classes. The interface charges an author 93 extension points to buy the handful they need. Leverage per unit of interface learned is poor.
Interface complexity outside the type signature
Under the definition used here, the interface includes every fact a caller must know. Several load-bearing contracts on this seam are enforced only by prose:
- Opaque handles.
object? GetManagerHandle(NodeId)returns an untyped handle the caller must round-trip correctly. This also conflicts with the repository rule againstobjectin public API. - The
Processedflag protocol. "The node manager must ignore ReadValueId with the Processed flag set to true. The node manager must set the Processed flag for any ReadValueId that it processes." This cooperative-multiplexing contract is restated six times inINodeManager.csand is not expressible in the type system. - Index-aligned accumulators.
Read,Write,Call,HistoryReadand friends take parallelIList<T>value and error collections that the caller pre-sizes and the implementer must index-align. - Pre-populated outputs.
Browsetakesref ContinuationPointand documents that "the references parameter may already contain references when the method is called", which the implementer must account for when deciding whether to return a continuation point.
Finding 3: the interface seam and the real seam are in different places
- Across the entire repository there is exactly one direct implementation of
INodeManageroutside the framework:SampleNodeManagerinsamples/Quickstarts.Servers. By the one-adapter rule, theINodeManagerseam is hypothetical. All 33 real node managers derive from the base classes instead. IAsyncNodeManagerexposesSyncNodeManager : INodeManager— the async interface publishes the sync adapter through itself, letting callers cross back over the seam.- The two bridging adapters (
AsyncNodeManagerAdapter, 915 lines;SyncNodeManagerAdapter) span a technology duality (sync versus async), not a domain one. - Internally,
NodeManagerRoutingTableisIReadOnlyList<IAsyncNodeManager>. Everything is already normalised to the async interface before dispatch.
The deletion test
| Module | Delete it | Verdict |
|---|---|---|
AsyncCustomNodeManager |
Complexity reappears in all 33 subclasses | Earns its keep. The implementation is not the problem. |
INodeManager / 2 / 3 |
Little reappears — routing already runs on IAsyncNodeManager |
Compatibility surface, not a load-bearing seam. |
The 11 History*Async virtuals |
Nothing reappears — they are handle loops delegating into HistorianDispatcher.Dispatch*Async |
Pure pass-through. |
MasterNodeManager |
Complexity reappears, but much of it is caused by the seam below it (fan-out, Processed multiplexing, handle resolution) |
Shrinks if the seam narrows; do not attack directly. |
Full classification of the 93 virtual members
The 80 distinct names partition exactly into five buckets, verified with no gaps, duplicates, or phantoms.
| Bucket | Count | Action |
|---|---|---|
| K — Keep at seam | 7 | The authoring interface |
| O — Observer hooks | 9 | Collapse into one opt-in interface |
| C — Capability interface | 26 | Remove virtual; keep the existing opt-in interfaces |
| D — Convert to DI | 2 | Replace inheritance-based provider selection with injection |
| H — Hide | 36 | Make private or sealed |
| Total | 80 |
K — Keep at seam (7)
The members that actually carry authoring weight.
| Member | Overrides | Note |
|---|---|---|
CreateAddressSpaceAsync |
21 | Primary authoring entry point |
Dispose |
14 | IDisposable; comes free, not part of the node-manager seam |
New |
12 | NodeId factory (INodeIdFactory) |
LoadPredefinedNodesAsync |
11 | NodeSet loading |
DeleteAddressSpaceAsync |
8 | Teardown |
AddBehaviourToPredefinedNodeAsync |
5 | Attach behaviour to loaded nodes |
NamespaceUris |
0 | Set through the constructor; should be non-virtual |
Discounting Dispose (inherited from IDisposable) and NamespaceUris (constructor
state), the genuine authoring seam is 5 members.
O — Observer hooks (9)
Every member here answers "tell me when X happened". Nine separate virtuals buy one concept.
| Member | Overrides |
|---|---|
OnMonitoredItemCreated |
4 |
OnMonitoredItemDeletedAsync |
3 |
OnMonitoringModeChangedAsync |
3 |
ValidateNodeAsync |
3 |
OnMonitoredItemModifiedAsync |
2 |
OnSubscribeToEventsAsync |
2 |
OnNodeRemovedAsync |
2 |
SessionClosingAsync |
1 |
SessionActivatedAsync |
1 |
Proposal: one opt-in INodeManagerObserver, resolved through DI, keeping the mandatory
seam at 5 members.
C — Capability interface (26)
Every member in this bucket is already declared on an I*AsyncNodeManager
capability interface. The base class then re-exposes each one as virtual, so an
author meets the same decision twice — once as "implement this interface", once as
"override this method". Only 7 of the 26 are ever overridden, mostly once.
ReadAsync · WriteAsync · CallAsync · TranslateBrowsePathAsync ·
HistoryReadAsync · HistoryUpdateAsync · ConditionRefreshAsync ·
SetMonitoringModeAsync · CreateMonitoredItemsAsync · ModifyMonitoredItemsAsync ·
DeleteMonitoredItemsAsync · TransferMonitoredItemsAsync ·
RestoreMonitoredItemsAsync · AddNodeAsync · DeleteNodeAsync ·
AddReferenceAsync · AddReferencesAsync · DeleteReferenceAsync ·
AllowNodeManagement · GetManagerHandleAsync · SubscribeToEventsAsync ·
SubscribeToAllEventsAsync · ValidateRolePermissionsAsync · IsNodeInView ·
IsNodeInViewAsync · FindMethodStateAsync
Proposal: drop the virtual modifier. The capability interfaces already provide the
opt-in route and are documented in NodeManagement.md and
AsyncServerSupport.md.
D — Convert to dependency injection (2)
| Member | Overrides | Note |
|---|---|---|
GetHistorianProvider |
3 (2 sample, 1 test) | Returns null by default; composed as HistorianDispatcher.ResolveProvider(Server, node, GetHistorianProvider(node)) |
HasHistorianProvider |
0 | Derived from the above |
This is provider selection by inheritance. Samples override it because there is no injection point at that layer. The repository convention is a provider model with injectable providers, so this should become an injected resolver.
H — Hide (36)
34 of these have zero overrides. Two (AddPredefinedNodeAsync,
ValidateViewDescription) are overridden only inside Opc.Ua.Server itself — internal
seams that should not surface at the authoring interface.
History fan-out (11) — verified to be handle loops delegating straight into
HistorianDispatcher.Dispatch*Async, with the real behaviour behind the 2-member
IHistorianProvider. Roughly 1,000 lines of pass-through published as 11 extension
points that nobody uses.
HistoryReadRawModifiedAsync · HistoryReadAtTimeAsync · HistoryReadProcessedAsync ·
HistoryReadEventsAsync · HistoryUpdateDataAsync · HistoryUpdateEventsAsync ·
HistoryUpdateStructureDataAsync · HistoryDeleteRawModifiedAsync ·
HistoryDeleteAtTimeAsync · HistoryDeleteEventsAsync ·
HistoryReleaseContinuationPointsAsync
Monitored-item internals (13) — IMonitoredItemManager (7 members) is already the
correct seam for these.
CreateMonitoredItemAsync · ModifyMonitoredItemAsync · DeleteMonitoredItemAsync ·
RestoreMonitoredItem · ReadInitialValue · ValidateMonitoringFilterAsync ·
ReviseAggregateFilterAsync · ValidationComplete · OnCreateMonitoredItemsComplete ·
OnDeleteMonitoredItemsCompleteAsync · OnModifyMonitoredItemsCompleteAsync ·
OnSetMonitoringModeCompleteAsync · OnMonitoredItemsTransferredAsync
Note the smell inside this group: the batch On*Complete family has zero overrides
while the singular On*Created / On*Deleted / On*Modified family (bucket O) has
nine. Two parallel hook families exist; only one is used.
Namespace and view predicates (5)
IsHandleInNamespace · IsNodeIdInNamespace · IsReferenceInView ·
FindNodeInCacheAsync · ValidateViewDescription
Node-graph internals (7)
AddPredefinedNodeAsync · RemovePredefinedNodeAsync · AddReverseReferencesAsync ·
AddRootNotifierAsync · RemoveRootNotifierAsync · OnReportEvent ·
OnReportEventAsync
Dependency category and testing strategy
Classifying the dependencies determines how much work re-cutting the seam actually is.
| Dependency | Category | Consequence |
|---|---|---|
Node graph (NodeState, in-memory dictionaries) |
In-process | Deepenable directly; no port needed |
| Historian | Already behind IHistorianProvider |
No new seam required |
| File system | Already behind IFileSystemProvider |
No new seam required |
| Distributed address space | Already behind ILocalAddressSpace |
No new seam required |
Because the core is in-process and every external dependency already sits behind its own provider seam, no new port is required. The deepened module is tested directly through its narrowed interface. This is the cheap case.
Testing should replace, not layer. Tests that drive 93 virtuals by subclassing are testing past the interface and will not survive the change:
| Test file | Lines |
|---|---|
NodeManagerLifecycleTests.cs |
8,406 |
AsyncCustomNodeManagerTests.cs |
5,821 |
MasterNodeManagerNodeManagementTests.cs |
2,499 |
MasterNodeManagerDeterministicTests.cs |
1,639 |
CustomNodeManagerDeterministicTests.cs |
1,355 |
New tests should assert observable outcomes through the narrow interface so they survive internal refactors.
Proposed target shape
Nothing moves. The 8,028 lines of AsyncCustomNodeManager stay exactly where they are.
Only the interface is re-cut:
- 5 mandatory members for authoring a node manager.
- 1 opt-in
INodeManagerObserverreplacing the 9 scattered notification hooks. - The 15 existing
I*AsyncNodeManagercapability interfaces remain the opt-in route for service-set behaviour, with the duplicate base-class virtuals removed. - 2 provider hooks converted to injection.
- 36 members made private or sealed.
Net: 93 extension points → 5 mandatory members, with the same behaviour behind them.
The strongest argument for the shape is that this assembly already contains the answer
twice. IHistorianProvider (2 members over ~5,900 lines) and IMonitoredItemManager
(7 members) demonstrate the target pattern, and they sit directly alongside the 24
virtuals that duplicate them.
Constraints and risks
- Backward compatibility. The repository requires compatibility with 1.5.378
(
master378); replaced API must be marked[Obsolete]rather than removed. This makes the change additive.INodeManager.cscurrently contains zero[Obsolete]markers, so the deprecation pass has not begun. The existingSyncNodeManagerAdapterandAsyncNodeManagerAdapterare the natural compatibility mechanism. FluentNodeManagerBaselayers rather than replaces. It derives fromAsyncCustomNodeManagerand therefore inherits all 93 members, addingEventSourceRegistryandSimulationRegistryon top. It would need re-basing onto the narrow seam rather than stacking on the wide one.- Source generators emit against this surface. The generated
NodeManagerBasederives fromFluentNodeManagerBase; generator templates must move in lockstep. See NodeManagers.md. SampleNodeManagerimplementsINodeManagerdirectly and is the one external adapter for that interface. It either migrates or stays as the demonstration of the obsolete path.
Open questions
- Should the 5-member authoring seam be an interface (
INodeSource-style, favouring composition) or remain an abstract base class (favouring the existing 33 subclasses)? This is the main design decision and is worth exploring more than once. - Should
GetManagerHandle's opaqueobjecthandle be replaced with a typed handle struct, and can that be done without breaking theProcessed-flag multiplexing thatMasterNodeManagerdepends on? - Can the
Processedcooperative-multiplexing protocol be replaced by explicit partitioning inMasterNodeManager, removing the contract from the seam entirely? - Does the sync/async duality still need two adapters, or can
INodeManagerbecome a pure[Obsolete]compatibility shim overIAsyncNodeManager?
Reproducing the evidence
Run from the repository root in PowerShell.
Enumerate the virtual/abstract surface:
$lines = Get-Content src\Opc.Ua.Server\NodeManager\AsyncCustomNodeManager.cs
($lines | Select-String '^ (public|protected)(\s+internal)?\s+(virtual|abstract)\s+').Count
Count the override census by consumer category:
$files = Get-ChildItem 'src','samples','tests' -Recurse -Filter *.cs -File |
Where-Object { $_.FullName -notmatch '\\obj\\|\\bin\\' }
$rows = @()
foreach ($f in $files) {
$t = Get-Content -LiteralPath $f.FullName -Raw
if ($t -notmatch ':\s*(AsyncCustomNodeManager|FluentNodeManagerBase|CoreNodeManager)\b') { continue }
$rel = $f.FullName.Replace("$PWD\", '')
$cat = if ($rel -like 'src\Opc.Ua.Server\*') { 'core' }
elseif ($rel -like 'src\*') { 'sib' }
elseif ($rel -like 'samples\*') { 'sample' }
else { 'test' }
foreach ($m in [regex]::Matches($t, '\boverride\s+[\w\<\>\?\[\]\.,\s]{0,80}?(\w+)\s*[\(\{]')) {
$rows += [PSCustomObject]@{ Name = $m.Groups[1].Value; Cat = $cat }
}
}
$rows | Group-Object Name | Sort-Object Count -Descending
Count direct INodeManager implementations outside the framework:
Get-ChildItem 'src','samples','tests' -Recurse -Filter *.cs -File |
Where-Object { $_.FullName -notmatch '\\obj\\|\\bin\\' } |
Select-String -Pattern '(class|record)\s+\w+\s*:[^{]*\bINodeManager\b'
See also
- Node Managers — the current, shipped node-manager guide.
- Async Server Support — the async capability-interface pattern.
- NodeManagement Service Set —
INodeManagementAsyncNodeManageropt-in. - Historical Access (Part 11) — the
IHistorianProviderprovider model used here as the reference shape. - Dependency Injection — the injection surface the provider hooks would move to.
- Developer Guide — coding standards referenced by this analysis.