fix(client): offer exactly the served aggregates in the CLI and UI

R08 2.3: AggregateType drops StandardDeviation (no backend serves it; always BadAggregateNotSupported) and gains Range, Delta, MinimumActualTime, MaximumActualTime, PercentGood and PercentBad. A parity test pins the enum to AggregateFamilies.Served and each member to its same-named Part 13 ObjectId. The CLI parses every enum name (plus aliases) and the UI picker lists them all. Existing members keep their numeric values; 6 is retired. Refs #747 Claude-Session: https://claude.ai/code/session_01Ub9hGmYLQvn9XETzw7CMsT
2026-10-22 16:24:18 -04:00
parent 57ae69a05e
commit a74a18e49a
9 changed files with 200 additions and 62 deletions
@@ -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, TimeAverage, TimeAverage2, Total, Total2, Interpolative")]
+    [CommandOption("aggregate", Description = "Aggregate function: Average, Minimum, Maximum, Count, Start, End, TimeAverage, TimeAverage2, Total, Total2, Interpolative, Range, Delta, MinimumActualTime, MaximumActualTime, PercentGood, PercentBad")]
    public string? Aggregate { get; init; }

    /// <summary>
@@ -199,24 +199,33 @@ public class HistoryReadCommand : CommandBase
        }
    }

+    private static readonly string SupportedAggregates = string.Join(", ", Enum.GetNames<AggregateType>());
+
    private static AggregateType ParseAggregateType(string name)
    {
-        return name.Trim().ToLowerInvariant() switch
+        var trimmed = name.Trim();
+        var alias = trimmed.ToLowerInvariant() switch
        {
-            "average" or "avg" => AggregateType.Average,
-            "minimum" or "min" => AggregateType.Minimum,
-            "maximum" or "max" => AggregateType.Maximum,
-            "count" => AggregateType.Count,
-            "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, TimeAverage, TimeAverage2, Total, Total2, Interpolative")
+            "avg" => AggregateType.Average,
+            "min" => AggregateType.Minimum,
+            "max" => AggregateType.Maximum,
+            "first" => AggregateType.Start,
+            "last" => AggregateType.End,
+            "timeavg" => AggregateType.TimeAverage,
+            "timeavg2" => AggregateType.TimeAverage2,
+            "interp" => AggregateType.Interpolative,
+            "mintime" => AggregateType.MinimumActualTime,
+            "maxtime" => AggregateType.MaximumActualTime,
+            _ => (AggregateType?)null
        };
+        if (alias is { } a) return a;
+
+        // Every enum name (the served set; a parity test pins it), case-insensitive. Numeric strings are refused:
+        // Enum.TryParse would accept "6" or "99" as an undefined value.
+        if (!int.TryParse(trimmed, out _) &&
+            Enum.TryParse<AggregateType>(trimmed, ignoreCase: true, out var parsed) && Enum.IsDefined(parsed))
+            return parsed;
+
+        throw new ArgumentException($"Unknown aggregate: '{name}'. Supported: {SupportedAggregates}");
    }
 }
@@ -23,12 +23,17 @@ public static class AggregateTypeMapper
            AggregateType.Count => ObjectIds.AggregateFunction_Count,
            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,
+            AggregateType.Range => ObjectIds.AggregateFunction_Range,
+            AggregateType.Delta => ObjectIds.AggregateFunction_Delta,
+            AggregateType.MinimumActualTime => ObjectIds.AggregateFunction_MinimumActualTime,
+            AggregateType.MaximumActualTime => ObjectIds.AggregateFunction_MaximumActualTime,
+            AggregateType.PercentGood => ObjectIds.AggregateFunction_PercentGood,
+            AggregateType.PercentBad => ObjectIds.AggregateFunction_PercentBad,
            _ => throw new ArgumentOutOfRangeException(nameof(aggregate), aggregate, "Unknown AggregateType value.")
        };
    }
@@ -1,43 +1,64 @@
 namespace ZB.MOM.WW.OtOpcUa.Client.Shared.Models;

 /// <summary>
-///     Aggregate functions for processed history reads.
+///     Aggregate functions for processed history reads — exactly the seventeen the server's SQL historian backends
+///     serve (<c>AggregateFamilies.Served</c>; a parity test pins it). The Gateway backend serves a subset and refuses
+///     the rest with <c>BadAggregateNotSupported</c>.
 /// </summary>
+/// <remarks>
+///     Values are explicit so the pre-existing members keep their numbers. 6 (the former <c>StandardDeviation</c>,
+///     which no backend serves) is retired; do not reuse it.
+/// </remarks>
 public enum AggregateType
 {
    /// <summary>Average of values in the interval.</summary>
-    Average,
+    Average = 0,

    /// <summary>Minimum value in the interval.</summary>
-    Minimum,
+    Minimum = 1,

    /// <summary>Maximum value in the interval.</summary>
-    Maximum,
+    Maximum = 2,

    /// <summary>Count of values in the interval.</summary>
-    Count,
+    Count = 3,

    /// <summary>First value in the interval.</summary>
-    Start,
+    Start = 4,

    /// <summary>Last value in the interval.</summary>
-    End,
-
-    /// <summary>Population standard deviation of values in the interval.</summary>
-    StandardDeviation,
+    End = 5,

    /// <summary>Time-weighted average, interpolated bounds (Part 13 TimeAverage).</summary>
-    TimeAverage,
+    TimeAverage = 7,

    /// <summary>Time-weighted average, stepped bounds (Part 13 TimeAverage2).</summary>
-    TimeAverage2,
+    TimeAverage2 = 8,

    /// <summary>Time-weighted integral over the interval (Part 13 Total).</summary>
-    Total,
+    Total = 9,

    /// <summary>Time-weighted integral, stepped bounds (Part 13 Total2).</summary>
-    Total2,
+    Total2 = 10,

    /// <summary>Interpolated value at the interval start (Part 13 Interpolative).</summary>
-    Interpolative
-}
+    Interpolative = 11,
+
+    /// <summary>Maximum minus minimum in the interval (Part 13 Range).</summary>
+    Range = 12,
+
+    /// <summary>Last minus first value in the interval (Part 13 Delta).</summary>
+    Delta = 13,
+
+    /// <summary>Minimum value with its own timestamp (Part 13 MinimumActualTime).</summary>
+    MinimumActualTime = 14,
+
+    /// <summary>Maximum value with its own timestamp (Part 13 MaximumActualTime).</summary>
+    MaximumActualTime = 15,
+
+    /// <summary>Percentage of the interval with Good data (Part 13 PercentGood).</summary>
+    PercentGood = 16,
+
+    /// <summary>Percentage of the interval with Bad data (Part 13 PercentBad).</summary>
+    PercentBad = 17
+}
@@ -43,23 +43,9 @@ public partial class HistoryViewModel : ObservableObject
        _dispatcher = dispatcher;
    }

-    /// <summary>Available aggregate types (null means "Raw").</summary>
+    /// <summary>Available aggregate types (null means "Raw"): every client aggregate, which is the served set.</summary>
    public IReadOnlyList<AggregateType?> AggregateTypes { get; } =
-    [
-        null,
-        AggregateType.Average,
-        AggregateType.Minimum,
-        AggregateType.Maximum,
-        AggregateType.Count,
-        AggregateType.Start,
-        AggregateType.End,
-        AggregateType.StandardDeviation,
-        AggregateType.TimeAverage,
-        AggregateType.TimeAverage2,
-        AggregateType.Total,
-        AggregateType.Total2,
-        AggregateType.Interpolative
-    ];
+        [null, .. Enum.GetValues<AggregateType>().Select(a => (AggregateType?)a)];

    /// <summary>Gets a value indicating whether an aggregate read is selected.</summary>
    public bool IsAggregateRead => SelectedAggregateType != null;
@@ -0,0 +1,87 @@
+using CliFx.Exceptions;
+using Shouldly;
+using Xunit;
+using ZB.MOM.WW.OtOpcUa.Client.CLI.Commands;
+using ZB.MOM.WW.OtOpcUa.Client.CLI.Tests.Fakes;
+using ZB.MOM.WW.OtOpcUa.Client.Shared.Models;
+
+namespace ZB.MOM.WW.OtOpcUa.Client.CLI.Tests;
+
+/// <summary>
+/// <c>historyread --aggregate</c> accepts every aggregate the client offers (which is exactly the served set), so the
+/// gate recipes in docs/Historian.md that need Range / Delta / PercentGood / ... can run from this CLI.
+/// </summary>
+public class HistoryReadAggregateParsingTests
+{
+    /// <summary>Every client aggregate, by its name.</summary>
+    /// <returns>Each <see cref="AggregateType"/> value.</returns>
+    public static TheoryData<AggregateType> AllAggregates() => new(Enum.GetValues<AggregateType>());
+
+    /// <summary>Every aggregate name (in any case) reaches the service as that aggregate.</summary>
+    /// <param name="aggregate">The aggregate.</param>
+    [Theory]
+    [MemberData(nameof(AllAggregates))]
+    public async Task Every_aggregate_name_parses(AggregateType aggregate)
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var command = new HistoryReadCommand(new FakeOpcUaClientServiceFactory(fakeService))
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=HistNode",
+            Aggregate = aggregate.ToString().ToUpperInvariant()
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        fakeService.HistoryReadAggregateCalls.ShouldHaveSingleItem().Aggregate.ShouldBe(aggregate);
+    }
+
+    /// <summary>The short aliases still parse.</summary>
+    /// <param name="alias">The alias.</param>
+    /// <param name="expected">The aggregate it names.</param>
+    [Theory]
+    [InlineData("avg", AggregateType.Average)]
+    [InlineData("min", AggregateType.Minimum)]
+    [InlineData("last", AggregateType.End)]
+    [InlineData("interp", AggregateType.Interpolative)]
+    [InlineData("mintime", AggregateType.MinimumActualTime)]
+    [InlineData("maxtime", AggregateType.MaximumActualTime)]
+    public async Task Aliases_parse(string alias, AggregateType expected)
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var command = new HistoryReadCommand(new FakeOpcUaClientServiceFactory(fakeService))
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=HistNode",
+            Aggregate = alias
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        await command.ExecuteAsync(console);
+
+        fakeService.HistoryReadAggregateCalls.ShouldHaveSingleItem().Aggregate.ShouldBe(expected);
+    }
+
+    /// <summary>
+    /// StandardDeviation is no longer offered (no backend serves it): it is refused before connecting, naming the
+    /// supported set.
+    /// </summary>
+    [Fact]
+    public async Task StandardDeviation_is_refused_before_connecting()
+    {
+        var fakeService = new FakeOpcUaClientService();
+        var command = new HistoryReadCommand(new FakeOpcUaClientServiceFactory(fakeService))
+        {
+            Url = "opc.tcp://localhost:4840",
+            NodeId = "ns=2;s=HistNode",
+            Aggregate = "stddev"
+        };
+
+        using var console = TestConsoleHelper.CreateConsole();
+        var ex = await Should.ThrowAsync<CommandException>(async () => await command.ExecuteAsync(console));
+
+        ex.Message.ShouldContain("PercentGood");
+        fakeService.HistoryReadAggregateCalls.ShouldBeEmpty();
+    }
+}
@@ -3,6 +3,7 @@ using Shouldly;
 using Xunit;
 using ZB.MOM.WW.OtOpcUa.Client.Shared.Helpers;
 using ZB.MOM.WW.OtOpcUa.Client.Shared.Models;
+using ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common;

 namespace ZB.MOM.WW.OtOpcUa.Client.Shared.Tests.Helpers;

@@ -17,7 +18,6 @@ public class AggregateTypeMapperTests
    [InlineData(AggregateType.Count)]
    [InlineData(AggregateType.Start)]
    [InlineData(AggregateType.End)]
-    [InlineData(AggregateType.StandardDeviation)]
    [InlineData(AggregateType.TimeAverage)]
    [InlineData(AggregateType.TimeAverage2)]
    [InlineData(AggregateType.Total)]
@@ -72,14 +72,6 @@ public class AggregateTypeMapperTests
        AggregateTypeMapper.ToNodeId(AggregateType.End).ShouldBe(ObjectIds.AggregateFunction_End);
    }

-    /// <summary>Verifies that StandardDeviation aggregate type maps to the correct OPC UA node ID.</summary>
-    [Fact]
-    public void ToNodeId_StandardDeviation_MapsCorrectly()
-    {
-        AggregateTypeMapper.ToNodeId(AggregateType.StandardDeviation)
-            .ShouldBe(ObjectIds.AggregateFunction_StandardDeviationPopulation);
-    }
-
    /// <summary>Verifies that invalid aggregate type value throws ArgumentOutOfRangeException.</summary>
    [Fact]
    public void ToNodeId_InvalidValue_Throws()
@@ -98,4 +90,31 @@ public class AggregateTypeMapperTests
        AggregateTypeMapper.ToNodeId(AggregateType.Total2).ShouldBe(ObjectIds.AggregateFunction_Total2);
        AggregateTypeMapper.ToNodeId(AggregateType.Interpolative).ShouldBe(ObjectIds.AggregateFunction_Interpolative);
    }
-}
+
+    /// <summary>
+    ///     The client offers exactly the aggregates the server's SQL backends serve (<see cref="AggregateFamilies.Served"/>):
+    ///     nothing the server always refuses (the old <c>StandardDeviation</c>), nothing served left unreachable
+    ///     (Range, Delta, Min/MaxActualTime, PercentGood/Bad).
+    /// </summary>
+    [Fact]
+    public void Client_aggregates_are_exactly_the_served_set()
+    {
+        Enum.GetNames<AggregateType>().Order()
+            .ShouldBe(AggregateFamilies.Served.Select(a => a.ToString()).Order());
+    }
+
+    /// <summary>Each aggregate maps to the Part 13 ObjectId of the same name.</summary>
+    /// <param name="aggregate">The aggregate.</param>
+    [Theory]
+    [MemberData(nameof(AllAggregates))]
+    public void ToNodeId_maps_to_the_same_named_AggregateFunction(AggregateType aggregate)
+    {
+        var field = typeof(ObjectIds).GetField("AggregateFunction_" + aggregate);
+        field.ShouldNotBeNull();
+        AggregateTypeMapper.ToNodeId(aggregate).ShouldBe((NodeId)field.GetValue(null)!);
+    }
+
+    /// <summary>Every client aggregate.</summary>
+    /// <returns>Each <see cref="AggregateType"/> value.</returns>
+    public static TheoryData<AggregateType> AllAggregates() => new(Enum.GetValues<AggregateType>());
+}
@@ -132,10 +132,10 @@ public class ModelConstructionTests
        ((int)ConnectionState.Reconnecting).ShouldBe(3);
    }

-    /// <summary>Verifies that AggregateType enum has the expected number of values.</summary>
+    /// <summary>AggregateType holds the seventeen served aggregates (the parity test pins which).</summary>
    [Fact]
    public void AggregateType_Enum_HasExpectedValues()
    {
-        Enum.GetValues<AggregateType>().Length.ShouldBe(12);
+        Enum.GetValues<AggregateType>().Length.ShouldBe(17);
    }
 }
@@ -21,6 +21,8 @@

    <ItemGroup>
        <ProjectReference Include="..\..\..\src\Client\ZB.MOM.WW.OtOpcUa.Client.Shared\ZB.MOM.WW.OtOpcUa.Client.Shared.csproj"/>
+        <!-- Test-only: the served-aggregate parity oracle (AggregateFamilies.Served). -->
+        <ProjectReference Include="..\..\..\src\Drivers\ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common\ZB.MOM.WW.OtOpcUa.Driver.Historian.Sql.Common.csproj"/>
    </ItemGroup>

 </Project>
@@ -141,7 +141,16 @@ public class HistoryViewModelTests
    public void AggregateTypes_ContainsNullForRaw()
    {
        _vm.AggregateTypes.ShouldContain((AggregateType?)null);
-        _vm.AggregateTypes.Count.ShouldBe(13); // null + 12 enum values
+        _vm.AggregateTypes.Count.ShouldBe(Enum.GetValues<AggregateType>().Length + 1); // null + every aggregate
+    }
+
+    /// <summary>The picker offers every client aggregate (the served set), including Range/Delta/PercentGood/...</summary>
+    [Fact]
+    public void AggregateTypes_offers_every_aggregate()
+    {
+        _vm.AggregateTypes.Where(a => a != null).Select(a => a!.Value)
+            .ShouldBe(Enum.GetValues<AggregateType>(), ignoreOrder: true);
+        _vm.AggregateTypes.ShouldContain(AggregateType.PercentGood);
    }

    /// <summary>Verifies that the Clear method resets the view model state.</summary>