From b811a2451b23b9b35bf8e5db686911a3cf82e3e5 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 06:14:09 -0400 Subject: [PATCH] fix(dcl): remove unused poll-based WriteBatchAndWaitAsync from IDataConnection and both adapters (review 2026-10-21 02/U3) --- .../Component-DataConnectionLayer.md | 3 +- .../Interfaces/Protocol/IDataConnection.cs | 10 ---- .../Actors/DataConnectionActor.cs | 5 +- .../Adapters/MxGatewayDataConnection.cs | 29 ---------- .../Adapters/OpcUaDataConnection.cs | 28 ---------- .../Actors/FakeBatchDataConnection.cs | 7 --- .../Adapters/MxGatewayDataConnectionTests.cs | 53 ------------------- .../SimulatedDataConnection.cs | 8 --- 8 files changed, 5 insertions(+), 138 deletions(-) diff --git a/docs/requirements/Component-DataConnectionLayer.md b/docs/requirements/Component-DataConnectionLayer.md index e18de8ce..9750fd15 100644 --- a/docs/requirements/Component-DataConnectionLayer.md +++ b/docs/requirements/Component-DataConnectionLayer.md @@ -31,11 +31,12 @@ IDataConnection : IAsyncDisposable ├── ReadBatch(tagPaths) → values ├── Write(tagPath, value) → void ├── WriteBatch(values) → void -├── WriteBatchAndWait(values, flagPath, flagValue, responsePath, responseValue, timeout) → bool ├── Status → ConnectionHealth └── Disconnected → event Action? ``` +There is deliberately no write-and-wait handshake on the adapter interface: the script-facing `Attributes.WriteBatchAndWaitAsync` composes a `WriteTagBatchRequest` (batch + trigger write) with the Instance Actor's event-driven attribute waiter, so the handshake can never fall back to a polling read loop that bypasses the waiter (the adapters' former poll-based `WriteBatchAndWaitAsync` was unused and was removed — review 2026-10-21). + The `Disconnected` event is raised by an adapter when it detects an unexpected connection loss (server offline, network failure, keep-alive timeout). The `DataConnectionActor` subscribes to this event to trigger the reconnection state machine. Additional protocols can be added by implementing this interface. ### Batch Capability Seam diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Protocol/IDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Protocol/IDataConnection.cs index 44ea692a..35c26a55 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Protocol/IDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Interfaces/Protocol/IDataConnection.cs @@ -56,16 +56,6 @@ public interface IDataConnection : IAsyncDisposable /// Cancellation token. /// A dictionary of tag paths to their write results. Task> WriteBatchAsync(IDictionary values, CancellationToken cancellationToken = default); - /// Writes a batch of values, then writes a flag and waits for a specific response value within the timeout. - /// Tag values to write before the flag. - /// Tag path of the trigger flag. - /// Value to write to the flag tag. - /// Tag path to monitor for the expected response value. - /// The response value that indicates completion. - /// Maximum time to wait for the response. - /// Cancellation token. - /// true if the response value was observed within the timeout; otherwise false. - Task WriteBatchAndWaitAsync(IDictionary values, string flagPath, object? flagValue, string responsePath, object? responseValue, TimeSpan timeout, CancellationToken cancellationToken = default); /// Current connection health status. ConnectionHealth Status { get; } diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs index 3683a27e..2d32ab3f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Actors/DataConnectionActor.cs @@ -1972,8 +1972,9 @@ public class DataConnectionActor : UntypedActor, IWithTimers /// the timeout, or an adapter exception is translated into a failed /// returned synchronously to the caller — never a /// dropped reply. NOTE: this deliberately composes the batch + trigger primitives and - /// uses the EXISTING event-driven WaitForAttribute waiter for the wait half; it does - /// NOT call the adapter's poll-based WriteBatchAndWaitAsync. + /// uses the EXISTING event-driven WaitForAttribute waiter for the wait half (the + /// adapters' former poll-based WriteBatchAndWaitAsync was removed — review + /// 2026-10-21 02/U3 — so no polling handshake can bypass that waiter). /// private void HandleWriteBatch(WriteTagBatchRequest request) { diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs index 0911e32e..3365166c 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/MxGatewayDataConnection.cs @@ -420,35 +420,6 @@ public class MxGatewayDataConnection return results.ToDictionary(w => w.TagPath, w => new WriteResult(w.Success, w.Error)); } - /// - public async Task WriteBatchAndWaitAsync( - IDictionary values, string flagPath, object? flagValue, - string responsePath, object? responseValue, TimeSpan timeout, CancellationToken cancellationToken = default) - { - await WriteBatchAsync(values, cancellationToken); - await WriteAsync(flagPath, flagValue, cancellationToken); - - using var timeoutCts = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken); - timeoutCts.CancelAfter(timeout); - try - { - while (!timeoutCts.IsCancellationRequested) - { - var r = await ReadAsync(responsePath, timeoutCts.Token); - // r.Value is a TagValue wrapper; compare its underlying scalar. String - // projection tolerates numeric type differences across the gRPC boundary. - if (r.Success && string.Equals(r.Value?.Value?.ToString(), responseValue?.ToString(), StringComparison.Ordinal)) - return true; - await Task.Delay(TimeSpan.FromMilliseconds(200), timeoutCts.Token); - } - } - catch (OperationCanceledException) when (!cancellationToken.IsCancellationRequested) - { - // Timeout elapsed (the linked CTS, not the caller's token) — fall through to false. - } - return false; - } - /// public async Task BrowseChildrenAsync( string? parentNodeId, diff --git a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs index 4cabfc02..90b8935f 100644 --- a/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs +++ b/src/ZB.MOM.WW.ScadaBridge.DataConnectionLayer/Adapters/OpcUaDataConnection.cs @@ -416,34 +416,6 @@ public class OpcUaDataConnection return _client.SearchAddressSpaceAsync(query, maxDepth, maxResults, cancellationToken); } - /// - public async Task WriteBatchAndWaitAsync( - IDictionary values, string flagPath, object? flagValue, - string responsePath, object? responseValue, TimeSpan timeout, - CancellationToken cancellationToken = default) - { - // Write all values including the flag - var allValues = new Dictionary(values) { [flagPath] = flagValue }; - var writeResults = await WriteBatchAsync(allValues, cancellationToken); - - if (writeResults.Values.Any(r => !r.Success)) - return false; - - // Poll for response value within timeout - var deadline = DateTimeOffset.UtcNow + timeout; - while (DateTimeOffset.UtcNow < deadline) - { - cancellationToken.ThrowIfCancellationRequested(); - var readResult = await ReadAsync(responsePath, cancellationToken); - if (readResult.Success && readResult.Value != null && Equals(readResult.Value.Value, responseValue)) - return true; - - await Task.Delay(100, cancellationToken); - } - - return false; - } - private void StopHeartbeatMonitor() { _staleMonitor?.Dispose(); diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Actors/FakeBatchDataConnection.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Actors/FakeBatchDataConnection.cs index 51ee60f4..8de09b1e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Actors/FakeBatchDataConnection.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Actors/FakeBatchDataConnection.cs @@ -163,13 +163,6 @@ public sealed class FakeBatchDataConnection => Task.FromResult>( values.ToDictionary(kv => kv.Key, _ => new WriteResult(true, null))); - // Fake behaviour: Reports that the handshake matched without writing anything. - /// - public Task WriteBatchAndWaitAsync( - IDictionary values, string flagPath, object? flagValue, string responsePath, - object? responseValue, TimeSpan timeout, CancellationToken cancellationToken = default) - => Task.FromResult(true); - // Fake behaviour: Captures the alarm callback and returns a generated subscription id. /// public Task SubscribeAlarmsAsync( diff --git a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs index 7f6d0cb0..fd91290c 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.DataConnectionLayer.Tests/Adapters/MxGatewayDataConnectionTests.cs @@ -186,59 +186,6 @@ public class MxGatewayDataConnectionTests Assert.Equal("rejected", r.ErrorMessage); } - // ── WriteBatchAndWait ── - - /// Verifies that a batch write and wait returns true when the response value matches. - /// A task that represents the asynchronous operation. - [Fact] - public async Task WriteBatchAndWait_returns_true_when_response_matches() - { - var writeCalls = new List(); - var fake = new FakeMxGatewayClient - { - WriteHandler = writes => - { - writeCalls.AddRange(writes.Select(w => w.TagPath)); - return writes.Select(w => new MxWriteOutcome(w.TagPath, true, null)).ToList(); - }, - // Response path already reads the expected value. - ReadHandler = tags => tags.Select(t => - new MxReadOutcome(t, true, "DONE", QualityCode.Good, DateTimeOffset.UtcNow, null)).ToList() - }; - var adapter = NewAdapter(fake); - await adapter.ConnectAsync(Details()); - - var ok = await adapter.WriteBatchAndWaitAsync( - new Dictionary { ["V"] = 1 }, "Flag", 1, "Resp", "DONE", - TimeSpan.FromSeconds(5)); - - Assert.True(ok); - // Values written before the flag, and the flag itself. - Assert.Contains("V", writeCalls); - Assert.Contains("Flag", writeCalls); - } - - /// Verifies that a batch write and wait returns false on timeout. - /// A task that represents the asynchronous operation. - [Fact] - public async Task WriteBatchAndWait_returns_false_on_timeout() - { - var fake = new FakeMxGatewayClient - { - WriteHandler = writes => writes.Select(w => new MxWriteOutcome(w.TagPath, true, null)).ToList(), - ReadHandler = tags => tags.Select(t => - new MxReadOutcome(t, true, "NEVER", QualityCode.Good, DateTimeOffset.UtcNow, null)).ToList() - }; - var adapter = NewAdapter(fake); - await adapter.ConnectAsync(Details()); - - var ok = await adapter.WriteBatchAndWaitAsync( - new Dictionary { ["V"] = 1 }, "Flag", 1, "Resp", "DONE", - TimeSpan.FromMilliseconds(300)); - - Assert.False(ok); - } - // ── browse ── /// Verifies that browsing children maps the children and the truncated flag. diff --git a/tests/ZB.MOM.WW.ScadaBridge.LoadHarness/SimulatedDataConnection.cs b/tests/ZB.MOM.WW.ScadaBridge.LoadHarness/SimulatedDataConnection.cs index e61cb558..84cd121e 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.LoadHarness/SimulatedDataConnection.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.LoadHarness/SimulatedDataConnection.cs @@ -152,14 +152,6 @@ public sealed class SimulatedDataConnection : IDataConnection, IBatchSubscribabl return Task.FromResult>(results); } - // Accepts the batch and reports that the awaited response matched immediately. - /// - public Task WriteBatchAndWaitAsync( - IDictionary values, string flagPath, object? flagValue, - string responsePath, object? responseValue, TimeSpan timeout, - CancellationToken cancellationToken = default) - => Task.FromResult(true); - /// Releases the connection; the simulated connection holds no resources. /// A task that represents the asynchronous operation. public ValueTask DisposeAsync() => ValueTask.CompletedTask;