diff --git a/docs/components/Communication.md b/docs/components/Communication.md index 5013fd03..629dd5ef 100644 --- a/docs/components/Communication.md +++ b/docs/components/Communication.md @@ -215,7 +215,7 @@ Central callers interact through `CommunicationService`, which wraps each comman | Instance deployment (notify-and-fetch) | `RefreshDeploymentAsync` | 120 s | | Instance lifecycle | `DisableInstanceAsync`, `EnableInstanceAsync`, `DeleteInstanceAsync` | 30 s | | Artifact deployment | `DeployArtifactsAsync` | 60 s | -| Integration routing | `RouteIntegrationCallAsync` | 30 s | +| Integration routing (Inbound API routed-site-script) | `RouteToCallAsync`, `RouteToGetAttributesAsync`, `RouteToSetAttributesAsync`, `RouteToWaitForAttributeAsync` | 30 s (`IntegrationTimeout`) | | Debug snapshot | `RequestDebugSnapshotAsync` | 30 s | | Remote queries | `QueryEventLogsAsync`, `QueryParkedMessagesAsync`, etc. | 30 s | | OPC UA tag browse | `BrowseNodeAsync` | 30 s | diff --git a/docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md b/docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md index efecb228..81c7ecc1 100644 --- a/docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md +++ b/docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md @@ -1,9 +1,17 @@ # Integration call routing (`IntegrationCallRequest`) is dead on both ends -**Date:** 2026-07-22 · **Status:** OPEN (decision needed: wire or delete) · **Tracked:** Gitea +**Date:** 2026-07-22 · **Status:** RESOLVED (DELETED 2026-07-23) · **Tracked:** Gitea [#32](https://gitea.dohertylan.com/dohertj2/ScadaBridge/issues/32) (filed 2026-07-23) · **Severity:** Low (no runtime impact — the path cannot be reached) · **Area:** Central–Site Communication +> **Resolution (2026-07-23):** Decision = **delete** (option 1 below). Removed the +> `IntegrationCallRequest`/`IntegrationCallResponse` messages, `CommunicationService.RouteIntegrationCallAsync`, +> the `SiteCommunicationActor` receive block + `_integrationHandler` field + `LocalHandlerType.Integration`, +> and the four tests that covered them. `IntegrationTimeout` was **kept** — it is the live timeout for the +> Inbound API's `RouteTo*` verbs, which are the actual implementation of this "External → Central → Site → +> Central" pattern (design §4). The narrative comments that documented the exclusion (proto/mapper/dispatcher) +> were updated. Full solution build clean; Communication suite 634 green. This note is retained as the record. + ## What "Pattern 4: Integration Routing" — `CommunicationService.RouteIntegrationCallAsync` → diff --git a/docs/requirements/Component-Communication.md b/docs/requirements/Component-Communication.md index 9533df56..ed8dd036 100644 --- a/docs/requirements/Component-Communication.md +++ b/docs/requirements/Component-Communication.md @@ -46,6 +46,7 @@ Both central and site clusters. Each side has communication actors that handle m - Central routes the request to the appropriate site. - Site reads values from the Instance Actor and responds. - Central returns the response to the external system. +- **Implementation**: this brokered round-trip is served by the **Inbound API's routed-site-script path** — a central-side inbound API method script composes the typed `RouteTo*` verbs on `CommunicationService` (`RouteToCall`, `RouteToGetAttributes`, `RouteToSetAttributes`, `RouteToWaitForAttribute`), driven from `InboundAPI/CommunicationServiceInstanceRouter` and sharing `IntegrationTimeout`. An earlier generic `IntegrationCallRequest` primitive was scaffolded for this pattern but never wired to a producer or a site handler; it was removed as dead code (Gitea #32, 2026-07-23). ### 5. Recipe/Command Delivery (External System → Central → Site) - **Pattern**: Fire-and-forget with acknowledgment. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallRequest.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallRequest.cs deleted file mode 100644 index 6b774bcc..00000000 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallRequest.cs +++ /dev/null @@ -1,14 +0,0 @@ -namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration; - -/// -/// Request routed from central to site to invoke an integration method -/// (external system call or notification) on behalf of the central UI or API. -/// -public record IntegrationCallRequest( - string CorrelationId, - string SiteId, - string InstanceUniqueName, - string TargetSystemName, - string MethodName, - IReadOnlyDictionary Parameters, - DateTimeOffset Timestamp); diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallResponse.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallResponse.cs deleted file mode 100644 index 1445e3a4..00000000 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Messages/Integration/IntegrationCallResponse.cs +++ /dev/null @@ -1,12 +0,0 @@ -namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration; - -/// -/// Response for an integration call routed through central-site communication. -/// -public record IntegrationCallResponse( - string CorrelationId, - string SiteId, - bool Success, - string? ResultJson, - string? ErrorMessage, - DateTimeOffset Timestamp); diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommandDispatcher.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommandDispatcher.cs index e37456ef..1e46d086 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommandDispatcher.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommandDispatcher.cs @@ -35,11 +35,6 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Actors; /// singleton proxy (design §7.3). This is the same target the actor used; the /// extraction does not "fix" it. /// -/// -/// Excluded by design: IntegrationCallRequest — the 29th command, dead at -/// both ends. It never enters this dispatcher; the actor keeps its own vestigial handler -/// for it (28 of 29 migrate). -/// /// public sealed class SiteCommandDispatcher { diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommunicationActor.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommunicationActor.cs index 9ce79872..f8fd7853 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommunicationActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Actors/SiteCommunicationActor.cs @@ -61,12 +61,6 @@ public class SiteCommunicationActor : ReceiveActor, IWithTimers /// The transport supplied by the Host (null falls back to ). private readonly ICentralTransport? _injectedTransport; - /// - /// Handler for the vestigial — the one command NOT - /// migrated to the dispatcher (dead at both ends), so it still routes on the actor. - /// - private IActorRef? _integrationHandler; - /// Akka timer scheduler injected by the framework via . public ITimerScheduler Timers { get; set; } = null!; @@ -158,20 +152,6 @@ public class SiteCommunicationActor : ReceiveActor, IWithTimers Receive(cmd => DispatchCommand(cmd)); Receive(cmd => DispatchCommand(cmd)); - // Integration Routing — the 29th command, NOT migrated to the dispatcher (dead at - // both ends; no production code registers the handler). Kept on the actor so the - // dispatcher's command surface stays the 28 that actually migrate. - Receive(msg => - { - if (_integrationHandler != null) - _integrationHandler.Forward(msg); - else - { - Sender.Tell(new IntegrationCallResponse( - msg.CorrelationId, _siteId, false, null, "Integration handler not available", DateTimeOffset.UtcNow)); - } - }); - // Central→site manual failover relay. Central and the site are separate clusters, // so central can only ask — this node performs the graceful Leave locally, scoped to // the SITE-SPECIFIC role, because that is what site singletons (the Deployment @@ -281,9 +261,6 @@ public class SiteCommunicationActor : ReceiveActor, IWithTimers case LocalHandlerType.ParkedMessages: _dispatcher.RegisterParkedMessageHandler(msg.Handler); break; - case LocalHandlerType.Integration: - _integrationHandler = msg.Handler; - break; case LocalHandlerType.Artifacts: _dispatcher.RegisterArtifactHandler(msg.Handler); break; @@ -364,6 +341,5 @@ public enum LocalHandlerType { EventLog, ParkedMessages, - Integration, Artifacts } diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs index 619308f6..158a3b35 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/CommunicationService.cs @@ -224,21 +224,13 @@ public class CommunicationService } // ── Pattern 4: Integration Routing ── - - /// - /// Routes an integration call to a site. - /// - /// The target site identifier. - /// The integration call request. - /// Cancellation token. - /// The integration call response. - public async Task RouteIntegrationCallAsync( - string siteId, IntegrationCallRequest request, CancellationToken cancellationToken = default) - { - var envelope = new SiteEnvelope(siteId, request); - return await GetActor().Ask( - envelope, _options.IntegrationTimeout, cancellationToken); - } + // The brokered "External System → Central → Site → Central → External System" + // round-trip (design §4) is served by the Inbound API's routed-site-script + // path — the RouteTo* verbs below (RouteToCall / RouteToGetAttributes / + // RouteToSetAttributes / RouteToWaitForAttribute), driven from + // InboundAPI/CommunicationServiceInstanceRouter and sharing IntegrationTimeout. + // The earlier generic IntegrationCallRequest primitive was never wired to a + // producer or a site handler and was removed as dead code (Gitea #32). // ── Pattern 5: Debug View ── diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteCommandDtoMapper.cs b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteCommandDtoMapper.cs index f9b850c7..6bbd6a73 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteCommandDtoMapper.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Grpc/SiteCommandDtoMapper.cs @@ -34,13 +34,6 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Grpc; /// server (T1B.2) and the central transport (T1B.3) can share one implementation /// without a project-reference cycle. /// -/// -/// Excluded by design: IntegrationCallRequest. It is the 29th -/// command on SiteCommunicationActor's receive table but is dead at both -/// ends — no production code registers the integration handler, so the site -/// always answers "Integration handler not available". See -/// docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md. -/// /// Null conventions. proto3 has no presence for scalars, so: /// /// diff --git a/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/site_command.proto b/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/site_command.proto index 9f61f69c..7f86b93f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/site_command.proto +++ b/src/ZB.MOM.WW.ScadaBridge.Communication/Protos/site_command.proto @@ -17,10 +17,6 @@ import "google/protobuf/wrappers.proto"; // place on the client and one dispatch switch on the server. A `oneof` request // / reply envelope preserves per-command typing inside the group. // -// IntegrationCallRequest (the 29th command) is deliberately NOT here — it is -// dead code at both ends; see -// docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md. -// // EVOLUTION RULE (same as sitestream.proto): field numbers are never reused and // changes are additive only. New commands take the next free oneof tag; an // older peer simply sees an unset oneof and answers with a clean error. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/MessageContractTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/MessageContractTests.cs index a3ece8da..7e18509e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/MessageContractTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/MessageContractTests.cs @@ -1,4 +1,3 @@ -using ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration; using ZB.MOM.WW.ScadaBridge.Commons.Messages.RemoteQuery; namespace ZB.MOM.WW.ScadaBridge.Communication.Tests; @@ -8,25 +7,6 @@ namespace ZB.MOM.WW.ScadaBridge.Communication.Tests; /// public class MessageContractTests { - [Fact] - public void IntegrationCallRequest_HasCorrelationId() - { - var msg = new IntegrationCallRequest( - "corr-123", "site1", "inst1", "ExtSys1", "GetData", - new Dictionary(), DateTimeOffset.UtcNow); - - Assert.Equal("corr-123", msg.CorrelationId); - } - - [Fact] - public void IntegrationCallResponse_HasCorrelationId() - { - var msg = new IntegrationCallResponse( - "corr-123", "site1", true, "{}", null, DateTimeOffset.UtcNow); - - Assert.Equal("corr-123", msg.CorrelationId); - } - [Fact] public void EventLogQueryRequest_HasCorrelationId() { @@ -79,10 +59,6 @@ public class MessageContractTests Assert.True(typeof(Commons.Messages.Artifacts.DeployArtifactsCommand).IsValueType == false); Assert.True(typeof(Commons.Messages.Artifacts.ArtifactDeploymentResponse).IsValueType == false); - // Pattern 4: Integration - Assert.True(typeof(IntegrationCallRequest).IsValueType == false); - Assert.True(typeof(IntegrationCallResponse).IsValueType == false); - // Pattern 5: Debug View Assert.True(typeof(Commons.Messages.DebugView.SubscribeDebugViewRequest).IsValueType == false); Assert.True(typeof(Commons.Messages.DebugView.DebugViewSnapshot).IsValueType == false); diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDispatcherTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDispatcherTests.cs index fc984581..6a5da87f 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDispatcherTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDispatcherTests.cs @@ -1,7 +1,6 @@ using Akka.Actor; using Akka.TestKit.Xunit2; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Artifacts; -using ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration; using ZB.MOM.WW.ScadaBridge.Commons.Messages.RemoteQuery; using ZB.MOM.WW.ScadaBridge.Commons.Types; using ZB.MOM.WW.ScadaBridge.Communication.Actors; @@ -243,18 +242,6 @@ public class SiteCommandDispatcherTests : TestKit dispatcher.ResolveRoute(new TriggerSiteFailover("c", SiteId))); } - [Fact] - public void ResolveRoute_RejectsTheExcludedIntegrationCommand() - { - // IntegrationCallRequest is the 29th command, dead at both ends and deliberately excluded - // (28 of 29 migrate). It never enters the dispatcher. - var dispatcher = Build(CreateTestProbe().Ref); - var command = new IntegrationCallRequest( - "c", SiteId, "inst", "es", "m", new Dictionary(), DateTimeOffset.UtcNow); - - Assert.Throws(() => dispatcher.ResolveRoute(command)); - } - // ── Failover (local path) ── [Fact] diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDtoMapperGoldenTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDtoMapperGoldenTests.cs index 16499a40..6013ca64 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDtoMapperGoldenTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommandDtoMapperGoldenTests.cs @@ -94,10 +94,8 @@ public class SiteCommandDtoMapperGoldenTests } /// - /// The contract carries exactly 28 commands — 29 on - /// SiteCommunicationActor's receive table minus the dead - /// IntegrationCallRequest. If the site gains a command, this count - /// moves deliberately, not silently. + /// The contract carries exactly 28 commands across six domain groups. If the + /// site gains a command, this count moves deliberately, not silently. /// [Fact] public void CommandInventory_Is28_AcrossSixGroups() @@ -431,17 +429,6 @@ public class SiteCommandDtoMapperGoldenTests // Group classification + rejection // ───────────────────────────────────────────────────────────────────── - /// IntegrationCallRequest is excluded by design and must be rejected, not silently dropped. - [Fact] - public void IntegrationCallRequest_IsRejected() - { - var dead = new Commons.Messages.Integration.IntegrationCallRequest( - "corr", "site-a", "Instance", "System", "Method", - new Dictionary(), DateTimeOffset.UtcNow); - - Assert.Throws(() => SiteCommandDtoMapper.GroupOf(dead)); - } - /// Packing a command into the wrong group's envelope is a hard error, not a silent no-op. [Fact] public void PackingIntoTheWrongGroup_Throws() => diff --git a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommunicationActorTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommunicationActorTests.cs index b458a484..28acc06b 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommunicationActorTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Communication.Tests/SiteCommunicationActorTests.cs @@ -5,7 +5,6 @@ using ZB.MOM.WW.ScadaBridge.Commons.Messages.DataConnection; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Deployment; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Health; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Lifecycle; -using ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Management; using ZB.MOM.WW.ScadaBridge.Commons.Messages.Notification; using ZB.MOM.WW.ScadaBridge.Commons.Messages.RemoteQuery; @@ -73,42 +72,6 @@ public class SiteCommunicationActorTests : TestKit dmProbe.ExpectMsg(msg => msg.CorrelationId == "corr-q"); } - [Fact] - public void IntegrationCall_WithoutHandler_ReturnsFailure() - { - var dmProbe = CreateTestProbe(); - var siteActor = Sys.ActorOf(Props.Create(() => - new SiteCommunicationActor("site1", _options, dmProbe.Ref))); - - var request = new IntegrationCallRequest( - "corr1", "site1", "inst1", "ExtSys1", "GetData", - new Dictionary(), DateTimeOffset.UtcNow); - - siteActor.Tell(request); - - ExpectMsg(msg => - !msg.Success && msg.ErrorMessage == "Integration handler not available"); - } - - [Fact] - public void IntegrationCall_WithHandler_ForwardedToHandler() - { - var dmProbe = CreateTestProbe(); - var handlerProbe = CreateTestProbe(); - var siteActor = Sys.ActorOf(Props.Create(() => - new SiteCommunicationActor("site1", _options, dmProbe.Ref))); - - // Register integration handler - siteActor.Tell(new RegisterLocalHandler(LocalHandlerType.Integration, handlerProbe.Ref)); - - var request = new IntegrationCallRequest( - "corr1", "site1", "inst1", "ExtSys1", "GetData", - new Dictionary(), DateTimeOffset.UtcNow); - - siteActor.Tell(request); - handlerProbe.ExpectMsg(msg => msg.CorrelationId == "corr1"); - } - // The site→central send-forwarding tests that used to live here (NotificationSubmit / // NotificationStatusQuery, with and without a registered central ClusterClient) were removed // with the Akka transport in the ClusterClient→gRPC migration's Phase 4: the actor no longer