fix(client): endpoint port rewrite, Part 13 time-weighted aggregates, clean history-refusal error (#678.5, #677.5, #674.6, #678.6)

- EndpointSelector rewrites host AND port to the user-supplied URL (same scheme only) - AggregateType/mapper/CLI/UI gain TimeAverage, TimeAverage2, Total, Total2, Interpolative - HistoryReadCommand turns ServiceResultException into a one-line CommandException - NodeManagerHistoryReadTests non-historized case sets IsReadModified=false
2026-10-11 10:45:18 -04:00
parent 825393299e
commit fe919f8b08
14 changed files with 173 additions and 17 deletions
@@ -57,6 +57,11 @@ otopcua-cli write -u opc.tcp://localhost:4840 -n "ns=2;s=MyNode" -v 42 -U operat

 ### Failover

+**Mapped ports and internal hostnames.** Endpoint discovery rewrites the host *and port* of the endpoint the server
+advertises to the URL you passed (when the scheme matches), so connecting through a port mapping works even though the
+server advertises its internal address (for example the docker-dev rig: `-u opc.tcp://localhost:4841` reaches the
+node mapped to 4841, although every container advertises `:4840`).
+
 When `-F` is provided, the shared service tries the primary URL first, then each failover URL in order. For long-running commands (`subscribe`, `alarms`), the service monitors the session via keep-alive and automatically reconnects to the next available server on failure.

 ```bash
@@ -229,7 +234,7 @@ continuation points up to `--max`. A server on the `Gateway` backend (no modific
 | `--start` | Start time in ISO 8601 UTC format, e.g. `2026-01-15T08:00:00Z` (default: 24 hours ago) |
 | `--end` | End time in ISO 8601 UTC format, e.g. `2026-01-15T09:00:00Z` (default: now) |
 | `--max` | Maximum number of values (default: 1000) |
-| `--aggregate` | Aggregate function: Average, Minimum, Maximum, Count, Start, End, StandardDeviation |
+| `--aggregate` | Aggregate function: Average, Minimum, Maximum, Count, Start, End, StandardDeviation, TimeAverage, TimeAverage2, Total, Total2, Interpolative |
 | `--interval` | Processing interval in milliseconds for aggregates (default: 3600000) |
 | `--modified` | Read modified-value history (Part 11 `IsReadModified`) instead of raw samples |

@@ -270,8 +275,9 @@ node-level code (`Good`, or `BadNoData` if the range held nothing). A node that
 updater, so no `HistoryWrite` access bit), answers `BadHistoryOperationUnsupported`; a `NaN`/`Infinity` double is `BadInvalidArgument`. The session's API key
 (`-U apikey -P otua_<keyId>_<secret>`) needs the `HistoryUpdate` permission — via a `Cluster`-scoped grant when `Security:Acl:Enabled` is off, or a scoped
 grant on the node when it is on; a key with only `WriteOperate` gets `BadUserAccessDenied`. See [Historian.md §HistoryUpdate](Historian.md#historyupdate) and
-[security.md](security.md#historyupdate-historian-writes). `historyread --aggregate` only knows the seven names in the table below; the SQL backend's other
-aggregates (Total, Range, Delta, PercentGood, …) need a different client.
+[security.md](security.md#historyupdate-historian-writes). `historyread --aggregate` only knows the twelve names in the table below; the SQL backend's other
+aggregates (Range, Delta, PercentGood, …) need a different client. A refused history read (for example
+`BadHistoryOperationUnsupported`) prints one error line and exits non-zero; it does not dump a stack trace.

 `delete-raw` is sent with `IsDeleteModified=false`. Invalid or inconsistent options (unknown mode, unpaired
 value/time, unparsable value, options that do not apply to the mode) fail before any connection is made.
@@ -287,6 +293,11 @@ value/time, unparsable value, options that do not apply to the mode) fail before
 | `Start` | `first` | `AggregateFunction_Start` |
 | `End` | `last` | `AggregateFunction_End` |
 | `StandardDeviation` | `stddev`, `stdev` | `AggregateFunction_StandardDeviationPopulation` |
+| `TimeAverage` | `timeavg` | `AggregateFunction_TimeAverage` |
+| `TimeAverage2` | `timeavg2` | `AggregateFunction_TimeAverage2` |
+| `Total` |  | `AggregateFunction_Total` |
+| `Total2` |  | `AggregateFunction_Total2` |
+| `Interpolative` | `interp` | `AggregateFunction_Interpolative` |

 ### alarms

@@ -206,7 +206,7 @@ All times are in UTC. Invalid input turns red on blur.

 | Option | Description |
 |--------|-------------|
-| Aggregate | Raw (default), Average, Minimum, Maximum, Count, Start, End, Standard Deviation |
+| Aggregate | Raw (default), Average, Minimum, Maximum, Count, Start, End, Standard Deviation, Time Average, Time Average 2, Total, Total 2, Interpolative |
 | Interval (ms) | Processing interval for aggregate queries (shown only for aggregates) |
 | Max Values | Maximum number of raw values to return (default 1000) |

@@ -47,7 +47,7 @@ public class HistoryReadCommand : CommandBase
    /// <summary>
    /// Gets the optional aggregate name used when the operator wants processed history instead of raw samples.
    /// </summary>
-    [CommandOption("aggregate", Description = "Aggregate function: Average, Minimum, Maximum, Count, Start, End, StandardDeviation")]
+    [CommandOption("aggregate", Description = "Aggregate function: Average, Minimum, Maximum, Count, Start, End, StandardDeviation, TimeAverage, TimeAverage2, Total, Total2, Interpolative")]
    public string? Aggregate { get; init; }

    /// <summary>
@@ -183,6 +183,11 @@ public class HistoryReadCommand : CommandBase
            await console.Output.WriteLineAsync();
            await console.Output.WriteLineAsync($"{values.Count} values returned.");
        }
+        catch (ServiceResultException ex)
+        {
+            // e.g. HistoryReadStatus.ThrowIfRefused: a refusal is an operator-facing outcome, not a crash.
+            throw new CommandException(ex.Message);
+        }
        finally
        {
            if (service != null)
@@ -204,8 +209,13 @@ public class HistoryReadCommand : CommandBase
            "start" or "first" => AggregateType.Start,
            "end" or "last" => AggregateType.End,
            "standarddeviation" or "stddev" or "stdev" => AggregateType.StandardDeviation,
+            "timeaverage" or "timeavg" => AggregateType.TimeAverage,
+            "timeaverage2" or "timeavg2" => AggregateType.TimeAverage2,
+            "total" => AggregateType.Total,
+            "total2" => AggregateType.Total2,
+            "interpolative" or "interp" => AggregateType.Interpolative,
            _ => throw new ArgumentException(
-                $"Unknown aggregate: '{name}'. Supported: Average, Minimum, Maximum, Count, Start, End, StandardDeviation")
+                $"Unknown aggregate: '{name}'. Supported: Average, Minimum, Maximum, Count, Start, End, StandardDeviation, TimeAverage, TimeAverage2, Total, Total2, Interpolative")
        };
    }
 }
@@ -85,17 +85,20 @@ internal static class EndpointSelector
                $"No endpoint found with security mode '{requestedMode}'. Available endpoints: {available}");
        }

-        // Rewrite endpoint URL hostname to match user-supplied hostname. Necessary
-        // when the OPC UA server returns a discovery URL using a different hostname
-        // (e.g. internal DNS name) than the one the operator routed to.
+        // Rewrite the endpoint URL host AND port to the user-supplied ones (as the SDK's
+        // CoreClientUtils.SelectEndpoint does). A server behind a port mapping or internal DNS
+        // advertises its own host:port (docker-dev containers all say :4840); following that would
+        // open the session on a different server and fail the secure-channel certificate check.
+        // Only when the scheme matches: a different scheme (https vs opc.tcp) is a different transport.
        var serverUri = new Uri(best.EndpointUrl);
        var requestedUri = new Uri(endpointUrl);
-        if (serverUri.Host != requestedUri.Host)
+        if (string.Equals(serverUri.Scheme, requestedUri.Scheme, StringComparison.OrdinalIgnoreCase)
+            && (serverUri.Host != requestedUri.Host || serverUri.Port != requestedUri.Port))
        {
-            var builder = new UriBuilder(best.EndpointUrl) { Host = requestedUri.Host };
+            var builder = new UriBuilder(best.EndpointUrl) { Host = requestedUri.Host, Port = requestedUri.Port };
            best.EndpointUrl = builder.ToString();
-            Logger.Debug("Rewrote endpoint host from {ServerHost} to {RequestedHost}", serverUri.Host,
-                requestedUri.Host);
+            Logger.Debug("Rewrote endpoint from {ServerHost}:{ServerPort} to {RequestedHost}:{RequestedPort}",
+                serverUri.Host, serverUri.Port, requestedUri.Host, requestedUri.Port);
        }

        return best;
@@ -24,6 +24,11 @@ public static class AggregateTypeMapper
            AggregateType.Start => ObjectIds.AggregateFunction_Start,
            AggregateType.End => ObjectIds.AggregateFunction_End,
            AggregateType.StandardDeviation => ObjectIds.AggregateFunction_StandardDeviationPopulation,
+            AggregateType.TimeAverage => ObjectIds.AggregateFunction_TimeAverage,
+            AggregateType.TimeAverage2 => ObjectIds.AggregateFunction_TimeAverage2,
+            AggregateType.Total => ObjectIds.AggregateFunction_Total,
+            AggregateType.Total2 => ObjectIds.AggregateFunction_Total2,
+            AggregateType.Interpolative => ObjectIds.AggregateFunction_Interpolative,
            _ => throw new ArgumentOutOfRangeException(nameof(aggregate), aggregate, "Unknown AggregateType value.")
        };
    }
@@ -24,5 +24,20 @@ public enum AggregateType
    End,

    /// <summary>Population standard deviation of values in the interval.</summary>
-    StandardDeviation
+    StandardDeviation,
+
+    /// <summary>Time-weighted average, interpolated bounds (Part 13 TimeAverage).</summary>
+    TimeAverage,
+
+    /// <summary>Time-weighted average, stepped bounds (Part 13 TimeAverage2).</summary>
+    TimeAverage2,
+
+    /// <summary>Time-weighted integral over the interval (Part 13 Total).</summary>
+    Total,
+
+    /// <summary>Time-weighted integral, stepped bounds (Part 13 Total2).</summary>
+    Total2,
+
+    /// <summary>Interpolated value at the interval start (Part 13 Interpolative).</summary>
+    Interpolative
 }
@@ -53,7 +53,12 @@ public partial class HistoryViewModel : ObservableObject
        AggregateType.Count,
        AggregateType.Start,
        AggregateType.End,
-        AggregateType.StandardDeviation
+        AggregateType.StandardDeviation,
+        AggregateType.TimeAverage,
+        AggregateType.TimeAverage2,
+        AggregateType.Total,
+        AggregateType.Total2,
+        AggregateType.Interpolative
    ];

    /// <summary>Gets a value indicating whether an aggregate read is selected.</summary>
@@ -108,6 +108,9 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
    /// </summary>
    public Dictionary<string, IReadOnlyList<BrowseResult>> BrowseResultsByParent { get; } = new();

+    /// <summary>Gets or sets an exception thrown by HistoryReadRawAsync (simulates a refused read).</summary>
+    public Exception? HistoryReadException { get; set; }
+
    /// <summary>Gets or sets the history read result returned by HistoryReadRawAsync and HistoryReadAggregateAsync.</summary>
    public IReadOnlyList<DataValue> HistoryReadResult { get; set; } = new List<DataValue>
    {
@@ -323,6 +326,7 @@ public sealed class FakeOpcUaClientService : IOpcUaClientService
        NodeId nodeId, DateTime startTime, DateTime endTime, int maxValues = 1000, CancellationToken ct = default)
    {
        HistoryReadRawCalls.Add((nodeId, startTime, endTime, maxValues));
+        if (HistoryReadException != null) throw HistoryReadException;
        return Task.FromResult(HistoryReadResult);
    }

@@ -197,4 +197,53 @@ public class HistoryReadCommandTests
        using var console = TestConsoleHelper.CreateConsole();
        await Should.ThrowAsync<CliFx.Exceptions.CommandException>(async () => await command.ExecuteAsync(console));
    }
+
+    /// <summary>#677.5: the new aggregate names parse (and the aliases).</summary>
+    [Theory]
+    [InlineData("TimeAverage", AggregateType.TimeAverage)]
+    [InlineData("timeaverage2", AggregateType.TimeAverage2)]
+    [InlineData("Total", AggregateType.Total)]
+    [InlineData("total2", AggregateType.Total2)]
+    [InlineData("Interpolative", AggregateType.Interpolative)]
+    [InlineData("interp", AggregateType.Interpolative)]
+    public async Task Execute_NewAggregates_Parse(string name, AggregateType expected)
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var command = new HistoryReadCommand(new FakeOpcUaClientServiceFactory(fakeService))
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=HistNode",
+            Aggregate = name,
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        fakeService.HistoryReadAggregateCalls.ShouldHaveSingleItem().Aggregate.ShouldBe(expected);
+    }
+
+    /// <summary>#674.6: a refused read is a one-line CommandException (non-zero exit), not a stack trace.</summary>
+    [Fact]
+    public async Task Execute_RefusedRead_ThrowsCommandException_WithOneLineMessage()
+    {
+        var fakeService = new FakeOpcUaClientService
+        {
+            HistoryReadException = new ServiceResultException(StatusCodes.BadHistoryOperationUnsupported,
+                "HistoryRead of ns=2;s=HistNode failed: BadHistoryOperationUnsupported (0x80B40000)."),
+        };
+        var command = new HistoryReadCommand(new FakeOpcUaClientServiceFactory(fakeService))
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=HistNode",
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        var ex = await Should.ThrowAsync<CliFx.Exceptions.CommandException>(async () => await command.ExecuteAsync(console));
+
+        ex.Message.ShouldContain("BadHistoryOperationUnsupported");
+        ex.Message.ShouldNotContain("\n");
+        ex.Message.ShouldNotContain(" at ");
+        ex.ExitCode.ShouldNotBe(0);
+        fakeService.DisconnectCalled.ShouldBeTrue();
+    }
 }
@@ -160,4 +160,41 @@ public class EndpointSelectorTests
        Should.Throw<ArgumentException>(() =>
            EndpointSelector.SelectBest(Array.Empty<EndpointDescription>(), "", MessageSecurityMode.None));
    }
+
+    /// <summary>#678.5: a server advertising its internal port must not redirect the session off the mapped port.</summary>
+    [Fact]
+    public void SelectBest_RewritesHostAndPort_WhenSchemeMatches()
+    {
+        var endpoints = new[] { Ep(MessageSecurityMode.None, SecurityPolicies.None, "opc.tcp://internal-host:4840/OtOpcUa") };
+
+        var best = EndpointSelector.SelectBest(endpoints, "opc.tcp://localhost:4841", MessageSecurityMode.None);
+
+        var uri = new Uri(best.EndpointUrl);
+        uri.Host.ShouldBe("localhost");
+        uri.Port.ShouldBe(4841);
+        uri.AbsolutePath.ShouldBe("/OtOpcUa");
+    }
+
+    /// <summary>A different scheme (e.g. https endpoint advertised to an opc.tcp caller) is left untouched.</summary>
+    [Fact]
+    public void SelectBest_LeavesEndpointUntouched_WhenSchemeDiffers()
+    {
+        var endpoints = new[] { Ep(MessageSecurityMode.None, SecurityPolicies.None, "https://internal-host:4843/x") };
+
+        var best = EndpointSelector.SelectBest(endpoints, "opc.tcp://localhost:4841", MessageSecurityMode.None);
+
+        new Uri(best.EndpointUrl).Host.ShouldBe("internal-host");
+        new Uri(best.EndpointUrl).Port.ShouldBe(4843);
+    }
+
+    /// <summary>Identical host, port and scheme: nothing to rewrite.</summary>
+    [Fact]
+    public void SelectBest_Unchanged_WhenHostPortAndSchemeMatch()
+    {
+        var endpoints = new[] { Ep(MessageSecurityMode.None, SecurityPolicies.None, "opc.tcp://server:4840") };
+
+        var best = EndpointSelector.SelectBest(endpoints, "opc.tcp://server:4840", MessageSecurityMode.None);
+
+        best.EndpointUrl.ShouldBe("opc.tcp://server:4840");
+    }
 }
@@ -18,6 +18,11 @@ public class AggregateTypeMapperTests
    [InlineData(AggregateType.Start)]
    [InlineData(AggregateType.End)]
    [InlineData(AggregateType.StandardDeviation)]
+    [InlineData(AggregateType.TimeAverage)]
+    [InlineData(AggregateType.TimeAverage2)]
+    [InlineData(AggregateType.Total)]
+    [InlineData(AggregateType.Total2)]
+    [InlineData(AggregateType.Interpolative)]
    public void ToNodeId_ReturnsNonNullForAllValues(AggregateType aggregate)
    {
        var nodeId = AggregateTypeMapper.ToNodeId(aggregate);
@@ -82,4 +87,15 @@ public class AggregateTypeMapperTests
        Should.Throw<ArgumentOutOfRangeException>(() =>
            AggregateTypeMapper.ToNodeId((AggregateType)99));
    }
+
+    /// <summary>#677.5: the time-weighted Part 13 aggregates map to their ObjectIds.</summary>
+    [Fact]
+    public void ToNodeId_TimeWeightedAggregates_MapCorrectly()
+    {
+        AggregateTypeMapper.ToNodeId(AggregateType.TimeAverage).ShouldBe(ObjectIds.AggregateFunction_TimeAverage);
+        AggregateTypeMapper.ToNodeId(AggregateType.TimeAverage2).ShouldBe(ObjectIds.AggregateFunction_TimeAverage2);
+        AggregateTypeMapper.ToNodeId(AggregateType.Total).ShouldBe(ObjectIds.AggregateFunction_Total);
+        AggregateTypeMapper.ToNodeId(AggregateType.Total2).ShouldBe(ObjectIds.AggregateFunction_Total2);
+        AggregateTypeMapper.ToNodeId(AggregateType.Interpolative).ShouldBe(ObjectIds.AggregateFunction_Interpolative);
+    }
 }
@@ -136,6 +136,6 @@ public class ModelConstructionTests
    [Fact]
    public void AggregateType_Enum_HasExpectedValues()
    {
-        Enum.GetValues<AggregateType>().Length.ShouldBe(7);
+        Enum.GetValues<AggregateType>().Length.ShouldBe(12);
    }
 }
@@ -141,7 +141,7 @@ public class HistoryViewModelTests
    public void AggregateTypes_ContainsNullForRaw()
    {
        _vm.AggregateTypes.ShouldContain((AggregateType?)null);
-        _vm.AggregateTypes.Count.ShouldBe(8); // null + 7 enum values
+        _vm.AggregateTypes.Count.ShouldBe(13); // null + 12 enum values
    }

    /// <summary>Verifies that the Clear method resets the view model state.</summary>
@@ -165,6 +165,7 @@ public sealed class NodeManagerHistoryReadTests : IDisposable
            StartTime = DateTime.UtcNow.AddHours(-1),
            EndTime = DateTime.UtcNow,
            NumValuesPerNode = 10,
+            IsReadModified = false, // SDK default is true; this test is about the raw path
        };

        var (_, errors) = InvokeHistoryRead(server, nm, details, nodeId);