mirror of
https://github.com/OPCFoundation/UA-.NETStandard.git
synced 2026-10-20 16:52:07 +08:00
668310f21f
# Description The fluent node-manager pipeline had two synchronous chokepoints, and two independent efforts have now hit both of them. `Configure` is a `partial void`, so it cannot `await`. A manager whose wiring depends on asynchronous setup had to invent a second hook outside the pipeline, create its own builder there, and hand-roll `CreateFluentBuilder(...).Configure(...).Seal()` inside it. `Seal()` was synchronous, so nothing a registration staged during `Configure` could be completed by awaiting. Registrations that needed to await either blocked — `EventSourceRegistry` did `.GetAwaiter().GetResult()` on the manager's root-notifier registration — or could not be expressed at all. ## What changed **`FluentNodeManagerBase.ConfigureAsync(INodeManagerBuilder, CancellationToken)`** is the awaitable wiring seam. It runs once per activation, with the manager's attached builder, immediately *before* the synchronous `Configure` partial(s) — so instances it materialises are in the address space by the time `Configure` wires callbacks against them. The default implementation is a no-op. It is a `virtual` method rather than a second `partial` declaration because a partial method can be optional (`partial void`, no return value) or awaitable (an extended partial method, which must be implemented) — not both, and the generator would otherwise have to detect which one the user declared. `partial void Configure(INodeManagerBuilder)` keeps working unchanged; a manager uses either hook or both. **`NodeManagerBuilder.Seal()` becomes `SealAsync(CancellationToken)`.** Sealing is now the single point where registrations that could not finish inside `Configure` get their turn to await. Every seal site awaits it, so a later change can move registry *setup* onto the same seam. **`DiNodeManager` now runs the standard pipeline itself** — create and attach the builder, `await ConfigureAsync`, `RegisterAuthoredNodesAsync`, `CompleteConfigureAsync`, the post-setup runner, then `await builder.SealAsync` — instead of handing subclasses a bare callback and leaving them to build and seal on their own. DI subclasses gain node registration and the reverse-reference pass, which they previously skipped. Sealing deliberately comes **after** the post-setup runner. The fluent registries are owned by the manager, not by a single builder, and sealing starts them — so sealing first locks post-setup configurators out of registering simulation loops of their own, which is exactly what `AddRobotics`' configurators do. Sealing before the runner makes `MinimalRobotServer` fail to start with *"Cannot add a simulation loop after the registry has started"*. Worth knowing for follow-up work: with several builders attached to one manager, the first seal freezes the shared registry for all of them. ## Workarounds removed - **`DiNodeManager.OnAddressSpaceReadyAsync`** — deleted. Its three overriders (`RoboticsNodeManager`, and the `PumpNodeManager` / `GeneratorNodeManager` samples) now override `ConfigureAsync` and no longer hand-roll `CreateFluentBuilder(...).Configure(Configure).Seal()`. - **`EventSourceRegistry`'s `.GetAwaiter().GetResult()`** on `AddRootNotifierAsync` — gone. `Publish(..., RegisterAsRootNotifier: true)` now stages the registration and `SealAsync` drains it, so the wait happens on an awaited path. A source whose root-notifier registration fails is rolled out of the registry before the failure surfaces, and a registry disposed before its builder sealed drops what it staged. - **`Isa95NodeManager.ConfigureStatusEvents()`** — the synchronous second builder pass is now `ConfigureStatusEventsAsync(CancellationToken)`, awaited from `CreateAddressSpaceAsync`. (`ConfigureCatalogChanges()` stays synchronous: it starts a background task and never seals a builder, so there was nothing to convert.) - **`IRoboticsBuildContext.Seal()`** — now `SealAsync(CancellationToken)`. These really were `NodeManagerBuilder.Seal()` calls behind a same-named method. The state transition still happens under the context lock; the builder's asynchronous completion runs outside it, so no `await` is taken while the lock is held. - The Pump / Generator incremental registration callbacks (`onRegistered`) changed from `Action<T>` to `Func<T, CancellationToken, ValueTask>` so their per-instance builder pass can await its own seal. No sync-over-async was introduced anywhere. `Dispose(bool)` is unchanged and stays non-blocking; teardown remains in `DeleteAddressSpaceAsync`. `NodeManagerBuilder.Seal()` and `IRoboticsBuildContext.Seal()` are removed rather than kept as synchronous overloads. A synchronous seal that silently skipped the asynchronous completion would reintroduce exactly the failure mode this change exists to remove. ## Generator `NodeManagerTemplates` emits, in order: predefined nodes load → `await ConfigureAsync(__m_builder, cancellationToken)` → `Configure(__m_builder)` and the typed `Configure` → `RegisterAuthoredNodesAsync` → `CompleteConfigureAsync` → `await SealConfigurationAsync(__m_builder, cancellationToken)`. `NodeManagerGeneratorTests` asserts that order literally, and that neither `__m_builder.Seal()` nor the synchronous `SealConfiguration(__m_builder)` is emitted. ### Merging with #4432 #4432 landed on `master` while this was in review and reworked the same seam from the other side: it split `Seal()` into `SealGraphAuthoring()` + `StartSimulations()` so a manager can replay `NotifyNodeAdded` *between* the two halves — sealing first stops a lifecycle handler authoring nodes nothing would register, starting the simulations last stops a simulated value change preceding its own node''s `OnNodeAdded` — and added `FluentNodeManagerBase.SealConfiguration(builder)` as the one call the generator emits. Both intents are kept. That split is precisely why the asynchronous completion could not simply live inside `SealAsync`: a manager reaching the seam through the split path would silently skip it, which is the failure mode this PR exists to remove. So the asynchronous half became its own step: - `NodeManagerBuilder.CompleteSealAsync(ct)` drains the staged root-notifier registrations, then starts the simulations. `SealAsync(ct)` is `SealGraphAuthoring()` followed by it. - #4432''s synchronous `StartSimulations()` is gone, folded into `CompleteSealAsync`. It had exactly one production caller after the merge, and leaving it standing would have kept a second, synchronous way to activate a builder that skips the staged registrations — the very hazard this PR removes. Activation is now one step with one entry point; the *split* itself stays, because the replay has to sit between the halves. - `SealConfiguration` becomes `SealConfigurationAsync(builder, ct)`: seal the graph, replay `NotifyNodeAdded`, then `CompleteSealAsync`. Activation still happens last, so #4432''s ordering guarantee holds unchanged. ## Consumers unblocked - **#4434 ("Reimplement the GDS ApplicationsNodeManager as a generated fluent node manager")** adds `OnAddressSpaceReadyAsync` to `FluentNodeManagerBase` purely because, in its own words, *"`Configure` is a `partial void` and cannot await"*. That hook is unnecessary once `ConfigureAsync` exists — the GDS manager overrides `ConfigureAsync` instead, and the ordering it wants (predefined nodes → async setup → `Configure`) is what the generator now emits. #4434 has not merged yet, so it can drop its addition rather than have it simplified afterwards. - **Branch `romanett/fluent-node-behaviors`** moved `SimulationRegistry`, `EventSourceRegistry`, `MonitoredSourceRegistry` and the alarm wiring onto `AttachToType`/`Attach` for *release* only, because activation ran from `CompleteConfigureAsync` and most managers only called the synchronous `Seal()`. Every seal site now reaches `SealAsync`, so registry and alarm *setup* can move onto behavior activation too — the remaining half of that work. ## Tests - New `DiNodeManagerConfigureAsyncTests` drives the real `DiNodeManager.CreateAddressSpaceAsync` and asserts the hook runs once with an attached builder, that nodes it stages are registered, that their references to externally owned nodes are published, and that the builder is sealed before `CreateAddressSpaceAsync` returns. - `PublishTests` now pins the root-notifier deferral contract: registration is staged during `Configure`, lands only on the seal, and draining twice does not double-register. - Every `Seal()` call site in the test suites became `await ...SealAsync()`, including the three #4432 added in `NodeSetImportBuilderTests` and `SimulationBuilderExtensionsTests`. ## Related Issues No tracking issue was opened for this; it is the shared prerequisite extracted from #4434 and branch `romanett/fluent-node-behaviors`, both of which had independently worked around the same synchrony. Happy to open one if maintainers want the ADR trail. ## 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. ### What was actually run locally `UA.slnx` builds clean. Post-merge on **net10.0**: `Opc.Ua.Server.Tests` 5020/0 and `Opc.Ua.SourceGeneration.Core.Tests` 3813/0. Pre-merge the same run plus `Opc.Ua.Di.Tests` 425/0, `Opc.Ua.Robotics.Tests` 627/0, `Opc.Ua.ISA95.Tests` 136/0, `Opc.Ua.Positioning.Tests` and `Opc.Ua.Robotics.Intent.Tests` were all green; `MinimalRobotServer` was also started directly to confirm the hosted robotics path comes up. Three caveats, so the unchecked box above is not a mystery: the full suite was **not** run against .NET Framework locally; `Opc.Ua.OpenUsd.Tests` has one pre-existing failure on this machine (`StructuredCartesianCoordinatesAreAccepted` expects `"1.5"` and gets `"1,5"` — a German-locale decimal separator, unrelated to this change); and one pre-existing `CA1861` warning comes from `NodeSetImportIntegrationTests.cs`, which is byte-identical to `master` on this branch. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
345 lines
13 KiB
C#
345 lines
13 KiB
C#
/* ========================================================================
|
|
* Copyright (c) 2005-2026 The OPC Foundation, Inc. All rights reserved.
|
|
*
|
|
* OPC Foundation MIT License 1.00
|
|
*
|
|
* Permission is hereby granted, free of charge, to any person
|
|
* obtaining a copy of this software and associated documentation
|
|
* files (the "Software"), to deal in the Software without
|
|
* restriction, including without limitation the rights to use,
|
|
* copy, modify, merge, publish, distribute, sublicense, and/or sell
|
|
* copies of the Software, and to permit persons to whom the
|
|
* Software is furnished to do so, subject to the following
|
|
* conditions:
|
|
*
|
|
* The above copyright notice and this permission notice shall be
|
|
* included in all copies or substantial portions of the Software.
|
|
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
|
|
* EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES
|
|
* OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
|
|
* NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT
|
|
* HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY,
|
|
* WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING
|
|
* FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR
|
|
* OTHER DEALINGS IN THE SOFTWARE.
|
|
*
|
|
* The complete license agreement can be found here:
|
|
* http://opcfoundation.org/License/MIT/1.00/
|
|
* ======================================================================*/
|
|
|
|
using System;
|
|
using System.Linq;
|
|
using System.Reflection;
|
|
using System.Threading;
|
|
using System.Threading.Tasks;
|
|
using Microsoft.Extensions.DependencyInjection;
|
|
using NUnit.Framework;
|
|
using Opc.Ua.Di.Server;
|
|
using Opc.Ua.Di.Server.Hosting;
|
|
using Opc.Ua.Robotics.Server;
|
|
using Opc.Ua.Server.Fluent;
|
|
using Opc.Ua.Server.Hosting;
|
|
|
|
namespace Opc.Ua.Robotics.Server.Tests
|
|
{
|
|
[TestFixture]
|
|
[Category("Robotics")]
|
|
[Category("Hosting")]
|
|
public sealed class RoboticsServerHostingTests
|
|
{
|
|
[Test]
|
|
public void AddOpcUaDiThenAddRoboticsThrows()
|
|
{
|
|
IServiceCollection services = new ServiceCollection();
|
|
IOpcUaServerBuilder builder = services.AddOpcUa()
|
|
.AddServer(options => options.ApplicationName = "test")
|
|
.AddOpcUaDi();
|
|
|
|
InvalidOperationException exception = Assert.Throws<InvalidOperationException>(
|
|
() => builder.AddRobotics())!;
|
|
|
|
Assert.That(exception.Message, Does.Contain(nameof(OpcUaServerDiBuilderExtensions.AddOpcUaDi)));
|
|
}
|
|
|
|
[Test]
|
|
public void AddRoboticsThenAddOpcUaDiThrows()
|
|
{
|
|
IServiceCollection services = new ServiceCollection();
|
|
IOpcUaServerBuilder builder = services.AddOpcUa()
|
|
.AddServer(options => options.ApplicationName = "test")
|
|
.AddRobotics();
|
|
|
|
InvalidOperationException exception = Assert.Throws<InvalidOperationException>(
|
|
() => builder.AddOpcUaDi())!;
|
|
|
|
Assert.That(exception.Message, Does.Contain(nameof(OpcUaServerRoboticsBuilderExtensions.AddRobotics)));
|
|
}
|
|
|
|
[Test]
|
|
public void AddRoboticsRegistersDiAddressSpaceOwnership()
|
|
{
|
|
IServiceCollection services = new ServiceCollection();
|
|
services.AddOpcUa()
|
|
.AddServer(options => options.ApplicationName = "test")
|
|
.AddRobotics();
|
|
|
|
using ServiceProvider provider = services.BuildServiceProvider();
|
|
DiAddressSpaceOwnership ownership =
|
|
provider.GetRequiredService<DiAddressSpaceOwnership>();
|
|
|
|
Assert.That(
|
|
ownership.OwnerName,
|
|
Is.EqualTo(nameof(OpcUaServerRoboticsBuilderExtensions.AddRobotics)));
|
|
}
|
|
|
|
[Test]
|
|
public async Task ConfigureRoboticsAsyncLambdaReturnsAwaitableValueTask()
|
|
{
|
|
IServiceCollection services = new ServiceCollection();
|
|
var completion = new TaskCompletionSource<bool>();
|
|
IOpcUaServerBuilder builder = services.AddOpcUa()
|
|
.AddServer(options => options.ApplicationName = "test");
|
|
|
|
builder.ConfigureRobotics(async context =>
|
|
{
|
|
_ = context;
|
|
await completion.Task.ConfigureAwait(false);
|
|
});
|
|
|
|
ServiceDescriptor registration = services.Single(descriptor =>
|
|
descriptor.ServiceType.Name == "IRoboticsConfigurationRegistration");
|
|
object instance = registration.ImplementationInstance!;
|
|
MethodInfo configureAsync = instance.GetType().GetMethod("ConfigureAsync")!;
|
|
var context = new FakeRoboticsBuildContext();
|
|
|
|
var task = (ValueTask)configureAsync.Invoke(instance, new object[] { context })!;
|
|
|
|
Assert.That(task.IsCompleted, Is.False);
|
|
completion.SetResult(true);
|
|
await task.ConfigureAwait(false);
|
|
}
|
|
|
|
[Test]
|
|
public async Task ConfigureRoboticsForAsyncLambdaReturnsAwaitableValueTask()
|
|
{
|
|
IServiceCollection services = new ServiceCollection();
|
|
var completion = new TaskCompletionSource<bool>();
|
|
IOpcUaServerBuilder builder = services.AddOpcUa()
|
|
.AddServer(options => options.ApplicationName = "test");
|
|
|
|
builder.ConfigureRoboticsFor<DiNodeManager>(async context =>
|
|
{
|
|
_ = context;
|
|
await completion.Task.ConfigureAwait(false);
|
|
});
|
|
|
|
ServiceDescriptor registration = services.Single(descriptor =>
|
|
descriptor.ServiceType.Name == "IRoboticsConfigurationRegistration");
|
|
object instance = registration.ImplementationInstance!;
|
|
MethodInfo configureAsync = instance.GetType().GetMethod("ConfigureAsync")!;
|
|
var context = new FakeRoboticsBuildContext();
|
|
|
|
var task = (ValueTask)configureAsync.Invoke(instance, new object[] { context })!;
|
|
|
|
Assert.That(task.IsCompleted, Is.False);
|
|
completion.SetResult(true);
|
|
await task.ConfigureAwait(false);
|
|
}
|
|
|
|
[Test]
|
|
public void EmptyProviderListThrowsConfigurationError()
|
|
{
|
|
ServiceResultException exception = Assert.Throws<ServiceResultException>(
|
|
() => new RoboticsNodeManagerFactory(
|
|
ArrayOf<IRoboticsModelProvider>.Empty,
|
|
new RoboticsServerOptions()))!;
|
|
|
|
Assert.That(exception.StatusCode, Is.EqualTo(StatusCodes.BadConfigurationError));
|
|
Assert.That(exception.Message, Does.Contain("At least one Robotics model provider"));
|
|
}
|
|
|
|
[Test]
|
|
public void CustomCoreProviderCanReplaceBuiltInProvider()
|
|
{
|
|
var factory = new RoboticsNodeManagerFactory(
|
|
new IRoboticsModelProvider[] { new CustomCoreProvider() },
|
|
new RoboticsServerOptions());
|
|
|
|
string[] namespaceUris = factory.NamespacesUris.Memory.ToArray();
|
|
Assert.That(
|
|
namespaceUris,
|
|
Does.Contain(Opc.Ua.IA.Namespaces.IA)
|
|
.And.Contain(Opc.Ua.Robotics.Namespaces.Robotics));
|
|
}
|
|
|
|
[Test]
|
|
public void ProviderMissingCoreNamespaceThrowsConfigurationError()
|
|
{
|
|
ServiceResultException exception = Assert.Throws<ServiceResultException>(
|
|
() => new RoboticsNodeManagerFactory(
|
|
new IRoboticsModelProvider[] { new MissingCoreProvider() },
|
|
new RoboticsServerOptions()))!;
|
|
|
|
Assert.That(exception.StatusCode, Is.EqualTo(StatusCodes.BadConfigurationError));
|
|
Assert.That(exception.Message, Does.Contain("collectively advertise"));
|
|
}
|
|
|
|
[Test]
|
|
public void BuiltInProviderUsesEarliestOrder()
|
|
{
|
|
Assert.That(new RoboticsModelProvider().Order, Is.EqualTo(int.MinValue));
|
|
}
|
|
|
|
[Test]
|
|
public void DefaultInstanceNamespaceIsStableProjectNamespace()
|
|
{
|
|
Assert.That(
|
|
RoboticsServerOptions.DefaultInstanceNamespaceUri,
|
|
Is.EqualTo("urn:opcua-netstandard:robotics:instances"));
|
|
Assert.DoesNotThrow(() => new RoboticsServerOptions().Validate());
|
|
}
|
|
|
|
[Test]
|
|
public void StandardModelNamespacesCannotBeInstanceNamespace()
|
|
{
|
|
string[] reservedNamespaces =
|
|
[
|
|
global::Opc.Ua.Namespaces.OpcUa,
|
|
DiNodeManager.DiNamespaceUri,
|
|
Opc.Ua.IA.Namespaces.IA,
|
|
Robotics.Namespaces.Robotics
|
|
];
|
|
|
|
for (int ii = 0; ii < reservedNamespaces.Length; ii++)
|
|
{
|
|
var options = new RoboticsServerOptions
|
|
{
|
|
InstanceNamespaceUri = reservedNamespaces[ii]
|
|
};
|
|
ServiceResultException exception = Assert.Throws<ServiceResultException>(
|
|
() => new RoboticsNodeManagerFactory(
|
|
new IRoboticsModelProvider[]
|
|
{
|
|
new RoboticsModelProvider()
|
|
},
|
|
options))!;
|
|
|
|
Assert.That(
|
|
exception.StatusCode,
|
|
Is.EqualTo(StatusCodes.BadConfigurationError));
|
|
Assert.That(exception.Message, Does.Contain(reservedNamespaces[ii]));
|
|
Assert.That(exception.Message, Does.Contain("application-owned"));
|
|
}
|
|
}
|
|
|
|
[Test]
|
|
public void ProviderNamespaceCannotBeInstanceNamespace()
|
|
{
|
|
var options = new RoboticsServerOptions
|
|
{
|
|
InstanceNamespaceUri = CustomModelProvider.ModelNamespaceUri
|
|
};
|
|
|
|
ServiceResultException exception = Assert.Throws<ServiceResultException>(
|
|
() => new RoboticsNodeManagerFactory(
|
|
new IRoboticsModelProvider[] { new CustomModelProvider() },
|
|
options))!;
|
|
|
|
Assert.That(
|
|
exception.StatusCode,
|
|
Is.EqualTo(StatusCodes.BadConfigurationError));
|
|
Assert.That(
|
|
exception.Message,
|
|
Does.Contain(CustomModelProvider.ModelNamespaceUri));
|
|
Assert.That(exception.Message, Does.Contain(nameof(CustomModelProvider)));
|
|
}
|
|
|
|
[Test]
|
|
public void BuiltInProviderGuardsNullArguments()
|
|
{
|
|
var provider = new RoboticsModelProvider();
|
|
|
|
Assert.That(
|
|
Assert.Throws<ArgumentNullException>(
|
|
() => provider.AddPredefinedNodes(null!, null!))!.ParamName,
|
|
Is.EqualTo("nodes"));
|
|
Assert.That(
|
|
Assert.Throws<ArgumentNullException>(
|
|
() => provider.AddPredefinedNodes(new NodeStateCollection(), null!))!.ParamName,
|
|
Is.EqualTo("context"));
|
|
}
|
|
|
|
private sealed class CustomCoreProvider : IRoboticsModelProvider
|
|
{
|
|
public int Order => 0;
|
|
|
|
public ArrayOf<string> NamespaceUris => new string[]
|
|
{
|
|
Opc.Ua.IA.Namespaces.IA,
|
|
Opc.Ua.Robotics.Namespaces.Robotics
|
|
};
|
|
|
|
public void AddPredefinedNodes(NodeStateCollection nodes, ISystemContext context)
|
|
{
|
|
}
|
|
}
|
|
|
|
private sealed class MissingCoreProvider : IRoboticsModelProvider
|
|
{
|
|
public int Order => 0;
|
|
|
|
public ArrayOf<string> NamespaceUris => new string[]
|
|
{
|
|
Opc.Ua.IA.Namespaces.IA
|
|
};
|
|
|
|
public void AddPredefinedNodes(NodeStateCollection nodes, ISystemContext context)
|
|
{
|
|
}
|
|
}
|
|
|
|
private sealed class CustomModelProvider : IRoboticsModelProvider
|
|
{
|
|
public const string ModelNamespaceUri = "urn:tests:robotics:custom-model";
|
|
|
|
public int Order => 0;
|
|
|
|
public ArrayOf<string> NamespaceUris => new string[]
|
|
{
|
|
Opc.Ua.IA.Namespaces.IA,
|
|
Robotics.Namespaces.Robotics,
|
|
ModelNamespaceUri
|
|
};
|
|
|
|
public void AddPredefinedNodes(NodeStateCollection nodes, ISystemContext context)
|
|
{
|
|
}
|
|
}
|
|
|
|
private sealed class FakeRoboticsBuildContext : IRoboticsBuildContext
|
|
{
|
|
public DiNodeManager Manager => throw new NotSupportedException();
|
|
|
|
public ISystemContext Context => throw new NotSupportedException();
|
|
|
|
public INodeManagerBuilder Nodes => throw new NotSupportedException();
|
|
|
|
public ushort InstanceNamespaceIndex => throw new NotSupportedException();
|
|
|
|
public NodeState DeviceSet => throw new NotSupportedException();
|
|
|
|
public CancellationToken CancellationToken => default;
|
|
|
|
public T GetRequiredService<T>() where T : notnull
|
|
{
|
|
throw new NotSupportedException();
|
|
}
|
|
|
|
public ValueTask SealAsync(CancellationToken cancellationToken = default)
|
|
{
|
|
throw new NotSupportedException();
|
|
}
|
|
}
|
|
}
|
|
}
|