diff --git a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs index a7658c8c1a..d3264af095 100644 --- a/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs @@ -1,3 +1,4 @@ +using System.Diagnostics; using System.Globalization; using System.Text.Json; using System.Text.RegularExpressions; @@ -80,12 +81,13 @@ internal static async Task EvaluateExpression_internal( ExpressionValue[]? positionalArguments = null ) { - if (!expr.IsFunctionExpression) + if (expr.IsLiteralValue) { return expr.ValueUnion; } + ValidateExpressionArgs(expr); - var args = new ExpressionValue[expr.Args.Count]; + var args = new ExpressionValue[expr.Args.Length]; for (var i = 0; i < args.Length; i++) { args[i] = await EvaluateExpression_internal(state, expr.Args[i], context, positionalArguments); @@ -93,6 +95,7 @@ internal static async Task EvaluateExpression_internal( ExpressionValue ret = expr.Function switch { + //ExpressionFunction.LITERAL_VALUE => expr.ValueUnion, // Handled above ExpressionFunction.dataModel => await DataModel(args, context, state), ExpressionFunction.component => await Component(args, context, state), ExpressionFunction.countDataElements => CountDataElements(args, state), @@ -129,27 +132,31 @@ internal static async Task EvaluateExpression_internal( ExpressionFunction.argv => Argv(args, positionalArguments), ExpressionFunction.gatewayAction => state.GetGatewayAction(), ExpressionFunction.language => state.GetLanguage(), - _ => throw new ExpressionEvaluatorTypeErrorException("Function not implemented", expr.Function, args), + ExpressionFunction.INVALID => throw new ExpressionEvaluatorTypeErrorException( + $"Function {expr.Args.FirstOrDefault()} not implemented in backend {expr}" + ), + _ => throw new UnreachableException($"Function {(int)expr.Function} not a valid enum value {expr}"), }; return ret; } private static void ValidateExpressionArgs(Expression expr) { + // Some functions have restrictions that arguments must be literal values and not subexpressions. switch (expr) { - case { Function: ExpressionFunction.dataModel, Args: [_, { IsFunctionExpression: true }] }: + case { Function: ExpressionFunction.dataModel, Args: [_, { IsLiteralValue: false }] }: throw new ExpressionEvaluatorTypeErrorException( "The data type must be a string (expressions cannot be used here)" ); - case { Function: ExpressionFunction.@if, Args: [_, _, { IsFunctionExpression: true }, _] }: + case { Function: ExpressionFunction.@if, Args: [_, _, { IsLiteralValue: false }, _] }: throw new ExpressionEvaluatorTypeErrorException("Expected third argument to be \"else\""); - case { Function: ExpressionFunction.compare, Args: [_, { IsFunctionExpression: true }, _] }: - case { Function: ExpressionFunction.compare, Args: [_, _, { IsFunctionExpression: true }, _] }: + case { Function: ExpressionFunction.compare, Args: [_, { IsLiteralValue: false }, _] }: + case { Function: ExpressionFunction.compare, Args: [_, _, { IsLiteralValue: false }, _] }: throw new ExpressionEvaluatorTypeErrorException( "Invalid operator (it cannot be an expression or null)" ); - case { Function: ExpressionFunction.compare, Args: [_, { IsFunctionExpression: true }, _, _] }: + case { Function: ExpressionFunction.compare, Args: [_, { IsLiteralValue: false }, _, _] }: throw new ExpressionEvaluatorTypeErrorException( "Second argument must be \"not\" when providing 4 arguments in total" ); @@ -333,7 +340,7 @@ private static bool Contains(ExpressionValue[] args) date = TimeZoneInfo.ConvertTime(date.Value, timezone); } - string? language = state.GetLanguage(); + string language = state.GetLanguage(); return UnicodeDateTimeTokenConverter.Format( date, args.Length == 2 ? args[1].ToStringForEquals() : null, @@ -653,12 +660,7 @@ private static string Round(ExpressionValue[] args) ); } - var number = PrepareNumericArg(args[0]); - - if (number is null) - { - number = 0; - } + var number = PrepareNumericArg(args[0]) ?? 0; int precision = 0; @@ -667,7 +669,7 @@ private static string Round(ExpressionValue[] args) precision = (int)(PrepareNumericArg(args[1]) ?? 0); } - return number.Value.ToString($"N{precision}", CultureInfo.InvariantCulture); + return number.ToString($"N{precision}", CultureInfo.InvariantCulture); } private static string? UpperCase(ExpressionValue[] args) @@ -770,9 +772,17 @@ private static bool PrepareBooleanArg(ExpressionValue arg) throw new ExpressionEvaluatorTypeErrorException("Expected 1+ argument(s), got 0"); } - var preparedArgs = args.Select(arg => PrepareBooleanArg(arg)).ToArray(); - // Ensure all args gets converted, because they might throw an Exception - return preparedArgs.All(a => a); + var all = true; + foreach (var arg in args) + { + // the LINQ All() method would short-circuit and not evaluate all args, so we do it manually to ensure exceptions are thrown correctly + if (!PrepareBooleanArg(arg)) + { + all = false; + } + } + + return all; } private static async Task Text( @@ -804,9 +814,17 @@ ExpressionValue[] args throw new ExpressionEvaluatorTypeErrorException("Expected 1+ argument(s), got 0"); } - var preparedArgs = args.Select(arg => PrepareBooleanArg(arg)).ToArray(); - // Ensure all args gets converted, because they might throw an Exception - return preparedArgs.Any(a => a); + bool any = false; + foreach (var arg in args) + { + // the LINQ Any() method would short-circuit and not evaluate all args, so we do it manually to ensure exceptions are thrown correctly + if (PrepareBooleanArg(arg)) + { + any = true; + } + } + + return any; } private static bool? Not(ExpressionValue[] args) diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs index 7249aa990c..2a58f33800 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs @@ -270,7 +270,7 @@ public async Task AddInidicies(ModelBinding binding, ComponentCon $"Failed to add indexes to path {binding.Field} with indexes " + $"{(context.RowIndices is null ? "null" : string.Join(", ", context.RowIndices))} on {dataElementId}" ); - ; + return new DataReference() { Field = field, DataElementIdentifier = dataElementId }; } @@ -401,7 +401,7 @@ public async Task TranslateText(string textKey, ComponentContext context // private void GetModelErrorsForExpression(Expression expr, BaseComponent component, List errors) // { - // if (!expr.IsFunctionExpression) + // if (expr.IsLiteralValue) // { // return; // } diff --git a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs index f9aec5e3f8..c58c761a89 100644 --- a/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs +++ b/src/Altinn.App.Core/Models/Expressions/ComponentContext.cs @@ -220,9 +220,9 @@ public DebuggerEvaluatedExpression(Expression expression, ComponentContext conte public IEnumerable? Args => _expression.Args?.Select(e => new DebuggerEvaluatedExpression(e, _context)); public Task EvaluationResult => - _expression.IsFunctionExpression - ? ExpressionEvaluator.EvaluateExpression_internal(_context.State, _expression, _context, null) - : Task.FromResult(_expression.ValueUnion); + _expression.IsLiteralValue + ? Task.FromResult(_expression.ValueUnion) + : ExpressionEvaluator.EvaluateExpression_internal(_context.State, _expression, _context, null); public override string ToString() { diff --git a/src/Altinn.App.Core/Models/Expressions/Expression.cs b/src/Altinn.App.Core/Models/Expressions/Expression.cs index 0b1e421d1e..28ed5a517b 100644 --- a/src/Altinn.App.Core/Models/Expressions/Expression.cs +++ b/src/Altinn.App.Core/Models/Expressions/Expression.cs @@ -15,60 +15,75 @@ namespace Altinn.App.Core.Models.Expressions; public readonly struct Expression : IEquatable { /// - /// Construct a value expression with the given value + /// Construct a value expression with the given literal value /// /// public Expression(ExpressionValue value) { + Function = ExpressionFunction.LITERAL_VALUE; + Args = null; ValueUnion = value; } /// - /// Construct a value expression with the given value + /// Construct a function expression with the given function and arguments /// - /// - [Obsolete("Use the constructor with ExpressionValue instead")] - public Expression(object? value) + public Expression(ExpressionFunction function, Expression[] args) { - ValueUnion = ExpressionValue.FromObject(value); + if (function == ExpressionFunction.LITERAL_VALUE) + { + throw new ArgumentException("Function LITERAL_VALUE cannot have arguments", nameof(function)); + } + if (args == null) + { + throw new ArgumentNullException( + nameof(args), + "Args cannot be null when constructing a function expression" + ); + } + Function = function; + Args = args; } /// /// Construct a function expression with the given function and arguments /// - public Expression(ExpressionFunction function, List? args) - { - Function = function; - Args = args; - } + [Obsolete("Use the constructor with Expression[] instead")] + public Expression(ExpressionFunction function, List args) + : this(function, args.ToArray()) { } + + /// + /// Construct a value expression with the given value + /// + /// + [Obsolete("Use the constructor with ExpressionValue instead")] + public Expression(object? value) + : this(ExpressionValue.FromObject(value)) { } /// /// Construct a function expression with the given function and arguments /// public Expression(ExpressionFunction function, Expression arg1) - { - Function = function; - Args = [arg1]; - } + : this(function, new[] { arg1 }) { } /// /// Construct a function expression with the given function and arguments /// public Expression(ExpressionFunction function, Expression arg1, Expression arg2) - { - Function = function; - Args = [arg1, arg2]; - } + : this(function, new[] { arg1, arg2 }) { } /// /// Test function to see if this is representing a function with args. /// - [MemberNotNullWhen(true, nameof(Function), nameof(Args))] -#pragma warning disable CS0618 // Type or member is obsolete - [MemberNotNullWhen(false, nameof(Value))] -#pragma warning restore CS0618 // Type or member is obsolete - [MemberNotNullWhen(false, nameof(ValueUnion))] - public bool IsFunctionExpression => Function != ExpressionFunction.INVALID && Args != null; + [MemberNotNullWhen(true, nameof(Args))] + [Obsolete("Use !IsLiteralValue instead")] + public bool IsFunctionExpression => !IsLiteralValue; + + /// + /// Test to see if this expression must be evaluated or is just a literal value. + /// + [MemberNotNullWhen(false, nameof(Args))] + public bool IsLiteralValue => Args == null; /// /// Name of the function. Must be one those actually implemented in @@ -78,7 +93,7 @@ public Expression(ExpressionFunction function, Expression arg1, Expression arg2) /// /// List of arguments to the function. These expressions will be evaluated before passed to the function. /// - public List? Args { get; } + public Expression[]? Args { get; } /// /// Get the object value for backwards compatibility @@ -115,7 +130,8 @@ public Expression(ExpressionFunction function, Expression arg1, Expression arg2) public bool IsLiteralString => ValueUnion.ValueKind == JsonValueKind.String; /// - /// Overridden for better debugging experience + /// The custom is a + /// that serializes the expression to JSON (array for function or literal value). /// public override string ToString() { @@ -131,7 +147,7 @@ public bool Equals(Expression other) // return false; // // // For function expressions, compare arguments - // if (IsFunctionExpression) + // if (!IsLiteralValue) // { // if (other.Args == null || Args.Count != other.Args.Count) // return false; @@ -153,7 +169,7 @@ public override bool Equals(object? obj) public override int GetHashCode() { throw new NotImplementedException(); - // if (IsFunctionExpression) + // if (!IsLiteralValue) // { // var hash = Function.GetHashCode(); // if (Args != null) diff --git a/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs b/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs index 97539fcd4d..3095ad950b 100644 --- a/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs +++ b/src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs @@ -69,16 +69,21 @@ private static Expression ReadArray(JsonElement element) { throw new JsonException("Function name in expression must be string"); } + var args = new List(); + var functionString = enumerator.Current.GetString(); - var functionEnum = ExpressionFunction(enumerator.Current.GetString()); + var functionEnum = ExpressionFunction(functionString); + if (functionEnum == Expressions.ExpressionFunction.INVALID) + { + args.Add(new Expression(functionString)); + } - var args = new List(); while (enumerator.MoveNext()) { args.Add(ReadStatic(enumerator.Current)); } - return new Expression(functionEnum, args); + return new Expression(functionEnum, args.ToArray()); } private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOptions options) @@ -93,24 +98,29 @@ private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOpt { throw new JsonException("Function name in expression should be string"); } + var args = new List(); - var functionEnum = ExpressionFunction(reader.GetString()); + var functionString = reader.GetString(); + var functionEnum = ExpressionFunction(functionString); - var args = new List(); + if (functionEnum == Expressions.ExpressionFunction.INVALID) + { + args.Add(new Expression(functionString)); + } while (reader.Read() && reader.TokenType != JsonTokenType.EndArray) { args.Add(ReadStatic(ref reader, options)); } - return new Expression(functionEnum, args); + return new Expression(functionEnum, args.ToArray()); } private static ExpressionFunction ExpressionFunction(string? stringFunction) { if (!Enum.TryParse(stringFunction, ignoreCase: false, out var functionEnum)) { - throw new JsonException($"Function \"{stringFunction}\" not implemented"); + return Expressions.ExpressionFunction.INVALID; } return functionEnum; @@ -119,21 +129,24 @@ private static ExpressionFunction ExpressionFunction(string? stringFunction) /// public override void Write(Utf8JsonWriter writer, Expression value, JsonSerializerOptions options) { - if (value.IsFunctionExpression) + if (value.IsLiteralValue) + { + // Just serialize the literal value + JsonSerializer.Serialize(writer, value.ValueUnion.ToObject(), options); + } + else { // Serialize with as an array expression ["functionName", arg1, arg2, ...] writer.WriteStartArray(); - writer.WriteStringValue(value.Function.ToString()); + if (value.Function != Expressions.ExpressionFunction.INVALID) + { + writer.WriteStringValue(value.Function.ToString()); + } foreach (var arg in value.Args) { JsonSerializer.Serialize(writer, arg, options); } writer.WriteEndArray(); } - else - { - // Just serialize the literal value - JsonSerializer.Serialize(writer, value.ValueUnion.ToObject(), options); - } } } diff --git a/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs b/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs index c4082ab03a..f799c95a9f 100644 --- a/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs +++ b/src/Altinn.App.Core/Models/Expressions/ExpressionFunction.cs @@ -12,6 +12,13 @@ namespace Altinn.App.Core.Models.Expressions; [JsonConverter(typeof(JsonStringEnumConverter))] public enum ExpressionFunction { + /// + /// Expressions that hold a literal value have this as function + /// +#pragma warning disable CA1707 // Identifiers should not contain underscores + LITERAL_VALUE = -1, +#pragma warning restore CA1707 // Identifiers should not contain underscores + /// /// Value for all unknown functions. /// diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs index 9c75e5e6ad..c935f44554 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestInvalid.cs @@ -55,7 +55,7 @@ public async Task Simple_Theory(string testName, string folder) await ExpressionEvaluator.EvaluateExpression(state, test.Expression, await test.GetContextOrNull(state)); }; - (await act.Should().ThrowAsync()).WithMessage(testCase.ExpectsFailure); + (await act.Should().ThrowAsync()).WithMessage(testCase.ExpectsFailure + "*"); } private static async Task LoadData(string testName, string folder) diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs index cf2c05f622..9ac8a44545 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/ExpressionEvaluatorTests/ExpressionTests.cs @@ -61,14 +61,14 @@ public void TestShortcuts() { var trueExpression = Expression.True; Assert.True(trueExpression.ValueUnion.Bool); - Assert.False(trueExpression.IsFunctionExpression); + Assert.True(trueExpression.IsLiteralValue); var falseExpression = Expression.False; Assert.False(falseExpression.ValueUnion.Bool); - Assert.False(falseExpression.IsFunctionExpression); + Assert.True(falseExpression.IsLiteralValue); var nullExpression = Expression.Null; Assert.Equal(JsonValueKind.Null, nullExpression.ValueUnion.ValueKind); - Assert.False(nullExpression.IsFunctionExpression); + Assert.True(nullExpression.IsLiteralValue); } } diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index bec0cac792..ecec455594 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -4428,22 +4428,20 @@ namespace Altinn.App.Core.Models.Expressions [System.Obsolete("Use the constructor with ExpressionValue instead")] public Expression(object? value) { } public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression arg1) { } - public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, System.Collections.Generic.List? args) { } + public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression[] args) { } + [System.Obsolete("Use the constructor with Expression[] instead")] + public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, System.Collections.Generic.List args) { } public Expression(Altinn.App.Core.Models.Expressions.ExpressionFunction function, Altinn.App.Core.Models.Expressions.Expression arg1, Altinn.App.Core.Models.Expressions.Expression arg2) { } - public System.Collections.Generic.List? Args { get; } + public Altinn.App.Core.Models.Expressions.Expression[]? Args { get; } public Altinn.App.Core.Models.Expressions.ExpressionFunction Function { get; } - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Value")] - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "ValueUnion")] - [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, new string?[]?[] { - "Function", - "Args"})] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Value")] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "ValueUnion")] - [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, new string?[]?[] { - "Function", - "Args"})] + [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] + [System.Obsolete("Use !IsLiteralValue instead")] + [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(true, "Args")] public bool IsFunctionExpression { get; } public bool IsLiteralString { get; } + [System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Args")] + [get: System.Diagnostics.CodeAnalysis.MemberNotNullWhen(false, "Args")] + public bool IsLiteralValue { get; } [System.Obsolete("Use ValueUnion instead")] public object? Value { get; } public Altinn.App.Core.Internal.Expressions.ExpressionValue ValueUnion { get; } @@ -4468,6 +4466,7 @@ namespace Altinn.App.Core.Models.Expressions [System.Text.Json.Serialization.JsonConverter(typeof(System.Text.Json.Serialization.JsonStringEnumConverter))] public enum ExpressionFunction { + LITERAL_VALUE = -1, INVALID = 0, dataModel = 1, component = 2,