From a37ad1c58f07eda9b8cec5b222bdce612432be61 Mon Sep 17 00:00:00 2001 From: OlliMartin Date: Tue, 4 Feb 2025 07:26:18 +0100 Subject: [PATCH 1/6] Handle empty-collection aggregations; Add count method; +AUTs --- .../TransformationChain.Payloads.cs | 21 ++++ .../TransformationChainTests.cs | 49 ++++++++++ .../CliOutputLexer.g4 | 1 + .../CliOutputParser.g4 | 17 +++- .../CliOutputParserImpl.cs | 23 ++++- .../EmptyEnumerationAggregationError.cs | 8 ++ .../Errors/ParserStateError.cs | 14 +++ .../Errors/ValueAggregationError.cs | 8 ++ .../Visitors/TransformationListener.Values.cs | 33 +++++++ .../Visitors/TransformationListener.cs | 9 ++ .../CliOutputParserImplTests.cs | 97 +++++++++++++++++-- .../component-configuration-windows.json | 4 +- 12 files changed, 267 insertions(+), 17 deletions(-) create mode 100644 Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs create mode 100644 Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs create mode 100644 Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs diff --git a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChain.Payloads.cs b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChain.Payloads.cs index 2eced71..a092f05 100644 --- a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChain.Payloads.cs +++ b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChain.Payloads.cs @@ -22,6 +22,19 @@ public static class SampleCommandOutcomes Minimum = 7ms, Maximum = 8ms, Average = 7ms """; + + internal const string PingTimeout = """ + + Pinging 1.1.1.1 with 32 bytes of data: + PING: transmit failed. General failure. + PING: transmit failed. General failure. + PING: transmit failed. General failure. + PING: transmit failed. General failure. + + Ping statistics for 2a00:1450:4001:806::200e: + Packets: Sent = 4, Received = 0, Lost = 4 (100% loss), + + """; } public static class Payloads @@ -86,6 +99,14 @@ public static class Payloads internal static string PingResultMultipleTransformations => InjectMultilineArray(PingResultMultipleTransformationsTemplate, SampleCommandOutcomes.Ping); + /// + /// Simulates a ping cli result when there is, for example, no internet connection. + /// The transformation relies on the presence of at least one value to aggregate, but there is none. + /// Refer to GitHub Issue #35 + /// + internal static string PingTimeoutResultOneTransformation => + InjectMultilineArray(PingResultOneTransformationTemplate, SampleCommandOutcomes.PingTimeout); + private static string InjectMultilineArray(string template, string toInject) { string[] lines = toInject.Split(Environment.NewLine); diff --git a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs index badf744..2c96b57 100644 --- a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs +++ b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs @@ -1,5 +1,6 @@ using System.Text.Json; using FluentAssertions; +using Microsoft.AspNetCore.Mvc; using Oma.WndwCtrl.Abstractions.Model; using Oma.WndwCtrl.Api.IntegrationTests.TestFramework; using Oma.WndwCtrl.CliOutputParser.Interfaces; @@ -53,6 +54,54 @@ public async Task ShouldProcessPingResultInMultipleTransformations() ); } + [Fact] + public async Task ShouldUnwrapParserTransformationResult() + { + string payload = Payloads.PingResultOneTransformation; + using HttpRequestMessage httpRequestMessage = ConstructCommandHttpRequestMessage(payload, isJson: true); + using HttpResponseMessage httpResponse = await _httpClient.SendAsync(httpRequestMessage, _cancelToken); + + const double expected = 7.5; + } + + [Fact] + public async Task ShouldReturnProblemDetailsOnEmptyValueAggregation() + { + string payload = Payloads.PingTimeoutResultOneTransformation; + using HttpRequestMessage httpRequestMessage = ConstructCommandHttpRequestMessage(payload, isJson: true); + using HttpResponseMessage httpResponse = await _httpClient.SendAsync(httpRequestMessage, _cancelToken); + + httpResponse.Should().Be500InternalServerError().And.Satisfy( + response => + { + response.Detail.Should() + .Be( + "Transformation expected at least one value to aggregate, but found none." + ); + } + ); + } + + [Fact] + public async Task ShouldReturn500OnEmptyValueAggregation() + { + string payload = Payloads.PingTimeoutResultOneTransformation; + using HttpRequestMessage httpRequestMessage = ConstructCommandHttpRequestMessage(payload, isJson: true); + using HttpResponseMessage httpResponse = await _httpClient.SendAsync(httpRequestMessage, _cancelToken); + + httpResponse.Should().Be500InternalServerError(); + } + + [Fact] + public async Task ShouldReturnJsonOnEmptyValueAggregation() + { + string payload = Payloads.PingTimeoutResultOneTransformation; + using HttpRequestMessage httpRequestMessage = ConstructCommandHttpRequestMessage(payload, isJson: true); + using HttpResponseMessage httpResponse = await _httpClient.SendAsync(httpRequestMessage, _cancelToken); + + httpResponse.Should().HaveHeader("Content-Type").And.Match("application/json*"); + } + private static void AssertParserResultOneNumber( TransformationOutcome response, double expectedValue diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputLexer.g4 b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputLexer.g4 index 4713300..a3ce6b0 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputLexer.g4 +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputLexer.g4 @@ -25,6 +25,7 @@ MIN : 'Min'; SUM : 'Sum'; AT : 'At'; INDEX : 'Index'; +COUNT : 'Count'; DOT : '.'; LPAREN : '('; diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputParser.g4 b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputParser.g4 index e0bf4b9..dd87a02 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputParser.g4 +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser.Grammar/CliOutputParser.g4 @@ -16,9 +16,15 @@ map : anchorFrom multiply : regexMatch ; reduce : regexYield - | valuesAvg | valuesSum | valuesMin | valuesMax - | valuesFirst | valuesLast - | valuesAt; + | strictValueAggregation + | forgivingValueAggregation + ; + +// Requires at least one value to be populated, otherwise returns an error +strictValueAggregation : valuesAvg | valuesMin | valuesMax | valuesFirst | valuesLast; + +// Returns 0 if no values are populated +forgivingValueAggregation : valuesSum | valuesCount | valuesAt; anchorFrom : ANCHOR DOT FROM LPAREN STRING_LITERAL RPAREN SEMI; anchorTo : ANCHOR DOT TO LPAREN STRING_LITERAL RPAREN SEMI; @@ -28,10 +34,13 @@ regexYield : REGEX DOT YIELD_GROUP LPAREN INT RPAREN SEMI; // Would be cleaner to combine those.. valuesAvg : VALUES DOT AVERAGE LPAREN RPAREN SEMI; -valuesSum : VALUES DOT SUM LPAREN RPAREN SEMI; valuesMin : VALUES DOT MIN LPAREN RPAREN SEMI; valuesMax : VALUES DOT MAX LPAREN RPAREN SEMI; valuesFirst : VALUES DOT FIRST LPAREN RPAREN SEMI; valuesLast : VALUES DOT LAST LPAREN RPAREN SEMI; + +// Forgiving, i.e. falling back (not raising an error) when there are no values in the enumeration +valuesSum : VALUES DOT SUM LPAREN RPAREN SEMI; +valuesCount : VALUES DOT COUNT LPAREN RPAREN SEMI; valuesAt : VALUES DOT (AT | INDEX) LPAREN INT RPAREN SEMI; \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs index 90b3abf..808b65c 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs @@ -3,6 +3,7 @@ using LanguageExt.Common; using Oma.WndwCtrl.CliOutputParser.Interfaces; using Oma.WndwCtrl.CliOutputParser.Visitors; +using static LanguageExt.Prelude; namespace Oma.WndwCtrl.CliOutputParser; @@ -39,12 +40,12 @@ Func transformationListenerFactory Either treeOrError = treeCache.GetOrCreateTree(transformation); - return treeOrError.Map( + return treeOrError.Bind( tree => ExecuteParser(transformationListenerFactory, tree) ); } - private static ParserResult ExecuteParser( + private static Either ExecuteParser( Func transformationListenerFactory, Grammar.CliOutputParser.TransformationContext tree ) @@ -52,13 +53,27 @@ Grammar.CliOutputParser.TransformationContext tree TransformationListener listener = transformationListenerFactory(); ParseTreeWalker walker = new(); - walker.Walk(listener, tree); + Exception? thrownException; + + try + { + walker.Walk(listener, tree); + } + catch (Exception ex) + { + thrownException = ex; + } + + if (listener.Error is not null) + { + return listener.Error; + } List enumeratedList = listener.CurrentValues.ToList(); if (enumeratedList.Count == 1) { - return [enumeratedList.Single(),]; + return Right([enumeratedList.Single(),]); } ParserResult result = []; diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs new file mode 100644 index 0000000..48e5c93 --- /dev/null +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs @@ -0,0 +1,8 @@ +namespace Oma.WndwCtrl.CliOutputParser.Errors; + +public record EmptyEnumerationAggregationError : ValueAggregationError +{ + public EmptyEnumerationAggregationError(string message, bool isExceptional) : base(message, isExceptional) + { + } +} \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs new file mode 100644 index 0000000..3177b32 --- /dev/null +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs @@ -0,0 +1,14 @@ +using Oma.WndwCtrl.Abstractions.Errors; + +namespace Oma.WndwCtrl.CliOutputParser.Errors; + +public record ParserStateError : FlowError +{ + public ParserStateError(string message, bool isExceptional) : base(message, isExceptional) + { + } + + public Exception? ThrownException { get; init; } + + public override Exception ToException() => ThrownException ?? ToErrorException(); +} \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs new file mode 100644 index 0000000..dac1dca --- /dev/null +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs @@ -0,0 +1,8 @@ +namespace Oma.WndwCtrl.CliOutputParser.Errors; + +public record ValueAggregationError : ParserStateError +{ + public ValueAggregationError(string message, bool isExceptional) : base(message, isExceptional) + { + } +} \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs index 92c0904..98c1863 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs @@ -1,4 +1,6 @@ using JetBrains.Annotations; +using Oma.WndwCtrl.CliOutputParser.Errors; +using Oma.WndwCtrl.CliOutputParser.Model; namespace Oma.WndwCtrl.CliOutputParser.Visitors; @@ -114,4 +116,35 @@ public override void ExitValuesAt(Grammar.CliOutputParser.ValuesAtContext contex : itemList[index]; } } + + public override void ExitValuesCount(Grammar.CliOutputParser.ValuesCountContext context) + { + object? result = FoldItemsRecursive(CurrentValues, Fold); + StoreFoldResult(result); + + base.ExitValuesCount(context); + return; + + object Fold(IEnumerable val) + { + object res = val.Count(); + return res; + } + } + + public override void EnterStrictValueAggregation( + Grammar.CliOutputParser.StrictValueAggregationContext context + ) + { + List collapsedValues = CurrentValues.ToList(); + + if (collapsedValues.Count == 0) + { + Error = new EmptyEnumerationAggregationError("No values to average.", isExceptional: true); + return; + } + + CurrentValues = NestedEnumerable.FromEnumerableInternal(collapsedValues, CurrentValues.IsNested); + base.EnterStrictValueAggregation(context); + } } \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.cs index 6559b30..813ce86 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.cs @@ -1,3 +1,4 @@ +using Oma.WndwCtrl.CliOutputParser.Errors; using Oma.WndwCtrl.CliOutputParser.Grammar; using Oma.WndwCtrl.CliOutputParser.Interfaces; using Oma.WndwCtrl.CliOutputParser.Model; @@ -26,6 +27,8 @@ public TransformationListener(IParserLogger log, string input) public NestedEnumerable CurrentValues { get; private set; } + public ParserStateError? Error { get; private set; } + public override void EnterStatement(Grammar.CliOutputParser.StatementContext context) { if (_log.Enabled) @@ -33,6 +36,12 @@ public override void EnterStatement(Grammar.CliOutputParser.StatementContext con _log.Log($"{Environment.NewLine}\t### COMMAND -> {context.GetChild(i: 0).GetText()}"); } + if (Error is not null) + { + // TODO: Check if this really short-circuits everything. + return; + } + base.EnterStatement(context); } } \ No newline at end of file diff --git a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs index 018e2be..a68e1ec 100644 --- a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs +++ b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs @@ -1,6 +1,7 @@ using FluentAssertions; using LanguageExt; using LanguageExt.Common; +using Oma.WndwCtrl.CliOutputParser.Errors; using Oma.WndwCtrl.CliOutputParser.Interfaces; using Oma.WndwCtrl.CliOutputParser.Tests.Fixtures; @@ -47,6 +48,15 @@ 3.g 3.h 3.i private readonly ICliOutputParser _instance = iocContext.Instance; + public static TheoryData StrictAggregations => new() + { + "Min", + "Max", + "Average", + "First", + "Last", + }; + [Fact] public void ShouldParseTransformationSuccessfully() { @@ -202,6 +212,7 @@ Either transformationResult [InlineData("Sum", 45)] [InlineData("First", "9")] [InlineData("Last", "1")] + [InlineData("Count", 9)] public void ShouldApplyAggregateFunctions(string aggregate, object expectedValue) { const string text = "9 8 7 6 5 4 3 2 1"; @@ -237,11 +248,11 @@ Either transformationResult [Fact] public void ShouldCacheTransformations() { - const string input = """ - This is - multiline - text - """; + const string text = """ + This is + multiline + text + """; const string transformationOne = """ Anchor.From('This'); @@ -254,16 +265,86 @@ This is """; Either transformationResult1 - = _instance.Parse(transformationOne, input); + = _instance.Parse(transformationOne, text); Either transformationResult2 - = _instance.Parse(transformationTwo, input); + = _instance.Parse(transformationTwo, text); Either transformationResult3 - = _instance.Parse(transformationOne, input); + = _instance.Parse(transformationOne, text); AssertSingleValue("This is", transformationResult1); AssertSingleValue("multiline", transformationResult2); AssertSingleValue("This is", transformationResult3); } + + [Theory] + [MemberData(nameof(StrictAggregations))] + public void ShouldReturnAggregationErrorOnEmptyValue(string aggregate) + { + const string text = """ + This has no number. + This also has no number. + """; + + string transformation = $""" + Regex.Match($'(\d)'); // -> [] + Values.Index(1); // [] -> [] + Values.{aggregate}(); + """; + + Either transformationResult + = _instance.Parse(transformation, text); + + transformationResult.Match( + Right: val => val.Should().BeNull(), + Left: val => val.Should().BeOfType() + ); + } + + [Theory] + [InlineData("Sum")] + [InlineData("Count")] + public void ShouldReturnZeroForForgivingAggregations(string aggregate) + { + const string text = """ + This has no number. + This also has no number. + """; + + string transformation = $""" + Regex.Match($'(\d)'); // -> [] + Values.Index(1); // [] -> [] + Values.{aggregate}(); + """; + + Either transformationResult + = _instance.Parse(transformation, text); + + AssertSingleValue(expectedValue: 0, transformationResult); + } + + [Theory] + [MemberData(nameof(StrictAggregations))] + public void ShouldPopulatedThrownExceptionOnAggregationError(string aggregate) + { + const string text = """ + This has no number. + This also has no number. + """; + + string transformation = $""" + Regex.Match($'(\d)'); // -> [] + Values.Index(1); // [] -> [] + Values.{aggregate}(); + """; + + Either transformationResult + = _instance.Parse(transformation, text); + + transformationResult.Match( + Right: val => val.Should().BeNull(), + Left: val => val.Should().BeOfType() + ); + } } \ No newline at end of file diff --git a/Oma.WndwCtrl.Configuration/component-configuration-windows.json b/Oma.WndwCtrl.Configuration/component-configuration-windows.json index 2312cef..3598bcb 100644 --- a/Oma.WndwCtrl.Configuration/component-configuration-windows.json +++ b/Oma.WndwCtrl.Configuration/component-configuration-windows.json @@ -25,7 +25,9 @@ "Anchor.To('WAIT_HINT');", "Regex.Match($'STATE[\\s:\\d]*(\\w+)');", "Values.Index(1);" - ] + ], + "returnType": "double", + "cardinality": "single" } ] }, From 5580569b9937301b12856884993fbf4ba05afd2b Mon Sep 17 00:00:00 2001 From: OlliMartin Date: Tue, 4 Feb 2025 07:38:17 +0100 Subject: [PATCH 2/6] Parser changes done. --- .../CliOutputParserImpl.cs | 13 +++++++++++-- .../CliOutputParserImplTests.cs | 5 ++++- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs index 808b65c..9f54bf7 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/CliOutputParserImpl.cs @@ -1,6 +1,7 @@ using Antlr4.Runtime.Tree; using LanguageExt; using LanguageExt.Common; +using Oma.WndwCtrl.Abstractions.Errors; using Oma.WndwCtrl.CliOutputParser.Interfaces; using Oma.WndwCtrl.CliOutputParser.Visitors; using static LanguageExt.Prelude; @@ -53,7 +54,7 @@ Grammar.CliOutputParser.TransformationContext tree TransformationListener listener = transformationListenerFactory(); ParseTreeWalker walker = new(); - Exception? thrownException; + Exception? thrownException = null; try { @@ -66,7 +67,15 @@ Grammar.CliOutputParser.TransformationContext tree if (listener.Error is not null) { - return listener.Error; + return listener.Error with + { + ThrownException = thrownException, + }; + } + + if (thrownException is not null) + { + return new FlowError(new TechnicalError("An unexpected error occurred.", Code: 5_000, thrownException)); } List enumeratedList = listener.CurrentValues.ToList(); diff --git a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs index a68e1ec..3567e0d 100644 --- a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs +++ b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs @@ -344,7 +344,10 @@ Either transformationResult transformationResult.Match( Right: val => val.Should().BeNull(), - Left: val => val.Should().BeOfType() + Left: val => val.Should().BeOfType().And + .Match( + err => err.ToException() is InvalidOperationException + ) ); } } \ No newline at end of file From 7c39344919c86490eeaaa47aa84b236b6f0be8e6 Mon Sep 17 00:00:00 2001 From: OlliMartin Date: Tue, 4 Feb 2025 10:49:54 +0100 Subject: [PATCH 3/6] Add result filter to write Either, Refactor Error hierarchy. --- .../TransformationChainTests.cs | 17 ++- .../EmptyEnumerationAggregationError.cs | 8 +- .../Errors/ParserStateError.cs | 7 +- .../Errors/ProcessingError.cs | 7 +- .../Errors/ValueAggregationError.cs | 2 +- .../Visitors/TransformationListener.Values.cs | 21 +++- .../Errors/CommandError.cs | 6 +- Oma.WndwCtrl.Abstractions/Errors/FlowError.cs | 59 +++++++-- .../Errors/TransformationError.cs | 4 +- .../Controllers/TestController.cs | 20 +-- .../CliOutputParserImplTests.cs | 6 +- .../Errors/SimulatedFlowError.cs | 17 +++ .../Commands/DummyCommandExecutor.cs | 3 +- .../Filters/EitherResultFilter.cs | 116 ++++++++++++++++++ Oma.WndwCtrl.CoreAsp/WebApplicationWrapper.cs | 2 + 15 files changed, 250 insertions(+), 45 deletions(-) create mode 100644 Oma.WndwCtrl.Core/Errors/SimulatedFlowError.cs create mode 100644 Oma.WndwCtrl.CoreAsp/Filters/EitherResultFilter.cs diff --git a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs index 2c96b57..8c8060e 100644 --- a/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs +++ b/.integrationTests/Oma.WndwCtrl.Api.IntegrationTests/Endpoints/TestController/TransformationChainTests.cs @@ -7,6 +7,11 @@ namespace Oma.WndwCtrl.Api.IntegrationTests.Endpoints.TestController; +internal class NestedProblemDetails : ProblemDetails +{ + public List InnerErrors { get; set; } = new(); +} + public sealed partial class TransformationChainTests( MockedCommandExecutorApiFixture mockedCommandExecutorApiFixture ) @@ -71,13 +76,21 @@ public async Task ShouldReturnProblemDetailsOnEmptyValueAggregation() using HttpRequestMessage httpRequestMessage = ConstructCommandHttpRequestMessage(payload, isJson: true); using HttpResponseMessage httpResponse = await _httpClient.SendAsync(httpRequestMessage, _cancelToken); - httpResponse.Should().Be500InternalServerError().And.Satisfy( + httpResponse.Should().Be500InternalServerError().And.Satisfy( response => { response.Detail.Should() .Be( - "Transformation expected at least one value to aggregate, but found none." + "The CLI parser was in an invalid state during the transformation and could not proceed." ); + + response.InnerErrors.Should().HaveCount(expected: 1).And.Satisfy( + pd => pd.Title == + "The CLI parser was in an invalid state during the transformation and could not proceed." + ).And.Satisfy( + pd => pd.Detail == + "Strict aggregation (function: 'Average') requires at least one value to be present, but the collection was empty. This could be caused by an invalid transformation or the input text was in an invalid/irregular format." + ); } ); } diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs index 48e5c93..b983d6e 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/EmptyEnumerationAggregationError.cs @@ -2,7 +2,13 @@ namespace Oma.WndwCtrl.CliOutputParser.Errors; public record EmptyEnumerationAggregationError : ValueAggregationError { - public EmptyEnumerationAggregationError(string message, bool isExceptional) : base(message, isExceptional) + private readonly string _aggregationFunction; + + public EmptyEnumerationAggregationError(string aggregationFunction) { + _aggregationFunction = aggregationFunction; } + + public override string? Detail => + $"Strict aggregation (function: '{_aggregationFunction}') requires at least one value to be present, but the collection was empty. This could be caused by an invalid transformation or the input text was in an invalid/irregular format."; } \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs index 3177b32..3107a88 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ParserStateError.cs @@ -2,12 +2,15 @@ namespace Oma.WndwCtrl.CliOutputParser.Errors; -public record ParserStateError : FlowError +public abstract record ParserStateError : FlowError { - public ParserStateError(string message, bool isExceptional) : base(message, isExceptional) + public ParserStateError(bool isExceptional) : base(isExceptional) { } + public override string Message => + "The CLI parser was in an invalid state during the transformation and could not proceed."; + public Exception? ThrownException { get; init; } public override Exception ToException() => ThrownException ?? ToErrorException(); diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ProcessingError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ProcessingError.cs index 20636ab..c17b450 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ProcessingError.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ProcessingError.cs @@ -2,8 +2,11 @@ namespace Oma.WndwCtrl.CliOutputParser.Errors; -public record ProcessingError(string Message, int Line, int CharPositionInLine) - : FlowError(Message, IsExceptional: false, IsExpected: true); +public record ProcessingError(string ErrorMessage, int Line, int CharPositionInLine) + : FlowError(IsExceptional: false, IsExpected: true) +{ + public override string Message => ErrorMessage; +} public record ProcessingError(string Message, int Line, int CharPositionInLine, TType OffendingSymbol) : ProcessingError(Message, Line, CharPositionInLine) diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs index dac1dca..7278b44 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Errors/ValueAggregationError.cs @@ -2,7 +2,7 @@ namespace Oma.WndwCtrl.CliOutputParser.Errors; public record ValueAggregationError : ParserStateError { - public ValueAggregationError(string message, bool isExceptional) : base(message, isExceptional) + public ValueAggregationError() : base(isExceptional: true) { } } \ No newline at end of file diff --git a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs index 98c1863..f97c6a0 100644 --- a/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs +++ b/Oma.CliOutputParser/Oma.WndwCtrl.CliOutputParser/Visitors/TransformationListener.Values.cs @@ -1,3 +1,4 @@ +using Antlr4.Runtime.Tree; using JetBrains.Annotations; using Oma.WndwCtrl.CliOutputParser.Errors; using Oma.WndwCtrl.CliOutputParser.Model; @@ -140,7 +141,25 @@ Grammar.CliOutputParser.StrictValueAggregationContext context if (collapsedValues.Count == 0) { - Error = new EmptyEnumerationAggregationError("No values to average.", isExceptional: true); + IParseTree aggregationFunctionContext = context.GetChild(i: 0); + + // TODO: This is really stupid.. Instead the parser should be fixed so that all aggregation functions are handled generically. + string aggregationFunction = aggregationFunctionContext switch + { + Grammar.CliOutputParser.ValuesAvgContext => "Average", + Grammar.CliOutputParser.ValuesMinContext => "Min", + Grammar.CliOutputParser.ValuesMaxContext => "Max", + Grammar.CliOutputParser.ValuesFirstContext => "First", + Grammar.CliOutputParser.ValuesLastContext => "Last", + var _ => throw new InvalidOperationException( + $"Unknown aggregation function {aggregationFunctionContext.GetText()}. This is a programming error. Finally fix the parser." + ), + }; + + Error = new EmptyEnumerationAggregationError( + aggregationFunction + ); + return; } diff --git a/Oma.WndwCtrl.Abstractions/Errors/CommandError.cs b/Oma.WndwCtrl.Abstractions/Errors/CommandError.cs index f14a4e9..4966327 100644 --- a/Oma.WndwCtrl.Abstractions/Errors/CommandError.cs +++ b/Oma.WndwCtrl.Abstractions/Errors/CommandError.cs @@ -9,20 +9,24 @@ public record CommandError : FlowError, ICommandExecutionMetadata { protected CommandError(Error other) : base(other) { + Message = other.Message; } protected CommandError(TechnicalError technicalError) : base(technicalError) { + Message = technicalError.Message; } protected CommandError(string message, bool isExceptional, bool isExpected) : base( - message, isExceptional, isExpected ) { + Message = message; } + public override string Message { get; } + [PublicAPI] public Option ExecutionDuration { get; } = Option.None; diff --git a/Oma.WndwCtrl.Abstractions/Errors/FlowError.cs b/Oma.WndwCtrl.Abstractions/Errors/FlowError.cs index 8cf0a86..363ad6c 100644 --- a/Oma.WndwCtrl.Abstractions/Errors/FlowError.cs +++ b/Oma.WndwCtrl.Abstractions/Errors/FlowError.cs @@ -5,9 +5,9 @@ namespace Oma.WndwCtrl.Abstractions.Errors; [method: PublicAPI] -public record FlowError(string Message, bool IsExceptional, bool IsExpected) : Error +public record FlowError(bool IsExceptional, bool IsExpected) : Error { - protected FlowError(Error other) : this(other.Message, other.IsExceptional, other.IsExpected) + protected FlowError(Error other) : this(other.IsExceptional, other.IsExpected) { Code = other.Code; Inner = other; @@ -19,12 +19,25 @@ public FlowError(TechnicalError technicalError) : this((Error)technicalError) } [PublicAPI] - public FlowError(string message, bool isExceptional) : this(message, isExceptional, !isExceptional) + public FlowError(bool isExceptional) : this(isExceptional, !isExceptional) { } + public virtual string? Detail => Inner.Match( + err => + { + return err switch + { + FlowError flowError => flowError.Message, + ManyErrors _ => "Multiple errors occurred. Refer to the nested properties for more details.", + var _ => null, + }; + }, + () => null + ); + public override int Code { get; } - public override string Message { get; } = Message; + public override string Message { get; } = $"An unexpected error occurred processing a flow."; public override bool IsExceptional { get; } = IsExceptional; public override bool IsExpected { get; } = IsExpected; @@ -37,16 +50,38 @@ public override ErrorException ToErrorException() => ); [System.Diagnostics.Contracts.Pure] - public static FlowError NoCommandExecutorFound(ICommand command) => new( - $"No command executor found that handles transformation type {command.GetType().FullName}.", - isExceptional: false - ); + public static FlowError NoCommandExecutorFound(ICommand command) => + new NoCommandExecutorFoundError(command); [System.Diagnostics.Contracts.Pure] - public static FlowError NoTransformerFound(ITransformation transformation) => new( - $"No transformation executor found that handles transformation type {transformation.GetType().FullName}.", - isExceptional: false - ); + public static FlowError NoTransformerFound(ITransformation transformation) => + new NoTransformerFoundError(transformation); public static implicit operator FlowError(TechnicalError error) => new(error); + + public record NoCommandExecutorFoundError : FlowError + { + private readonly string _commandType; + + public NoCommandExecutorFoundError(ICommand command) : base(isExceptional: true) + { + _commandType = command.GetType().FullName ?? "unknown"; + } + + public override string Message => + $"No command executor found that handles transformation type {_commandType}."; + } + + public record NoTransformerFoundError : FlowError + { + private readonly string _transformationName; + + public NoTransformerFoundError(ITransformation transformation) : base(isExceptional: true) + { + _transformationName = transformation.GetType().FullName ?? "unknown"; + } + + public override string Message => + $"No command executor found that handles transformation type {_transformationName}."; + } } \ No newline at end of file diff --git a/Oma.WndwCtrl.Abstractions/Errors/TransformationError.cs b/Oma.WndwCtrl.Abstractions/Errors/TransformationError.cs index 828f69a..1a9c91e 100644 --- a/Oma.WndwCtrl.Abstractions/Errors/TransformationError.cs +++ b/Oma.WndwCtrl.Abstractions/Errors/TransformationError.cs @@ -11,9 +11,7 @@ public TransformationError(Error other) : base(other) [PublicAPI] public TransformationError(string message, bool isExceptional, bool isExpected) : base( - message, - isExceptional, - isExpected + isExceptional ) { } diff --git a/Oma.WndwCtrl.Api/Controllers/TestController.cs b/Oma.WndwCtrl.Api/Controllers/TestController.cs index 818361c..38c46c2 100644 --- a/Oma.WndwCtrl.Api/Controllers/TestController.cs +++ b/Oma.WndwCtrl.Api/Controllers/TestController.cs @@ -33,7 +33,7 @@ [FromServices] [UsedImplicitly] [EndpointSummary("Test Command")] [EndpointDescription("Run an ad-hoc command")] [Produces("application/json")] - public async Task TestCommandAsync([FromBody] ICommand command) + public async Task> TestCommandAsync([FromBody] ICommand command) { // TODO: Obvious security concerns here... // During development ok. @@ -48,23 +48,7 @@ public async Task TestCommandAsync([FromBody] ICommand command) AppendLogsToHeader(); - return flowResult.BiFold( - null!, - Right: (_, outcome) => Ok(outcome), - Left: (_, error) => Problem( - error.Message, - title: $"[{error.Code}] A {error.GetType().Name} occurred.", - statusCode: error.IsExceptional - ? 500 - : 400, - extensions: error.Inner.IsSome - ? new Dictionary - { - ["inner"] = (Error)error.Inner, - } - : null - ) - ); + return flowResult; } [HttpPost("transformation/parser")] diff --git a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs index 3567e0d..63e901c 100644 --- a/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs +++ b/Oma.WndwCtrl.CliOutputParser.Tests/CliOutputParserImplTests.cs @@ -298,7 +298,11 @@ Either transformationResult transformationResult.Match( Right: val => val.Should().BeNull(), - Left: val => val.Should().BeOfType() + Left: val => val.Should().BeOfType().And + .Match( + err => err.Detailgit == + $"Strict aggregation (function: '{aggregate}') requires at least one value to be present, but the collection was empty. This could be caused by an invalid transformation or the input text was in an invalid/irregular format." + ) ); } diff --git a/Oma.WndwCtrl.Core/Errors/SimulatedFlowError.cs b/Oma.WndwCtrl.Core/Errors/SimulatedFlowError.cs new file mode 100644 index 0000000..7e1a3e2 --- /dev/null +++ b/Oma.WndwCtrl.Core/Errors/SimulatedFlowError.cs @@ -0,0 +1,17 @@ +using JetBrains.Annotations; +using Oma.WndwCtrl.Abstractions.Errors; + +namespace Oma.WndwCtrl.Core.Errors; + +[PublicAPI] +public record SimulatedFlowError : FlowError +{ + public SimulatedFlowError(string errorMessage, bool isExceptional) : base(isExceptional) + { + ErrorMessage = errorMessage; + } + + public string ErrorMessage { get; } + + public override string Message => ErrorMessage; +} \ No newline at end of file diff --git a/Oma.WndwCtrl.Core/Executors/Commands/DummyCommandExecutor.cs b/Oma.WndwCtrl.Core/Executors/Commands/DummyCommandExecutor.cs index 937bfc1..de9e44c 100644 --- a/Oma.WndwCtrl.Core/Executors/Commands/DummyCommandExecutor.cs +++ b/Oma.WndwCtrl.Core/Executors/Commands/DummyCommandExecutor.cs @@ -4,6 +4,7 @@ using Oma.WndwCtrl.Abstractions; using Oma.WndwCtrl.Abstractions.Errors; using Oma.WndwCtrl.Abstractions.Model; +using Oma.WndwCtrl.Core.Errors; using Oma.WndwCtrl.Core.Model.Commands; using static LanguageExt.Prelude; @@ -34,7 +35,7 @@ public Task> ExecuteAsync( } else if (command.SimulateFailure) { - result = Left(new FlowError(message, command.IsExceptional, command.IsExpected)); + result = Left(new SimulatedFlowError(message, command.IsExceptional)); } else { diff --git a/Oma.WndwCtrl.CoreAsp/Filters/EitherResultFilter.cs b/Oma.WndwCtrl.CoreAsp/Filters/EitherResultFilter.cs new file mode 100644 index 0000000..2ce9b60 --- /dev/null +++ b/Oma.WndwCtrl.CoreAsp/Filters/EitherResultFilter.cs @@ -0,0 +1,116 @@ +using LanguageExt; +using LanguageExt.Common; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Filters; +using Oma.WndwCtrl.Abstractions.Errors; + +namespace Oma.WndwCtrl.CoreAsp.Filters; + +public class EitherResultFilter : IAsyncResultFilter +{ + public async Task OnResultExecutionAsync(ResultExecutingContext context, ResultExecutionDelegate next) + { + if (context.Result is not ObjectResult { Value: IEither either, }) + { + await next(); + return; + } + + context.Result = either.MatchUntyped( + err => GetErrorResult(context, err), + obj => new OkObjectResult(obj) + ); + + await next(); + } + + private static IActionResult GetErrorResult(ResultExecutingContext context, object? error) + { + if (error is null) + { + return GetGenericServerError(context); + } + + if (error is Error langExtError) + { + return Problem(context, langExtError.ToProblemDetails()); + } + + return GetGenericServerError(context); + } + + private static ObjectResult GetGenericServerError(ResultExecutingContext context) + => Problem(context, title: "An unexpected error occurred.", statusCode: 500); + + private static ObjectResult Problem( + ResultExecutingContext context, + string? detail = null, + string? instance = null, + int? statusCode = null, + string? title = null, + string? type = null, + IDictionary? extensions = null + ) + { + if (context.Controller is not ControllerBase controllerBase) + { + throw new InvalidOperationException("Writing problem details without a controller is not supported."); + } + + return controllerBase.Problem(detail, instance, statusCode, title, type, extensions); + } + + private static ObjectResult Problem(ResultExecutingContext context, ProblemDetails problemDetails) + => Problem( + context, + problemDetails.Detail, + problemDetails.Instance, + problemDetails.Status, + problemDetails.Title, + problemDetails.Type, + problemDetails.Extensions + ); +} + +public static class FlowErrorExtensions +{ + public static ProblemDetails ToProblemDetails(this Error error) + { + Option> inner = error.Inner.Map( + innerError => + { + if (innerError is ManyErrors many) + { + return many.Errors.Select(e => e.ToProblemDetails()); + } + + return [innerError.ToProblemDetails(),]; + } + ); + + ProblemDetails details = new() + { + Title = error.Message, + Status = error.IsExceptional + ? 500 + : 400, + }; + + if (error is FlowError fErr && !string.IsNullOrEmpty(fErr.Detail)) + { + details.Detail = fErr.Detail; + } + + inner.Do( + innerErrors => + { + details.Extensions = new Dictionary() + { + { "innerErrors", innerErrors.ToList() }, + }; + } + ); + + return details; + } +} \ No newline at end of file diff --git a/Oma.WndwCtrl.CoreAsp/WebApplicationWrapper.cs b/Oma.WndwCtrl.CoreAsp/WebApplicationWrapper.cs index 2ac246d..e282192 100644 --- a/Oma.WndwCtrl.CoreAsp/WebApplicationWrapper.cs +++ b/Oma.WndwCtrl.CoreAsp/WebApplicationWrapper.cs @@ -9,6 +9,7 @@ using Oma.WndwCtrl.Core.Model.Settings; using Oma.WndwCtrl.CoreAsp.Api.Filters; using Oma.WndwCtrl.CoreAsp.Conventions; +using Oma.WndwCtrl.CoreAsp.Filters; using Oma.WndwCtrl.Messaging.Bus; using Oma.WndwCtrl.Messaging.Extensions; using OpenTelemetry.Logs; @@ -112,6 +113,7 @@ public async Task StartAsync(CancellationToken cancelToken = default, params str opts.Conventions.Add(new ContainingAssemblyApplicationModelConvention()); opts.Filters.Add(); + opts.Filters.Add(); PostConfigureMvcOptions(opts); } From d6a16f36455740a36328c652210f2bd42fe6e7f7 Mon Sep 17 00:00:00 2001 From: OlliMartin Date: Tue, 4 Feb 2025 10:52:57 +0100 Subject: [PATCH 4/6] Return Either instead of IActionResult --- .../Components/ButtonController.cs | 6 ++++- .../Components/ComponentControllerBase.cs | 14 ++-------- .../Components/SensorController.cs | 6 ++++- .../Components/SwitchController.cs | 9 ++++--- .../Controllers/TestController.cs | 27 +++---------------- .../CliOutputParserImplTests.cs | 2 +- 6 files changed, 23 insertions(+), 41 deletions(-) diff --git a/Oma.WndwCtrl.Api/Controllers/Components/ButtonController.cs b/Oma.WndwCtrl.Api/Controllers/Components/ButtonController.cs index c7b0c9c..ff7f5a7 100644 --- a/Oma.WndwCtrl.Api/Controllers/Components/ButtonController.cs +++ b/Oma.WndwCtrl.Api/Controllers/Components/ButtonController.cs @@ -1,6 +1,9 @@ using System.Diagnostics.CodeAnalysis; +using LanguageExt; using Microsoft.AspNetCore.Mvc; +using Oma.WndwCtrl.Abstractions.Errors; using Oma.WndwCtrl.Abstractions.Messaging.Model.ComponentExecution; +using Oma.WndwCtrl.Abstractions.Model; using Oma.WndwCtrl.Api.Attributes; using Oma.WndwCtrl.Core.Model; @@ -17,5 +20,6 @@ public class ButtonController : ComponentControllerBase