Skip to content

Commit

Permalink
WIP: Add alias ["value", 0] in addition to ["argv", 0]
Browse files Browse the repository at this point in the history
* Move values to a separate interface added to context `IContextValueAccessor`
* Provide `["value"]` as a shortcut for `["dataModel", ["argv", 0]]`
  • Loading branch information
ivarne committed Nov 27, 2024
1 parent 099c4a0 commit 8a163d7
Show file tree
Hide file tree
Showing 10 changed files with 161 additions and 52 deletions.
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Text;
using System.Text.Json;
using Altinn.App.Core.Helpers.DataModel;
using Altinn.App.Core.Internal.App;
Expand Down Expand Up @@ -136,23 +137,21 @@ internal async Task<List<ValidationIssue>> ValidateFormData(
{
continue;
}
var valueAccessor = new ExpressionContextValueAccessor(
resolvedField,
await evaluatorState.GetModelData(resolvedField)
);
var context = new ComponentContext(
component: null,
rowIndices: DataModel.GetRowIndices(resolvedField.Field),
rowLength: null,
dataElementIdentifier: resolvedField.DataElementIdentifier
dataElementIdentifier: resolvedField.DataElementIdentifier,
contextValueAccessor: valueAccessor
);
var positionalArguments = new object[] { resolvedField };

foreach (var validation in validations)
{
await RunValidation(
evaluatorState,
validationIssues,
resolvedField,
context,
positionalArguments,
validation
);
await RunValidation(evaluatorState, validationIssues, resolvedField, context, validation);
}
}
}
Expand All @@ -165,7 +164,6 @@ private async Task RunValidation(
List<ValidationIssue> validationIssues,
DataReference resolvedField,
ComponentContext context,
object[] positionalArguments,
ExpressionValidation validation
)
{
Expand All @@ -179,8 +177,7 @@ ExpressionValidation validation
var validationResult = await ExpressionEvaluator.EvaluateExpression(
evaluatorState,
validation.Condition.Value,
context,
positionalArguments
context
);
switch (validationResult)
{
Expand Down Expand Up @@ -433,4 +430,36 @@ out JsonElement validationsObject
}
return expressionValidations;
}

private class ExpressionContextValueAccessor(DataReference reference, object? value) : IContextValueAccessor
{
public object? GetValue(ExpressionFunction function, ReadOnlySpan<object?> args)
{
if (function != ExpressionFunction.value)
{
throw new ExpressionEvaluatorTypeErrorException(
$"Invalid function for value/argv expression ({function})"
);
}
switch (args)
{
case []:
return value;
case [0.0]:
case ["0"]:
case ["reference"]:
return reference;
default:
var sb = new StringBuilder();
foreach (var arg in args)
{
sb.Append(", ");
sb.Append(arg);
}
throw new ExpressionEvaluatorTypeErrorException(
$"Invalid arguments for value/argv expression ([\"value\"{sb}])"
);
}
}
}
}
10 changes: 10 additions & 0 deletions src/Altinn.App.Core/Helpers/DataModel/DataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ DataElementIdentifier defaultDataElementIdentifier
return modelWrapper.GetModelData(key.Field, rowIndexes);
}

/// <summary>
/// Get model data based on an explicit data reference
/// </summary>
public async Task<object?> GetModelData(DataReference dataReference)
{
var model = await _dataAccessor.GetFormData(dataReference.DataElementIdentifier);
var modelWrapper = new DataModelWrapper(model);
return modelWrapper.GetModelData(dataReference.Field);
}

/// <summary>
/// Get the count of data elements set in a group (enumerable)
/// </summary>
Expand Down
47 changes: 19 additions & 28 deletions src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,7 @@ bool defaultReturn
public static async Task<object?> EvaluateExpression(
LayoutEvaluatorState state,
Expression expr,
ComponentContext context,
object[]? positionalArguments = null
ComponentContext context
)
{
if (!expr.IsFunctionExpression)
Expand All @@ -69,7 +68,7 @@ bool defaultReturn
var args = new object?[expr.Args.Count];
for (var i = 0; i < args.Length; i++)
{
args[i] = await EvaluateExpression(state, expr.Args[i], context, positionalArguments);
args[i] = await EvaluateExpression(state, expr.Args[i], context);
}
// ! TODO: should find better ways to deal with nulls here for the next major version
var ret = expr.Function switch
Expand Down Expand Up @@ -98,7 +97,7 @@ bool defaultReturn
ExpressionFunction.round => Round(args),
ExpressionFunction.upperCase => UpperCase(args),
ExpressionFunction.lowerCase => LowerCase(args),
ExpressionFunction.argv => Argv(args, positionalArguments),
ExpressionFunction.value => ContextValueLookup(args, context),
ExpressionFunction.gatewayAction => state.GetGatewayAction(),
ExpressionFunction.language => state.GetLanguage() ?? "nb",
_ => throw new ExpressionEvaluatorTypeErrorException($"Function \"{expr.Function}\" not implemented"),
Expand All @@ -110,12 +109,13 @@ bool defaultReturn
{
if (args is [DataReference dataReference])
{
return await DataModel(
new ModelBinding() { Field = dataReference.Field },
dataReference.DataElementIdentifier,
context.RowIndices,
state
);
// Only allow IConvertible types to be returned from data model
// Objects and arrays should return null
return await state.GetModelData(dataReference) switch
{
IConvertible c => c,
_ => null,
};
}
var key = args switch
{
Expand Down Expand Up @@ -192,7 +192,7 @@ LayoutEvaluatorState state
args.Select(a =>
a switch
{
string s => s,
string s => s, // concat must not convert "NULL" to null
_ => ToStringForEquals(a),
}
)
Expand Down Expand Up @@ -534,28 +534,19 @@ private static (double?, double?) PrepareNumericArgs(object?[] args)
return string.Equals(ToStringForEquals(args[0]), ToStringForEquals(args[1]), StringComparison.Ordinal);
}

private static object Argv(object?[] args, object[]? positionalArguments)
private static object? ContextValueLookup(ReadOnlySpan<object?> args, ComponentContext? context)
{
if (args.Length != 1)
{
throw new ExpressionEvaluatorTypeErrorException($"Expected 1 argument(s), got {args.Length}");
}

var index = (int?)PrepareNumericArg(args[0]);
if (!index.HasValue)
{
throw new ExpressionEvaluatorTypeErrorException($"Expected number, got value \"{args[0]}\"");
}

if (positionalArguments == null)
IContextValueAccessor? contextValueAccessor = null;
while (context != null && contextValueAccessor == null)
{
throw new ExpressionEvaluatorTypeErrorException("No positional arguments available");
contextValueAccessor = context.ContextValueAccessor;
context = context.Parent;
}
if (index < 0 || index >= positionalArguments.Length)
if (contextValueAccessor == null)
{
throw new ExpressionEvaluatorTypeErrorException($"Index {index} out of range");
throw new ExpressionEvaluatorTypeErrorException("Tried to evaluate ");
}

return positionalArguments[index.Value];
return contextValueAccessor.GetValue(ExpressionFunction.value, args);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,14 @@ private static bool CompareRowIndexes(int[]? targetRowIndexes, int[]? sourceRowI
return true;
}

/// <summary>
/// Get model data from an explicit data reference
/// </summary>
public async Task<object?> GetModelData(DataReference dataReference)
{
return await _dataModel.GetModelData(dataReference);
}

/// <summary>
/// Get field from dataModel with key and context
/// </summary>
Expand Down
9 changes: 8 additions & 1 deletion src/Altinn.App.Core/Models/Expressions/ComponentContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,12 @@ public ComponentContext(
int[]? rowIndices,
int? rowLength,
DataElementIdentifier dataElementIdentifier,
IEnumerable<ComponentContext>? childContexts = null
IEnumerable<ComponentContext>? childContexts = null,
IContextValueAccessor? contextValueAccessor = null
)
{
DataElementIdentifier = dataElementIdentifier;
ContextValueAccessor = contextValueAccessor;
Component = component;
RowIndices = rowIndices;
_rowLength = rowLength;
Expand Down Expand Up @@ -123,6 +125,11 @@ public async Task<BitArray> GetHiddenRows(LayoutEvaluatorState state)
/// </summary>
public DataElementIdentifier DataElementIdentifier { get; }

/// <summary>
/// Accessor for the ["value"] expression (used in expression validation and when expressions are used in filters)
/// </summary>
public IContextValueAccessor? ContextValueAccessor { get; }

/// <summary>
/// Get all children and children of children of this componentContext (not including this)
/// </summary>
Expand Down
48 changes: 43 additions & 5 deletions src/Altinn.App.Core/Models/Expressions/ExpressionConverter.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using System.Buffers;
using System.Text.Json;
using System.Text.Json.Serialization;

Expand Down Expand Up @@ -48,11 +49,7 @@ private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOpt
throw new JsonException("Function name in expression should be string");
}

var stringFunction = reader.GetString();
if (!Enum.TryParse<ExpressionFunction>(stringFunction, ignoreCase: false, out var functionEnum))
{
throw new JsonException($"Function \"{stringFunction}\" not implemented");
}
var functionEnum = ParseFunctionName(ref reader);

var args = new List<Expression>();

Expand All @@ -64,6 +61,47 @@ private static Expression ReadArray(ref Utf8JsonReader reader, JsonSerializerOpt
return new Expression(functionEnum, args);
}

private static ExpressionFunction ParseFunctionName(ref Utf8JsonReader reader)
{
var functionSpan = reader.HasValueSequence ? reader.ValueSequence.ToArray() : reader.ValueSpan;
return functionSpan switch
{
[100, 97, 116, 97, 77, 111, 100, 101, 108] => ExpressionFunction.dataModel,
[99, 111, 109, 112, 111, 110, 101, 110, 116] => ExpressionFunction.component,
[105, 110, 115, 116, 97, 110, 99, 101, 67, 111, 110, 116, 101, 120, 116] =>
ExpressionFunction.instanceContext,
[105, 102] => ExpressionFunction.@if,
[102, 114, 111, 110, 116, 101, 110, 100, 83, 101, 116, 116, 105, 110, 103, 115] =>
ExpressionFunction.frontendSettings,
[99, 111, 110, 99, 97, 116] => ExpressionFunction.concat,
[117, 112, 112, 101, 114, 67, 97, 115, 101] => ExpressionFunction.upperCase,
[108, 111, 119, 101, 114, 67, 97, 115, 101] => ExpressionFunction.lowerCase,
[99, 111, 110, 116, 97, 105, 110, 115] => ExpressionFunction.contains,
[110, 111, 116, 67, 111, 110, 116, 97, 105, 110, 115] => ExpressionFunction.notContains,
[99, 111, 109, 109, 97, 67, 111, 110, 116, 97, 105, 110, 115] => ExpressionFunction.commaContains,
[101, 110, 100, 115, 87, 105, 116, 104] => ExpressionFunction.endsWith,
[115, 116, 97, 114, 116, 115, 87, 105, 116, 104] => ExpressionFunction.startsWith,
[101, 113, 117, 97, 108, 115] => ExpressionFunction.equals,
[110, 111, 116, 69, 113, 117, 97, 108, 115] => ExpressionFunction.notEquals,
[103, 114, 101, 97, 116, 101, 114, 84, 104, 97, 110, 69, 113] => ExpressionFunction.greaterThanEq,
[108, 101, 115, 115, 84, 104, 97, 110] => ExpressionFunction.lessThan,
[108, 101, 115, 115, 84, 104, 97, 110, 69, 113] => ExpressionFunction.lessThanEq,
[103, 114, 101, 97, 116, 101, 114, 84, 104, 97, 110] => ExpressionFunction.greaterThan,
[115, 116, 114, 105, 110, 103, 76, 101, 110, 103, 116, 104] => ExpressionFunction.stringLength,
[114, 111, 117, 110, 100] => ExpressionFunction.round,
[97, 110, 100] => ExpressionFunction.and,
[111, 114] => ExpressionFunction.or,
[110, 111, 116] => ExpressionFunction.not,
[118, 97, 108, 117, 101] => ExpressionFunction.value,
[97, 114, 103, 118] => ExpressionFunction.value, // "argv" works for compatibility
[103, 97, 116, 101, 119, 97, 121, 65, 99, 116, 105, 111, 110] => ExpressionFunction.gatewayAction,
[108, 97, 110, 103, 117, 97, 103, 101] => ExpressionFunction.language,
_ => throw new JsonException(
$"Function \"{System.Text.Encoding.UTF8.GetString(functionSpan)}\" not implemented"
),
};
}

/// <inheritdoc />
public override void Write(Utf8JsonWriter writer, Expression value, JsonSerializerOptions options)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,9 +134,12 @@ public enum ExpressionFunction
not,

/// <summary>
/// Returns a positional argument
/// Get the value from context (eg the value from expression validation, or the option to filter)
/// </summary>
argv,
/// <remarks>
/// This vas previously named argv for expression validation and it works for compatibility reasons
/// </remarks>
value,

/// <summary>
/// Get the action performed in task prior to bpmn gateway
Expand Down
18 changes: 18 additions & 0 deletions src/Altinn.App.Core/Models/Expressions/IContextValueAccessor.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
namespace Altinn.App.Core.Models.Expressions;

/// <summary>
/// Provide an implementation for the ["value", ?] expression
/// The expression is special, because it is only valid in contexts where there exists a value
/// * Expression validation (`["value"]` is the value to validate)
/// * Filtering (["value"] is the value that is should be compared)
/// </summary>
public interface IContextValueAccessor
{
/// <summary>
/// Get the value from the context
/// </summary>
/// <param name="function">The function that was called</param>
/// <param name="args">List of arguments for the expression</param>
/// <returns>The value</returns>
object? GetValue(ExpressionFunction function, ReadOnlySpan<object?> args);
}
Original file line number Diff line number Diff line change
Expand Up @@ -43,20 +43,20 @@
{
"message": "email must be real",
"severity": "error",
"condition": ["contains", ["dataModel", ["argv", 0]], "fake"]
"condition": ["contains", ["dataModel", ["argv", "reference"]], "fake"]
},
{
"ref": "str-len",
"condition": [
"lessThan",
["stringLength", ["dataModel", ["argv", 0]]],
["stringLength", ["value"]],
20
]
},
{
"message": "email must contain @",
"severity": "error",
"condition": ["notContains", ["dataModel", ["argv", 0]], "@"]
"condition": ["notContains", ["dataModel", ["value", 0]], "@"]
}
]
},
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
"name": "Special strings must not be lowercased",
"expression": ["concat", "TRUE", "FALSE", "true", "false", "Null", "null", "NULL", "undefined", "UNDEFINED"],
"expects": "TRUEFALSEtruefalseNullnullNULLundefinedUNDEFINED"
}

0 comments on commit 8a163d7

Please sign in to comment.