Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Add alias ["value", 0] in addition to ["argv", 0] #929

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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"
}
Loading