Skip to content

Commit

Permalink
add tests and refactor jsondatamodel
Browse files Browse the repository at this point in the history
  • Loading branch information
bjosttveit committed Sep 21, 2023
1 parent ed81fb6 commit ee4527a
Show file tree
Hide file tree
Showing 20 changed files with 1,048 additions and 57 deletions.
11 changes: 6 additions & 5 deletions src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs
Original file line number Diff line number Diff line change
Expand Up @@ -227,17 +227,21 @@ public async Task<List<ValidationIssue>> ValidateDataElement(Instance instance,
int instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId);
object data = await _dataClient.GetFormData(
instanceGuid, modelType, instance.Org, app, instanceOwnerPartyId, Guid.Parse(dataElement.Id));
var layoutSet = _appResourcesService.GetLayoutSetForTask(dataType.TaskId);
var evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id);

if (_appSettings.RemoveHiddenDataPreview)
{
var layoutSet = _appResourcesService.GetLayoutSetForTask(dataType.TaskId);
var evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id);
// Remove hidden data before validation
LayoutEvaluator.RemoveHiddenData(evaluationState);
// Evaluate expressions in layout and validate that all required data is included and that maxLength
// is respected on groups
var layoutErrors = LayoutEvaluator.RunLayoutValidationsForRequired(evaluationState, dataElement.Id);
messages.AddRange(layoutErrors);

// Run expression validations
var expressionErrors = ExpressionValidator.Validate(dataType.Id, _appResourcesService, new DataModel(data), evaluationState, _logger);
messages.AddRange(expressionErrors);
}

// run Standard mvc validation using the System.ComponentModel.DataAnnotations
Expand All @@ -259,9 +263,6 @@ public async Task<List<ValidationIssue>> ValidateDataElement(Instance instance,
messages.AddRange(MapModelStateToIssueList(actionContext.ModelState, instance, dataElement.Id, data.GetType()));
}

// Run expression validations
var expressionErrors = ExpressionValidator.Validate(dataType.Id, _appResourcesService, new DataModel(data), evaluationState, _logger);
messages.AddRange(expressionErrors);
}

return messages;
Expand Down
12 changes: 11 additions & 1 deletion src/Altinn.App.Core/Helpers/DataModel/DataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,11 @@ public DataModel(object serviceModel)
/// <inheritdoc />
public string[] GetResolvedKeys(string key)
{
if (_serviceModel is null)
{
return new string[0];
}

var keyParts = key.Split('.');
return GetResolvedKeysRecursive(keyParts, _serviceModel);
}
Expand All @@ -113,13 +118,18 @@ internal static string JoinFieldKeyParts(string? currentKey, string? key)

private string[] GetResolvedKeysRecursive(string[] keyParts, object currentModel, int currentIndex = 0, string currentKey = "")
{
if (currentModel is null)
{
return new string[0];
}

if (currentIndex == keyParts.Length)
{
return new[] { currentKey };
}

var (key, groupIndex) = ParseKeyPart(keyParts[currentIndex]);
var prop = currentModel.GetType().GetProperties().FirstOrDefault(p => IsPropertyWithJsonName(p, key));
var prop = currentModel?.GetType().GetProperties().FirstOrDefault(p => IsPropertyWithJsonName(p, key));
var childModel = prop?.GetValue(currentModel);
if (childModel is null)
{
Expand Down
2 changes: 0 additions & 2 deletions src/Altinn.App.Core/Internal/App/FrontendFeatures.cs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ public FrontendFeatures(IFeatureManager featureManager)
{
features.Add("jsonObjectInDataResponse", false);
}

features.Add("expressionValidation", true);
}

/// <inheritdoc />
Expand Down
21 changes: 16 additions & 5 deletions src/Altinn.App.Core/Internal/Expressions/ExpressionEvaluator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public static bool EvaluateBooleanExpression(LayoutEvaluatorState state, Compone
var args = expr.Args.Select(a => EvaluateExpression(state, a, context, positionalArguments)).ToArray();
var ret = expr.Function switch
{
ExpressionFunction.dataModel => state.GetModelData(args.First()?.ToString(), context),
ExpressionFunction.dataModel => DataModel(args.First()?.ToString(), context, state),
ExpressionFunction.component => Component(args, context, state),
ExpressionFunction.instanceContext => state.GetInstanceContext(args.First()?.ToString()!),
ExpressionFunction.@if => IfImpl(args),
Expand Down Expand Up @@ -92,6 +92,19 @@ public static bool EvaluateBooleanExpression(LayoutEvaluatorState state, Compone
return ret;
}

private static object? DataModel(string? key, ComponentContext? context, LayoutEvaluatorState state)
{
var data = state.GetModelData(key, context);

// Only allow IConvertible types to be returned from data model
// Objects and arrays should return null
return data switch
{
IConvertible c => c,
_ => null,
};
}

private static object? Component(object?[] args, ComponentContext? context, LayoutEvaluatorState state)
{
var componentId = args.First()?.ToString();
Expand Down Expand Up @@ -127,7 +140,7 @@ public static bool EvaluateBooleanExpression(LayoutEvaluatorState state, Compone
parent = parent.Parent;
}

return state.GetModelData(binding, context);
return DataModel(binding, context, state);
}

private static string? Concat(object?[] args)
Expand Down Expand Up @@ -338,9 +351,7 @@ private static (double?, double?) PrepareNumericArgs(object?[] args)
{
bool ab => throw new ExpressionEvaluatorTypeErrorException($"Expected number, got value {(ab ? "true" : "false")}"),
string s => parseNumber(s),
int i => Convert.ToDouble(i),
decimal d => Convert.ToDouble(d),
object o => o as double?, // assume all relevant numbers are representable as double (as in frontend)
IConvertible c => Convert.ToDouble(c),
_ => null
};
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System.Reflection;
using System.Text.Json;
using System.Text.Json.Nodes;
using System.Text.Json.Serialization;
using Altinn.App.Core.Features.Validation;
using Altinn.App.Core.Internal.Expressions;
Expand All @@ -24,6 +25,8 @@ public void RunExpressionValidationTest(ExpressionValidationTestModel testCase)
var logger = Mock.Of<ILogger<ValidationAppSI>>();
var dataModel = new JsonDataModel(testCase.FormData);
var evaluatorState = new LayoutEvaluatorState(dataModel, testCase.Layouts, new(), new());

LayoutEvaluator.RemoveHiddenData(evaluatorState);
var validationIssues = ExpressionValidator.Validate(testCase.ValidationConfig, dataModel, evaluatorState, logger).ToArray();

var result = validationIssues.Select(i => new
Expand Down Expand Up @@ -73,7 +76,7 @@ public class ExpressionValidationTestModel

public JsonElement ValidationConfig { get; set; }

public JsonElement FormData { get; set; }
public JsonObject FormData { get; set; }

[JsonConverter(typeof(LayoutModelConverterFromObject))]
public LayoutModel Layouts { get; set; }
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"name": "Should not return an error if component is hidden",
"expects": [],
"validationConfig": {
"$schema": "https://altinncdn.no/schemas/json/validation/validation.schema.v1.json",
"validations": {
"form.name": ["none-is-not-allowed"]
},
"definitions": {
"none-is-not-allowed": {
"message": "none is not allowed",
"severity": "errors",
"condition": ["equals", ["dataModel", ["argv", 0]], "none"]
}
}
},
"formData": {
"form": {
"name": "none"
}
},
"layouts": {
"Page": {
"$schema": "https://altinncdn.no/schemas/json/layout/layout.schema.v1.json",
"data": {
"layout": [
{
"id": "name-input",
"type": "Input",
"dataModelBindings": {
"simpleBinding": "form.name"
},
"hidden": true
}
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
{
"name": "Should not return an error if page is hidden",
"expects": [],
"validationConfig": {
"$schema": "https://altinncdn.no/schemas/json/validation/validation.schema.v1.json",
"validations": {
"form.name": ["none-is-not-allowed"]
},
"definitions": {
"none-is-not-allowed": {
"message": "none is not allowed",
"severity": "errors",
"condition": ["equals", ["dataModel", ["argv", 0]], "none"]
}
}
},
"formData": {
"form": {
"name": "none"
}
},
"layouts": {
"Page": {
"$schema": "https://altinncdn.no/schemas/json/layout/layout.schema.v1.json",
"data": {
"hidden": true,
"layout": [
{
"id": "name-input",
"type": "Input",
"dataModelBindings": {
"simpleBinding": "form.name"
}
}
]
}
}
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
{
"name": "Should return all of the correct errors",
"expects": [
{
"message": "none is not allowed",
"severity": "errors",
"field": "form.name",
"componentId": "name-input"
},
{
"message": "string is too short",
"severity": "errors",
"field": "form.name",
"componentId": "name-input"
},
{
"message": "email must be real",
"severity": "errors",
"field": "form.email",
"componentId": "email-input"
},
{
"message": "string is too short",
"severity": "errors",
"field": "form.email",
"componentId": "email-input"
}
],
"validationConfig": {
"$schema": "https://altinncdn.no/schemas/json/validation/validation.schema.v1.json",
"validations": {
"form.name": [
"none-is-not-allowed",
"str-len",
{
"message": "this should not be shown",
"severity": "warnings",
"condition": ["startsWith", ["dataModel", ["argv", 0]], "a"]
}
],
"form.email": [
"none-is-not-allowed",
{
"message": "email must be real",
"severity": "errors",
"condition": ["contains", ["dataModel", ["argv", 0]], "fake"]
},
{
"ref": "str-len",
"condition": ["lessThan", ["stringLength", ["dataModel", ["argv", 0]]], 20]
},
{
"message": "email must contain @",
"severity": "errors",
"condition": ["notContains", ["dataModel", ["argv", 0]], "@"]
}
]
},
"definitions": {
"none-is-not-allowed": {
"message": "none is not allowed",
"severity": "errors",
"condition": ["equals", ["dataModel", ["argv", 0]], "none"]
},
"str-len": {
"message": "string is too short",
"severity": "errors",
"condition": ["lessThan", ["stringLength", ["dataModel", ["argv", 0]]], 5]
}
}
},
"formData": {
"form": {
"name": "none",
"email": "[email protected]"
}
},
"layouts": {
"Page": {
"$schema": "https://altinncdn.no/schemas/json/layout/layout.schema.v1.json",
"data": {
"layout": [
{
"id": "name-input",
"type": "Input",
"dataModelBindings": {
"simpleBinding": "form.name"
}
},
{
"id": "email-input",
"type": "Input",
"dataModelBindings": {
"simpleBinding": "form.email"
}
}
]
}
}
}
}
Loading

0 comments on commit ee4527a

Please sign in to comment.