dohertj2/lmxopcua
1
0
Fork 0
Code Issues 32 Pull Requests Actions Packages Projects Releases Wiki Activity

refactor(mesh): collapse MeshPaths to local actor names — no longer a wire contract (review follow-up)

Browse Source 
MeshPaths.CentralCommunication / .NodeCommunication (the "/user/..." full-path constants) had
ZERO production consumers after the ClusterClient deletion: both gRPC services resolve their
comm actor through ActorRegistry by typed key, never by ActorSelection over a path. Only the
path-probe test still read them, while the class doc went on calling them "the wire contract
of the central↔node boundary" — a claim that stopped being true the moment nothing addressed
an actor across a process boundary by name.

  - The two full-path constants are deleted; the two Name constants remain.
  - MeshPaths' doc now says what it is (the local actor-naming convention the registry-keyed
    services are spawned under) and records what it stopped being, so the next reader does not
    re-derive a contract from the file's history.
  - MeshCommActorPathTests composes "/user/" + Name itself — the absolute path is that test's
    own addressing detail now, not something the product declares. Renamed
    Both_mesh_comm_actors_exist_at_their_contract_paths ->
    ..._are_spawned_under_their_declared_names, because what it still pins is that the two
    registration sites actually ran on a real booted node; a rename is now a compile error, so
    "contract" was the wrong word for what would break.
  - NodeCommunicationActor's summary no longer cross-references the deleted constant or calls
    the name an address. CentralCommunicationActor was checked and makes no such claim.
This commit is contained in:
Joseph Doherty
2026-08-06 16:14:25 -04:00
parent 4d2dfb5b50
commit cd7135570c
3 changed files with 62 additions and 37 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs
+27 -17
View File
@@ -1,33 +1,43 @@
namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh;
/// <summary>
/// Actor paths the two mesh comm actors register under. <b>These strings are the wire
/// contract</b> of the central↔node boundary.
/// The <b>local actor names</b> the two mesh comm actors are spawned under. Not a wire contract:
/// nothing off-process addresses either actor by name.
/// </summary>
/// <remarks>
/// <para>
/// Declared once, here, rather than as literals on each side. The <c>deployments</c> /
/// <c>deployment-acks</c> topic names are declared twice — once in
/// <c>ConfigPublishCoordinator</c> and again in <c>DriverHostActor</c> — and agree only by
/// convention; renaming one would compile, deploy, and deliver nothing. A mesh path
/// has the same failure mode and no compiler to catch it, so the constant is shared.
/// <b>This used to be a wire contract and no longer is.</b> While the cross-mesh transport
/// was an Akka <c>ClusterClient</c>, central addressed a node by the literal path string
/// <c>/user/node-communication</c> and the far side resolved it — so the two processes had to
/// agree on a string neither could see the other spell. Both gRPC services now resolve
/// their comm actor through <c>ActorRegistry</c> (<c>CentralCommunicationActorKey</c> /
/// <c>NodeCommunicationActorKey</c>), which is type-checked, so the name below never crosses
/// a process boundary. The full <c>/user/...</c> path constants that carried the old contract
/// are deleted; what remains is a naming convention.
/// </para>
/// <para>
/// Nothing else pins these values, which is why <c>MeshCommActorPathTests</c> boots a real
/// host and <c>Identify</c>-probes both paths.
/// Still declared once, here, rather than as literals at each spawn site — a shared constant
/// costs nothing and keeps the name an operator sees in a log or an <c>ActorSelection</c>
/// attached to one declaration. The registry key, not this string, is what a mis-rename
/// would break, and that failure is a compile error rather than a silent drop.
/// </para>
/// <para>
/// <c>MeshCommActorPathTests</c> boots a real host and <c>Identify</c>-probes
/// <c>/user/{name}</c> on both nodes, which is what pins these values against the two
/// registration sites.
/// </para>
/// </remarks>
public static class MeshPaths
{
/// <summary>Central-side comm actor — where a driver node sends its deployment acks.</summary>
public const string CentralCommunication = "/user/central-communication";
/// <summary>Node-side comm actor — where central sends commands.</summary>
public const string NodeCommunication = "/user/node-communication";
/// <summary>Actor name of <see cref="CentralCommunication"/> (the path's final segment).</summary>
/// <summary>
/// Actor name of the central-side comm actor — where an inbound deployment ack is handed
/// over, after <c>CentralCommandService</c> has resolved it by registry key.
/// </summary>
public const string CentralCommunicationName = "central-communication";
/// <summary>Actor name of <see cref="NodeCommunication"/> (the path's final segment).</summary>
/// <summary>
/// Actor name of the node-side comm actor — where an inbound command is handed over, after
/// <c>NodeCommandService</c> has resolved it by registry key.
/// </summary>
public const string NodeCommunicationName = "node-communication";
}
src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs
+5 -3
View File
@@ -8,9 +8,11 @@ using ZB.MOM.WW.OtOpcUa.Commons.OpcUa;
namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication;
/// <summary>
/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2), living at
/// <see cref="MeshPaths.NodeCommunication"/> — the path this node's <c>NodeCommandService</c>
/// gRPC ingress hands every decoded command to.
/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2). This node's
/// <c>NodeCommandService</c> gRPC ingress hands it every decoded command, resolving it from
/// <c>ActorRegistry</c> by <c>NodeCommunicationActorKey</c> — the actor's name
/// (<see cref="MeshPaths.NodeCommunicationName"/>) is a local convention, not an address anything
/// off-process uses.
/// </summary>
/// <remarks>
/// <para>
tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs
+30 -17
View File
@@ -6,45 +6,58 @@ using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh;
namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests;
/// <summary>
/// Pins the two mesh comm-actor paths against a <b>really booted</b> host.
/// Pins that both mesh comm actors are actually <b>spawned</b>, under their declared names, on a
/// really booted host.
/// </summary>
/// <remarks>
/// <para>
/// <c>/user/central-communication</c> and <c>/user/node-communication</c> are where each
/// side's gRPC ingress expects to find its comm actor: the services resolve them by registry
/// key, and these constants are the names the actors are spawned under. Nothing else in the
/// codebase asserts they agree. Renaming either actor would compile, pass every unit test,
/// deploy — and refuse every inbound call as <c>Unavailable</c> ("the node is still
/// starting"), which reads as a boot-window race rather than a permanent misnaming.
/// <b>What this pins is existence, not a wire contract.</b> Since the cross-mesh transport
/// became gRPC, both services resolve their comm actor through <c>ActorRegistry</c> rather
/// than by path, so a rename is a compile error rather than a silent drop and
/// <see cref="MeshPaths"/> is only a local naming convention. What is still unpinned by
/// anything else is whether the two registration sites
/// (<c>WithOtOpcUaControlPlaneSingletons</c> / <c>WithOtOpcUaRuntimeActors</c>) actually run
/// and spawn these actors on a real node — and if either did not, every inbound call would be
/// refused as <c>Unavailable</c> ("the node is still starting"), which reads as a boot-window
/// race rather than as an actor that will never appear.
/// </para>
/// <para>
/// Booting the real host is the point. A test that spawned the actors itself would pin the
/// constant against itself and prove nothing about the wiring in
/// <c>WithOtOpcUaControlPlaneSingletons</c> / <c>WithOtOpcUaRuntimeActors</c>.
/// constant against itself and prove nothing about that wiring.
/// </para>
/// </remarks>
public class MeshCommActorPathTests
{
private static async Task AssertActorExistsAsync(ActorSystem system, string path)
/// <summary>
/// Builds the <c>ActorSelection</c> string for a top-level actor name. Composed here rather
/// than held as a constant: the full path is a local addressing detail of this test, not
/// something the product declares — see <see cref="MeshPaths"/>.
/// </summary>
/// <param name="actorName">The declared actor name.</param>
/// <returns>The absolute user-guardian path.</returns>
private static string UserPath(string actorName) => "/user/" + actorName;
private static async Task AssertActorExistsAsync(ActorSystem system, string actorName)
{
var path = UserPath(actorName);
var identity = await system.ActorSelection(path)
.Ask<ActorIdentity>(new Identify(path), TimeSpan.FromSeconds(10));
identity.Subject.ShouldNotBeNull(
$"{path} is where the central↔node boundary expects its comm actor; nothing else pins "
+ "it, and a mismatch turns every inbound call into a misleading readiness rejection");
$"no actor exists at {path}; the registration that spawns it did not run, and every "
+ "inbound mesh call to this node would be refused as a misleading readiness rejection");
}
[Fact]
public async Task Both_mesh_comm_actors_exist_at_their_contract_paths()
public async Task Both_mesh_comm_actors_are_spawned_under_their_declared_names()
{
await using var harness = await TwoNodeClusterHarness.StartAsync();
// Harness nodes carry admin+driver, so BOTH comm actors live on each of them.
await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.CentralCommunication);
await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.NodeCommunication);
await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.CentralCommunication);
await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.NodeCommunication);
await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.CentralCommunicationName);
await AssertActorExistsAsync(harness.NodeASystem, MeshPaths.NodeCommunicationName);
await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.CentralCommunicationName);
await AssertActorExistsAsync(harness.NodeBSystem, MeshPaths.NodeCommunicationName);
}
// NOT TESTED HERE: that the comm actors are registered per node rather than as cluster