From cd7135570cbc4647eb6e2ceb8e29db510a806846 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 6 Aug 2026 16:14:25 -0400 Subject: [PATCH] =?UTF-8?q?refactor(mesh):=20collapse=20MeshPaths=20to=20l?= =?UTF-8?q?ocal=20actor=20names=20=E2=80=94=20no=20longer=20a=20wire=20con?= =?UTF-8?q?tract=20(review=20follow-up)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../Messages/Mesh/MeshPaths.cs | 44 ++++++++++------- .../Communication/NodeCommunicationActor.cs | 8 ++-- .../MeshCommActorPathTests.cs | 47 ++++++++++++------- 3 files changed, 62 insertions(+), 37 deletions(-) diff --git a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs index 99337b79..1bf2ec4d 100644 --- a/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs +++ b/src/Core/ZB.MOM.WW.OtOpcUa.Commons/Messages/Mesh/MeshPaths.cs @@ -1,33 +1,43 @@ namespace ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; /// -/// Actor paths the two mesh comm actors register under. These strings are the wire -/// contract of the central↔node boundary. +/// The local actor names the two mesh comm actors are spawned under. Not a wire contract: +/// nothing off-process addresses either actor by name. /// /// /// -/// Declared once, here, rather than as literals on each side. The deployments / -/// deployment-acks topic names are declared twice — once in -/// ConfigPublishCoordinator and again in DriverHostActor — 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. +/// This used to be a wire contract and no longer is. While the cross-mesh transport +/// was an Akka ClusterClient, central addressed a node by the literal path string +/// /user/node-communication 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 ActorRegistry (CentralCommunicationActorKey / +/// NodeCommunicationActorKey), which is type-checked, so the name below never crosses +/// a process boundary. The full /user/... path constants that carried the old contract +/// are deleted; what remains is a naming convention. /// /// -/// Nothing else pins these values, which is why MeshCommActorPathTests boots a real -/// host and Identify-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 ActorSelection +/// 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. +/// +/// +/// MeshCommActorPathTests boots a real host and Identify-probes +/// /user/{name} on both nodes, which is what pins these values against the two +/// registration sites. /// /// public static class MeshPaths { - /// Central-side comm actor — where a driver node sends its deployment acks. - public const string CentralCommunication = "/user/central-communication"; - - /// Node-side comm actor — where central sends commands. - public const string NodeCommunication = "/user/node-communication"; - - /// Actor name of (the path's final segment). + /// + /// Actor name of the central-side comm actor — where an inbound deployment ack is handed + /// over, after CentralCommandService has resolved it by registry key. + /// public const string CentralCommunicationName = "central-communication"; - /// Actor name of (the path's final segment). + /// + /// Actor name of the node-side comm actor — where an inbound command is handed over, after + /// NodeCommandService has resolved it by registry key. + /// public const string NodeCommunicationName = "node-communication"; } diff --git a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs index e54da301..5d6bbeea 100644 --- a/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs +++ b/src/Server/ZB.MOM.WW.OtOpcUa.Runtime/Communication/NodeCommunicationActor.cs @@ -8,9 +8,11 @@ using ZB.MOM.WW.OtOpcUa.Commons.OpcUa; namespace ZB.MOM.WW.OtOpcUa.Runtime.Communication; /// -/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2), living at -/// — the path this node's NodeCommandService -/// gRPC ingress hands every decoded command to. +/// Node-side end of the central↔node command boundary (per-cluster mesh Phase 2). This node's +/// NodeCommandService gRPC ingress hands it every decoded command, resolving it from +/// ActorRegistry by NodeCommunicationActorKey — the actor's name +/// () is a local convention, not an address anything +/// off-process uses. /// /// /// diff --git a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs index 0e65d371..ac2bc21b 100644 --- a/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs +++ b/tests/Server/ZB.MOM.WW.OtOpcUa.Host.IntegrationTests/MeshCommActorPathTests.cs @@ -6,45 +6,58 @@ using ZB.MOM.WW.OtOpcUa.Commons.Messages.Mesh; namespace ZB.MOM.WW.OtOpcUa.Host.IntegrationTests; /// -/// Pins the two mesh comm-actor paths against a really booted host. +/// Pins that both mesh comm actors are actually spawned, under their declared names, on a +/// really booted host. /// /// /// -/// /user/central-communication and /user/node-communication 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 Unavailable ("the node is still -/// starting"), which reads as a boot-window race rather than a permanent misnaming. +/// What this pins is existence, not a wire contract. Since the cross-mesh transport +/// became gRPC, both services resolve their comm actor through ActorRegistry rather +/// than by path, so a rename is a compile error rather than a silent drop and +/// is only a local naming convention. What is still unpinned by +/// anything else is whether the two registration sites +/// (WithOtOpcUaControlPlaneSingletons / WithOtOpcUaRuntimeActors) actually run +/// and spawn these actors on a real node — and if either did not, every inbound call would be +/// refused as Unavailable ("the node is still starting"), which reads as a boot-window +/// race rather than as an actor that will never appear. /// /// /// 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 -/// WithOtOpcUaControlPlaneSingletons / WithOtOpcUaRuntimeActors. +/// constant against itself and prove nothing about that wiring. /// /// public class MeshCommActorPathTests { - private static async Task AssertActorExistsAsync(ActorSystem system, string path) + /// + /// Builds the ActorSelection 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 . + /// + /// The declared actor name. + /// The absolute user-guardian path. + 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(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