chore(comm): delete dead IntegrationCallRequest routing (Gitea #32)

'Pattern 4: Integration Routing' — RouteIntegrationCallAsync → SiteEnvelope(IntegrationCallRequest) → SiteCommunicationActor integration handler — was plumbed end to end but connected at neither end: no producer (zero callers) and no handler (AkkaHostedService never registered LocalHandlerType.Integration). It was an early scaffold the architecture routed around — the brokered External→Central→Site→Central round-trip is served by the Inbound API's routed-site-script path (the RouteTo* verbs, driven from CommunicationServiceInstanceRouter), which is live, tested, and shares IntegrationTimeout. Decision (#32): delete. Removed the IntegrationCall{Request,Response} messages, RouteIntegrationCallAsync, the SiteCommunicationActor receive block + _integrationHandler field + LocalHandlerType.Integration, and the four tests that covered them (2 actor, 2 message-contract, 1 dispatcher- reject, 1 mapper-reject). KEPT IntegrationTimeout — it is the live timeout for the RouteTo* verbs. Updated the exclusion-narrative comments (proto/mapper/dispatcher), design §4, the components doc timeout table, and marked the known-issue RESOLVED. Full solution build clean (0/0); Communication 634 + Host 421 green. Net -172/+20 across 14 files. Not in the gRPC proto (was deliberately excluded there), so no wire-format change.
2026-07-23 14:27:32 -04:00
parent e0f105c3b3
commit a5256e9b12
14 changed files with 20 additions and 172 deletions
@@ -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 |
@@ -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` →
@@ -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.
@@ -1,14 +0,0 @@
-namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration;
-
-/// <summary>
-/// Request routed from central to site to invoke an integration method
-/// (external system call or notification) on behalf of the central UI or API.
-/// </summary>
-public record IntegrationCallRequest(
-    string CorrelationId,
-    string SiteId,
-    string InstanceUniqueName,
-    string TargetSystemName,
-    string MethodName,
-    IReadOnlyDictionary<string, object?> Parameters,
-    DateTimeOffset Timestamp);
@@ -1,12 +0,0 @@
-namespace ZB.MOM.WW.ScadaBridge.Commons.Messages.Integration;
-
-/// <summary>
-/// Response for an integration call routed through central-site communication.
-/// </summary>
-public record IntegrationCallResponse(
-    string CorrelationId,
-    string SiteId,
-    bool Success,
-    string? ResultJson,
-    string? ErrorMessage,
-    DateTimeOffset Timestamp);
@@ -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.
 /// </para>
-/// <para>
-/// <b>Excluded by design:</b> <c>IntegrationCallRequest</c> — 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).
-/// </para>
 /// </remarks>
 public sealed class SiteCommandDispatcher
 {
@@ -61,12 +61,6 @@ public class SiteCommunicationActor : ReceiveActor, IWithTimers
    /// <summary>The transport supplied by the Host (null falls back to <see cref="NoOpCentralTransport"/>).</summary>
    private readonly ICentralTransport? _injectedTransport;

-    /// <summary>
-    /// Handler for the vestigial <see cref="IntegrationCallRequest"/> — the one command NOT
-    /// migrated to the dispatcher (dead at both ends), so it still routes on the actor.
-    /// </summary>
-    private IActorRef? _integrationHandler;
-
    /// <summary>Akka timer scheduler injected by the framework via <see cref="IWithTimers"/>.</summary>
    public ITimerScheduler Timers { get; set; } = null!;

@@ -158,20 +152,6 @@ public class SiteCommunicationActor : ReceiveActor, IWithTimers
        Receive<RetryParkedOperation>(cmd => DispatchCommand(cmd));
        Receive<DiscardParkedOperation>(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<IntegrationCallRequest>(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
 }
@@ -224,21 +224,13 @@ public class CommunicationService
    }

    // ── Pattern 4: Integration Routing ──
-
-    /// <summary>
-    /// Routes an integration call to a site.
-    /// </summary>
-    /// <param name="siteId">The target site identifier.</param>
-    /// <param name="request">The integration call request.</param>
-    /// <param name="cancellationToken">Cancellation token.</param>
-    /// <returns>The integration call response.</returns>
-    public async Task<IntegrationCallResponse> RouteIntegrationCallAsync(
-        string siteId, IntegrationCallRequest request, CancellationToken cancellationToken = default)
-    {
-        var envelope = new SiteEnvelope(siteId, request);
-        return await GetActor().Ask<IntegrationCallResponse>(
-            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 ──

@@ -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.
 /// </para>
-/// <para>
-/// <b>Excluded by design:</b> <c>IntegrationCallRequest</c>. It is the 29th
-/// command on <c>SiteCommunicationActor</c>'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
-/// <c>docs/known-issues/2026-07-22-integration-call-routing-is-dead-code.md</c>.
-/// </para>
 /// <para><b>Null conventions.</b> proto3 has no presence for scalars, so:</para>
 /// <list type="bullet">
 ///   <item>
@@ -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.
@@ -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;
 /// </summary>
 public class MessageContractTests
 {
-    [Fact]
-    public void IntegrationCallRequest_HasCorrelationId()
-    {
-        var msg = new IntegrationCallRequest(
-            "corr-123", "site1", "inst1", "ExtSys1", "GetData",
-            new Dictionary<string, object?>(), 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);
@@ -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<string, object?>(), DateTimeOffset.UtcNow);
-
-        Assert.Throws<ArgumentException>(() => dispatcher.ResolveRoute(command));
-    }
-
    // ── Failover (local path) ──

    [Fact]
@@ -94,10 +94,8 @@ public class SiteCommandDtoMapperGoldenTests
    }

    /// <summary>
-    /// The contract carries exactly 28 commands — 29 on
-    /// <c>SiteCommunicationActor</c>'s receive table minus the dead
-    /// <c>IntegrationCallRequest</c>. 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.
    /// </summary>
    [Fact]
    public void CommandInventory_Is28_AcrossSixGroups()
@@ -431,17 +429,6 @@ public class SiteCommandDtoMapperGoldenTests
    // Group classification + rejection
    // ─────────────────────────────────────────────────────────────────────

-    /// <summary><c>IntegrationCallRequest</c> is excluded by design and must be rejected, not silently dropped.</summary>
-    [Fact]
-    public void IntegrationCallRequest_IsRejected()
-    {
-        var dead = new Commons.Messages.Integration.IntegrationCallRequest(
-            "corr", "site-a", "Instance", "System", "Method",
-            new Dictionary<string, object?>(), DateTimeOffset.UtcNow);
-
-        Assert.Throws<ArgumentException>(() => SiteCommandDtoMapper.GroupOf(dead));
-    }
-
    /// <summary>Packing a command into the wrong group's envelope is a hard error, not a silent no-op.</summary>
    [Fact]
    public void PackingIntoTheWrongGroup_Throws() =>
@@ -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<DeploymentStateQueryRequest>(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<string, object?>(), DateTimeOffset.UtcNow);
-
-        siteActor.Tell(request);
-
-        ExpectMsg<IntegrationCallResponse>(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<string, object?>(), DateTimeOffset.UtcNow);
-
-        siteActor.Tell(request);
-        handlerProbe.ExpectMsg<IntegrationCallRequest>(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