From de315c74c6b9e7d69f49e83f003f7507b49823f5 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 7 Dec 2023 21:49:36 +0100 Subject: [PATCH 1/7] Start work on new validation Improve GetJsonPath(Expression) and fix some sonarcloud issues Improve test coverage Full validation rewrite complete and existing tests work Probably still some issues and test coverage was spoty so more tests needs to be written. Make IInstanceValidator obsolete Add DataType as input to validators Fix a few issues to prepare for tests of expressions Add more tests --- .../Controllers/ProcessController.cs | 9 +- .../Controllers/ValidateController.cs | 11 +- .../Extensions/ServiceCollectionExtensions.cs | 3 +- .../FileAnalyzis/FileAnalysisResult.cs | 2 +- .../Features/IFormDataValidator.cs | 49 ++ .../Features/IInstanceValidator.cs | 2 + .../Features/ITaskValidator.cs | 39 ++ .../Default/DataAnnotationValidator.cs | 71 +++ .../Default/DefaultDataElementValidation.cs | 96 ++++ .../Default/DefaultTaskValidator.cs | 63 +++ .../Validation/Default/ExpressionValidator.cs | 301 ++++++++++++ .../LegacyIValidationFormDataValidator.cs | 52 +++ .../Default/LegacyIValidationTaskValidator.cs | 47 ++ .../Validation/Default/RequiredValidator.cs | 40 ++ .../Validation/ExpressionValidator.cs | 276 ----------- .../Validation/GenericFormDataValidator.cs | 108 +++++ .../Validation/Helpers/ModelStateHelpers.cs | 156 +++++++ .../Validation/IDataElementValidator.cs | 38 ++ .../Features/Validation/IValidation.cs | 28 -- .../Features/Validation/IValidationService.cs | 53 +++ .../Validation/NullInstanceValidator.cs | 23 - .../Features/Validation/ValidationAppSI.cs | 433 ------------------ .../Features/Validation/ValidationService.cs | 161 +++++++ .../Helpers/LinqExpressionHelpers.cs | 83 ++++ .../Internal/Expressions/LayoutEvaluator.cs | 2 +- .../Expressions/LayoutEvaluatorState.cs | 8 + .../LayoutEvaluatorStateInitializer.cs | 2 +- .../Models/Validation/ValidationIssue.cs | 12 +- .../Controllers/ValidateControllerTests.cs | 19 +- .../ValidateControllerValidateDataTests.cs | 11 +- .../Default/DataAnnotationValidatorTests.cs | 217 +++++++++ .../Default/ExpressionValidatorTests.cs | 124 +++++ .../Default/LegacyIValidationFormDataTests.cs | 141 ++++++ .../Validators/ExpressionValidationTests.cs | 5 +- .../Validators/GenericValidatorTests.cs | 83 ++++ ...ITests.cs => ValidationServiceOldTests.cs} | 139 +++--- .../Validators/ValidationServiceTests.cs | 115 +++++ .../Helpers/LinqExpressionHelpersTests.cs | 68 +++ .../NullInstanceValidatorTests.cs | 38 -- 39 files changed, 2238 insertions(+), 890 deletions(-) create mode 100644 src/Altinn.App.Core/Features/IFormDataValidator.cs create mode 100644 src/Altinn.App.Core/Features/ITaskValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs delete mode 100644 src/Altinn.App.Core/Features/Validation/ExpressionValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs create mode 100644 src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs create mode 100644 src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs delete mode 100644 src/Altinn.App.Core/Features/Validation/IValidation.cs create mode 100644 src/Altinn.App.Core/Features/Validation/IValidationService.cs delete mode 100644 src/Altinn.App.Core/Features/Validation/NullInstanceValidator.cs delete mode 100644 src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs create mode 100644 src/Altinn.App.Core/Features/Validation/ValidationService.cs create mode 100644 src/Altinn.App.Core/Helpers/LinqExpressionHelpers.cs create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs rename test/Altinn.App.Core.Tests/Features/Validators/{ValidationAppSITests.cs => ValidationServiceOldTests.cs} (51%) create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs create mode 100644 test/Altinn.App.Core.Tests/Helpers/LinqExpressionHelpersTests.cs delete mode 100644 test/Altinn.App.Core.Tests/Implementation/NullInstanceValidatorTests.cs diff --git a/src/Altinn.App.Api/Controllers/ProcessController.cs b/src/Altinn.App.Api/Controllers/ProcessController.cs index d207a2560..1871c3326 100644 --- a/src/Altinn.App.Api/Controllers/ProcessController.cs +++ b/src/Altinn.App.Api/Controllers/ProcessController.cs @@ -2,6 +2,7 @@ using System.Net; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Models; +using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation; using Altinn.App.Core.Helpers; using Altinn.App.Core.Internal.Instances; @@ -34,7 +35,7 @@ public class ProcessController : ControllerBase private readonly ILogger _logger; private readonly IInstanceClient _instanceClient; private readonly IProcessClient _processClient; - private readonly IValidation _validationService; + private readonly IValidationService _validationService; private readonly IAuthorizationService _authorization; private readonly IProcessEngine _processEngine; private readonly IProcessReader _processReader; @@ -46,7 +47,7 @@ public ProcessController( ILogger logger, IInstanceClient instanceClient, IProcessClient processClient, - IValidation validationService, + IValidationService validationService, IAuthorizationService authorization, IProcessReader processReader, IProcessEngine processEngine) @@ -202,7 +203,7 @@ public async Task>> GetNextElements( } } - private async Task CanTaskBeEnded(Instance instance, string currentElementId) + private async Task CanTaskBeEnded(Instance instance, string currentTaskId) { List validationIssues = new List(); @@ -210,7 +211,7 @@ private async Task CanTaskBeEnded(Instance instance, string currentElement if (instance.Process?.CurrentTask?.Validated == null || !instance.Process.CurrentTask.Validated.CanCompleteTask) { - validationIssues = await _validationService.ValidateAndUpdateProcess(instance, currentElementId); + validationIssues = await _validationService.ValidateInstanceAtTask(instance, currentTaskId); canEndTask = await ProcessHelper.CanEndProcessTask(instance, validationIssues); } diff --git a/src/Altinn.App.Api/Controllers/ValidateController.cs b/src/Altinn.App.Api/Controllers/ValidateController.cs index 3c376af73..4da14bbde 100644 --- a/src/Altinn.App.Api/Controllers/ValidateController.cs +++ b/src/Altinn.App.Api/Controllers/ValidateController.cs @@ -1,5 +1,6 @@ #nullable enable +using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation; using Altinn.App.Core.Helpers; using Altinn.App.Core.Infrastructure.Clients; @@ -21,14 +22,14 @@ public class ValidateController : ControllerBase { private readonly IInstanceClient _instanceClient; private readonly IAppMetadata _appMetadata; - private readonly IValidation _validationService; + private readonly IValidationService _validationService; /// /// Initialises a new instance of the class /// public ValidateController( IInstanceClient instanceClient, - IValidation validationService, + IValidationService validationService, IAppMetadata appMetadata) { _instanceClient = instanceClient; @@ -66,7 +67,7 @@ public async Task ValidateInstance( try { - List messages = await _validationService.ValidateAndUpdateProcess(instance, taskId); + List messages = await _validationService.ValidateInstanceAtTask(instance, taskId); return Ok(messages); } catch (PlatformHttpException exception) @@ -130,9 +131,11 @@ public async Task ValidateData( throw new ValidationException("Unknown element type."); } - messages.AddRange(await _validationService.ValidateDataElement(instance, dataType, element)); + messages.AddRange(await _validationService.ValidateDataElement(instance, element, dataType)); string taskId = instance.Process.CurrentTask.ElementId; + + // Should this be a BadRequest instead? if (!dataType.TaskId.Equals(taskId, StringComparison.OrdinalIgnoreCase)) { ValidationIssue message = new ValidationIssue diff --git a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs index 7b91c4794..addd6d21b 100644 --- a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs @@ -133,7 +133,7 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati { // Services for Altinn App services.TryAddTransient(); - services.TryAddTransient(); + services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddSingleton(); @@ -146,7 +146,6 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati #pragma warning restore CS0618, CS0612 // Type or member is obsolete services.TryAddTransient(); services.TryAddTransient(); - services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); diff --git a/src/Altinn.App.Core/Features/FileAnalyzis/FileAnalysisResult.cs b/src/Altinn.App.Core/Features/FileAnalyzis/FileAnalysisResult.cs index 6bbf62d56..279cd97d3 100644 --- a/src/Altinn.App.Core/Features/FileAnalyzis/FileAnalysisResult.cs +++ b/src/Altinn.App.Core/Features/FileAnalyzis/FileAnalysisResult.cs @@ -37,7 +37,7 @@ public FileAnalysisResult(string analyserId) public string? MimeType { get; set; } /// - /// Key/Value pairs contaning fining from the analysis. + /// Key/Value pairs containing findings from the analysis. /// public IDictionary Metadata { get; private set; } = new Dictionary(); } diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs new file mode 100644 index 000000000..0d7d97e1e --- /dev/null +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -0,0 +1,49 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace Altinn.App.Core.Features; + +/// +/// Interface for handling validation of form data. +/// (i.e. dataElements with AppLogic defined +/// +public interface IFormDataValidator +{ + /// + /// The data type this validator is for. Typically either hard coded by implementation or + /// or set by constructor using a and a keyed service. + /// + string DataType { get; } + + /// + /// Extension point if the validator should run for multiple data types. + /// Typical users will just set the property. + /// + /// The ID of the data type that might be validated + bool CanValidateDataType(string dataType) => DataType == dataType; + + /// + /// Used for partial validation to ensure that the validator only runs when relevant fields have changed. + /// + /// List of the json path to all changed fields for incremental validation + bool ShouldRunForIncrementalValidation(List? changedFields = null); + + /// + /// Returns the group id of the validator. This is used to run partial validations on the backend. + /// + /// + /// The default implementation should work for most cases. + /// + public string? Code => $"{this.GetType().FullName}-{DataType}"; + + /// + /// + /// + /// + /// + /// + /// + /// List of validation issues + Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null); +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/IInstanceValidator.cs b/src/Altinn.App.Core/Features/IInstanceValidator.cs index 929e65e49..956d0aaeb 100644 --- a/src/Altinn.App.Core/Features/IInstanceValidator.cs +++ b/src/Altinn.App.Core/Features/IInstanceValidator.cs @@ -1,3 +1,4 @@ +using Altinn.App.Core.Features.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -6,6 +7,7 @@ namespace Altinn.App.Core.Features; /// /// IInstanceValidator defines the methods that are used to validate data and tasks /// +[Obsolete($"Use {nameof(ITaskValidator)}, {nameof(IDataElementValidator)} or {nameof(IFormDataValidator)} instead")] public interface IInstanceValidator { /// diff --git a/src/Altinn.App.Core/Features/ITaskValidator.cs b/src/Altinn.App.Core/Features/ITaskValidator.cs new file mode 100644 index 000000000..614b67e45 --- /dev/null +++ b/src/Altinn.App.Core/Features/ITaskValidator.cs @@ -0,0 +1,39 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace Altinn.App.Core.Features; + +/// +/// Interface for handling validation of tasks. +/// +public interface ITaskValidator +{ + /// + /// The task id this validator is for. Typically either hard coded by implementation or + /// or set by constructor using a and a keyed service. + /// + /// + /// + /// string TaskId { get; init; } + /// // constructor + /// public MyTaskValidator([ServiceKey] string taskId) + /// { + /// TaskId = taskId; + /// } + /// + /// + string TaskId { get; } + + /// + /// Unique code for the validator. Used to run partial validations on the backend. + /// + public string Code => this.GetType().FullName ?? string.Empty; + + /// + /// Actual validation logic for the task + /// + /// The instance to validate + /// List of validation issues to add to this task validation + Task> ValidateTask(Instance instance); +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs new file mode 100644 index 000000000..c3f268b47 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs @@ -0,0 +1,71 @@ +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation.Helpers; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.AspNetCore.Http; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Abstractions; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// Runs validation on the data object. +/// +public class DataAnnotationValidator : IFormDataValidator +{ + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly IObjectModelValidator _objectModelValidator; + private readonly GeneralSettings _generalSettings; + + /// + /// Constructor + /// + public DataAnnotationValidator([ServiceKey] string dataType, IHttpContextAccessor httpContextAccessor, IObjectModelValidator objectModelValidator, IOptions generalSettings) + { + DataType = dataType; + _httpContextAccessor = httpContextAccessor; + _objectModelValidator = objectModelValidator; + _generalSettings = generalSettings.Value; + } + + /// + /// Dummy implementation to satisfy interface, We use instead + /// + public string DataType { get; } + + /// + /// Run validator for all data types. + /// + public bool CanValidateDataType(string dataType) => true; + + /// + /// Disable incremental validation for this validator. + /// + public bool ShouldRunForIncrementalValidation(List? changedFields = null) => false; + + /// + public Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) + { + try + { + var modelState = new ModelStateDictionary(); + var actionContext = new ActionContext( + _httpContextAccessor.HttpContext!, + new Microsoft.AspNetCore.Routing.RouteData(), + new ActionDescriptor(), + modelState); + ValidationStateDictionary validationState = new ValidationStateDictionary(); + _objectModelValidator.Validate(actionContext, validationState, null!, data); + + return Task.FromResult(ModelStateHelpers.ModelStateToIssueList(modelState, instance, dataElement, _generalSettings, data.GetType(), ValidationIssueSources.ModelState)); + } + catch (Exception e) + { + return Task.FromException>(e); + } + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs new file mode 100644 index 000000000..1cb99af02 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs @@ -0,0 +1,96 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Enums; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// Default validations that run on all data elements to validate metadata and file scan results. +/// +public class DefaultDataElementValidation : IDataElementValidator +{ + public string DataType { get; } + + /// + /// Runs on all data elements to validate metadata and file scan results. + /// + public bool CanValidateDataType(DataType dataType) => true; + + public Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType) + { + var issues = new List(); + if (dataElement.ContentType == null) + { + issues.Add( new ValidationIssue + { + InstanceId = instance.Id, + Code = ValidationIssueCodes.DataElementCodes.MissingContentType, + DataElementId = dataElement.Id, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.DataElementCodes.MissingContentType + }); + } + else + { + var contentTypeWithoutEncoding = dataElement.ContentType.Split(";")[0]; + + if (dataType.AllowedContentTypes != null && dataType.AllowedContentTypes.Count > 0 && + dataType.AllowedContentTypes.All(ct => + !ct.Equals(contentTypeWithoutEncoding, StringComparison.OrdinalIgnoreCase))) + { + issues.Add( new ValidationIssue + { + InstanceId = instance.Id, + DataElementId = dataElement.Id, + Code = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, + Field = dataType.Id + }); + } + } + + if (dataType.MaxSize.HasValue && dataType.MaxSize > 0 && + (long)dataType.MaxSize * 1024 * 1024 < dataElement.Size) + { + issues.Add( new ValidationIssue + { + InstanceId = instance.Id, + DataElementId = dataElement.Id, + Code = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, + Field = dataType.Id + }); + } + + if (dataType.EnableFileScan && dataElement.FileScanResult == FileScanResult.Infected) + { + issues.Add( new ValidationIssue + { + InstanceId = instance.Id, + DataElementId = dataElement.Id, + Code = ValidationIssueCodes.DataElementCodes.DataElementFileInfected, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.DataElementCodes.DataElementFileInfected, + Field = dataType.Id + }); + } + + if (dataType.EnableFileScan && dataType.ValidationErrorOnPendingFileScan && + dataElement.FileScanResult == FileScanResult.Pending) + { + issues.Add( new ValidationIssue + { + InstanceId = instance.Id, + DataElementId = dataElement.Id, + Code = ValidationIssueCodes.DataElementCodes.DataElementFileScanPending, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.DataElementCodes.DataElementFileScanPending, + Field = dataType.Id + }); + } + + return Task.FromResult(issues); + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs new file mode 100644 index 000000000..5916f2b62 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs @@ -0,0 +1,63 @@ +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Enums; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// Implement the default validation of DataElements based on the metadata in appMetadata +/// +public class DefaultTaskValidator : ITaskValidator +{ + private readonly IAppMetadata _appMetadata; + + public DefaultTaskValidator([ServiceKey] string taskId, IAppMetadata appMetadata) + { + TaskId = taskId; + _appMetadata = appMetadata; + } + + /// + public string TaskId { get; } + + public async Task> ValidateTask(Instance instance) + { + var messages = new List(); + var application = await _appMetadata.GetApplicationMetadata(); + + foreach (var dataType in application.DataTypes.Where(et => et.TaskId == TaskId)) + { + List elements = instance.Data.Where(d => d.DataType == dataType.Id).ToList(); + + if (dataType.MaxCount > 0 && dataType.MaxCount < elements.Count) + { + var message = new ValidationIssue + { + InstanceId = instance.Id, + Code = ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType, + Field = dataType.Id + }; + messages.Add(message); + } + + if (dataType.MinCount > 0 && dataType.MinCount > elements.Count) + { + var message = new ValidationIssue + { + InstanceId = instance.Id, + Code = ValidationIssueCodes.InstanceCodes.TooFewDataElementsOfType, + Severity = ValidationIssueSeverity.Error, + Description = ValidationIssueCodes.InstanceCodes.TooFewDataElementsOfType, + Field = dataType.Id + }; + messages.Add(message); + } + } + + return messages; + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs new file mode 100644 index 000000000..104a236bb --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -0,0 +1,301 @@ +using System.Text.Json; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Helpers.DataModel; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.Expressions; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// Validates form data against expression validations +/// +public class ExpressionValidator : IFormDataValidator +{ + private readonly ILogger _logger; + private readonly IAppResources _appResourceService; + private readonly LayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; + + /// + /// Constructor + /// + public ExpressionValidator([ServiceKey] string dataType, ILogger logger, IAppResources appResourceService, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer) + { + DataType = dataType; + _logger = logger; + _appResourceService = appResourceService; + _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; + } + + /// + public string DataType { get; } + /// + /// Expression validations should always run (they're likely quicker to run than to figure out if relevant fields changed) + /// + public bool ShouldRunForIncrementalValidation(List? changedFields = null) => true; + + public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) + { + var rawValidationConfig = _appResourceService.GetValidationConfiguration(dataElement.DataType); + if (rawValidationConfig == null) + { + // No validation configuration exists for this data type + return new List(); + } + + var validationConfig = JsonDocument.Parse(rawValidationConfig).RootElement; + var evaluatorState = await _layoutEvaluatorStateInitializer.Init(instance, data, dataElement.Id); + return Validate(validationConfig, evaluatorState, _logger); + } + + + + public static List Validate(JsonElement validationConfig, LayoutEvaluatorState evaluatorState, ILogger logger) + { + var validationIssues = new List(); + var expressionValidations = ParseExpressionValidationConfig(validationConfig, logger); + foreach (var validationObject in expressionValidations) + { + var baseField = validationObject.Key; + var resolvedFields = evaluatorState.GetResolvedKeys(baseField); + var validations = validationObject.Value; + foreach (var resolvedField in resolvedFields) + { + var positionalArguments = new[] { resolvedField }; + foreach (var validation in validations) + { + try + { + if (validation.Condition == null) + { + continue; + } + + var isInvalid = ExpressionEvaluator.EvaluateExpression(evaluatorState, validation.Condition, null, positionalArguments); + if (isInvalid is not bool) + { + throw new ArgumentException($"Validation condition for {resolvedField} did not evaluate to a boolean"); + } + if ((bool)isInvalid) + { + var validationIssue = new ValidationIssue + { + Field = resolvedField, + Severity = validation.Severity ?? ValidationIssueSeverity.Error, + CustomTextKey = validation.Message, + Code = validation.Message, + Source = ValidationIssueSources.Expression, + }; + validationIssues.Add(validationIssue); + } + } + catch(Exception e) + { + logger.LogError(e, "Error while evaluating expression validation for {resolvedField}", resolvedField); + throw; + } + } + } + } + + + return validationIssues; + } + + private static RawExpressionValidation? ResolveValidationDefinition(string name, JsonElement definition, Dictionary resolvedDefinitions, ILogger logger) + { + var resolvedDefinition = new RawExpressionValidation(); + var rawDefinition = definition.Deserialize(new JsonSerializerOptions + { + ReadCommentHandling = JsonCommentHandling.Skip, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + if (rawDefinition == null) + { + logger.LogError($"Validation definition {name} could not be parsed"); + return null; + } + if (rawDefinition.Ref != null) + { + var reference = resolvedDefinitions.GetValueOrDefault(rawDefinition.Ref); + if (reference == null) + { + logger.LogError($"Could not resolve reference {rawDefinition.Ref} for validation {name}"); + return null; + + } + resolvedDefinition.Message = reference.Message; + resolvedDefinition.Condition = reference.Condition; + resolvedDefinition.Severity = reference.Severity; + } + + if (rawDefinition.Message != null) + { + resolvedDefinition.Message = rawDefinition.Message; + } + + if (rawDefinition.Condition != null) + { + resolvedDefinition.Condition = rawDefinition.Condition; + } + + if (rawDefinition.Severity != null) + { + resolvedDefinition.Severity = rawDefinition.Severity; + } + + if (resolvedDefinition.Message == null) + { + logger.LogError($"Validation {name} is missing message"); + return null; + } + + if (resolvedDefinition.Condition == null) + { + logger.LogError($"Validation {name} is missing condition"); + return null; + } + + return resolvedDefinition; + } + + private static ExpressionValidation? ResolveExpressionValidation(string field, JsonElement definition, Dictionary resolvedDefinitions, ILogger logger) + { + + var rawExpressionValidatıon = new RawExpressionValidation(); + + if (definition.ValueKind == JsonValueKind.String) + { + var stringReference = definition.GetString(); + if (stringReference == null) + { + logger.LogError($"Could not resolve null reference for validation for field {field}"); + return null; + } + var reference = resolvedDefinitions.GetValueOrDefault(stringReference); + if (reference == null) + { + logger.LogError($"Could not resolve reference {stringReference} for validation for field {field}"); + return null; + } + rawExpressionValidatıon.Message = reference.Message; + rawExpressionValidatıon.Condition = reference.Condition; + rawExpressionValidatıon.Severity = reference.Severity; + } + else + { + var expressionDefinition = definition.Deserialize(new JsonSerializerOptions + { + ReadCommentHandling = JsonCommentHandling.Skip, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }); + if (expressionDefinition == null) + { + logger.LogError($"Validation for field {field} could not be parsed"); + return null; + } + + if (expressionDefinition.Ref != null) + { + var reference = resolvedDefinitions.GetValueOrDefault(expressionDefinition.Ref); + if (reference == null) + { + logger.LogError($"Could not resolve reference {expressionDefinition.Ref} for validation for field {field}"); + return null; + + } + rawExpressionValidatıon.Message = reference.Message; + rawExpressionValidatıon.Condition = reference.Condition; + rawExpressionValidatıon.Severity = reference.Severity; + } + + if (expressionDefinition.Message != null) + { + rawExpressionValidatıon.Message = expressionDefinition.Message; + } + + if (expressionDefinition.Condition != null) + { + rawExpressionValidatıon.Condition = expressionDefinition.Condition; + } + + if (expressionDefinition.Severity != null) + { + rawExpressionValidatıon.Severity = expressionDefinition.Severity; + } + } + + if (rawExpressionValidatıon.Message == null) + { + logger.LogError($"Validation for field {field} is missing message"); + return null; + } + + if (rawExpressionValidatıon.Condition == null) + { + logger.LogError($"Validation for field {field} is missing condition"); + return null; + } + + var expressionValidation = new ExpressionValidation + { + Message = rawExpressionValidatıon.Message, + Condition = rawExpressionValidatıon.Condition, + Severity = rawExpressionValidatıon.Severity ?? ValidationIssueSeverity.Error, + }; + + return expressionValidation; + } + + private static Dictionary> ParseExpressionValidationConfig(JsonElement expressionValidationConfig, ILogger logger) + { + var expressionValidationDefinitions = new Dictionary(); + JsonElement definitionsObject; + var hasDefinitions = expressionValidationConfig.TryGetProperty("definitions", out definitionsObject); + if (hasDefinitions) + { + foreach (var definitionObject in definitionsObject.EnumerateObject()) + { + var name = definitionObject.Name; + var definition = definitionObject.Value; + var resolvedDefinition = ResolveValidationDefinition(name, definition, expressionValidationDefinitions, logger); + if (resolvedDefinition == null) + { + logger.LogError($"Validation definition {name} could not be resolved"); + continue; + } + expressionValidationDefinitions[name] = resolvedDefinition; + } + } + var expressionValidations = new Dictionary>(); + JsonElement validationsObject; + var hasValidations = expressionValidationConfig.TryGetProperty("validations", out validationsObject); + if (hasValidations) + { + foreach (var validationArray in validationsObject.EnumerateObject()) + { + var field = validationArray.Name; + var validations = validationArray.Value; + foreach (var validation in validations.EnumerateArray()) + { + if (!expressionValidations.ContainsKey(field)) + { + expressionValidations[field] = new List(); + } + var resolvedExpressionValidation = ResolveExpressionValidation(field, validation, expressionValidationDefinitions, logger); + if (resolvedExpressionValidation == null) + { + logger.LogError($"Validation for field {field} could not be resolved"); + continue; + } + expressionValidations[field].Add(resolvedExpressionValidation); + } + } + } + return expressionValidations; + } + +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs new file mode 100644 index 000000000..aea7d7e78 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs @@ -0,0 +1,52 @@ +#pragma warning disable CS0618 // Type or member is obsolete +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation.Helpers; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// +/// +public class LegacyIValidationFormDataValidator : IFormDataValidator +{ + private readonly IInstanceValidator? _instanceValidator; + private readonly GeneralSettings _generalSettings; + + /// + /// constructor + /// + public LegacyIValidationFormDataValidator(IInstanceValidator? instanceValidator, IOptions generalSettings) + { + _instanceValidator = instanceValidator; + _generalSettings = generalSettings.Value; + } + + /// + /// + /// + public string DataType { get; } = "AnyType"; + + /// + /// Always run for incremental validation + /// + public bool ShouldRunForIncrementalValidation(List? changedFields = null) => true; + + + /// + public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) + { + if (_instanceValidator is null) + { + return new List(); + } + + var modelState = new ModelStateDictionary(); + await _instanceValidator.ValidateData(data, modelState); + return ModelStateHelpers.ModelStateToIssueList(modelState, instance, dataElement, _generalSettings, data.GetType(), ValidationIssueSources.Custom); + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs new file mode 100644 index 000000000..02c18c0ec --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs @@ -0,0 +1,47 @@ +#pragma warning disable CS0618 // Type or member is obsolete +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation.Helpers; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; + +namespace Altinn.App.Core.Features.Validation.Default; + +/// +/// Ensures that the old extention hook is still supported. +/// +public class LegacyIValidationTaskValidator : ITaskValidator +{ + private readonly IInstanceValidator? _instanceValidator; + private readonly GeneralSettings _generalSettings; + + /// + /// Constructor + /// + public LegacyIValidationTaskValidator([ServiceKey] string taskId, IInstanceValidator? instanceValidator, IOptions generalSettings) + { + TaskId = taskId; + _instanceValidator = instanceValidator; + _generalSettings = generalSettings.Value; + } + + /// + /// The task id this validator is registrered for. + /// + public string TaskId { get; } + + /// + public async Task> ValidateTask(Instance instance) + { + if (_instanceValidator is null) + { + return new List(); + } + + var modelState = new ModelStateDictionary(); + await _instanceValidator.ValidateTask(instance, TaskId, modelState); + return ModelStateHelpers.MapModelStateToIssueList(modelState, instance, _generalSettings); + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs new file mode 100644 index 000000000..77bd010b2 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs @@ -0,0 +1,40 @@ +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.Expressions; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; + +namespace Altinn.App.Core.Features.Validation.Default; + +public class RequiredLayoutValidator : IFormDataValidator +{ + private readonly LayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; + private readonly IAppResources _appResourcesService; + private readonly IAppMetadata _appMetadata; + + public RequiredLayoutValidator([ServiceKey] string dataType, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, IAppResources appResourcesService, IAppMetadata appMetadata) + { + DataType = dataType; + _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; + _appResourcesService = appResourcesService; + _appMetadata = appMetadata; + } + /// + public string DataType { get; } + + /// + /// Required validator should always run for incremental validation, as they're almost quicker to run than to verify. + /// + public bool ShouldRunForIncrementalValidation(List? changedFields = null) => true; + + /// + /// Validate the form data against the required rules in the layout + /// + public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) + { + var appMetadata = await _appMetadata.GetApplicationMetadata(); + var layoutSet = _appResourcesService.GetLayoutSetForTask(appMetadata.DataTypes.First(dt=>dt.Id == dataElement.DataType).TaskId); + var evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id); + return LayoutEvaluator.RunLayoutValidationsForRequired(evaluationState, dataElement.Id); + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/ExpressionValidator.cs deleted file mode 100644 index 96329e6ea..000000000 --- a/src/Altinn.App.Core/Features/Validation/ExpressionValidator.cs +++ /dev/null @@ -1,276 +0,0 @@ -using System.Text.Json; -using Altinn.App.Core.Helpers; -using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.Expressions; -using Altinn.App.Core.Models.Validation; -using Microsoft.Extensions.Logging; - - -namespace Altinn.App.Core.Features.Validation -{ - /// - /// Validates form data against expression validations - /// - public static class ExpressionValidator - { - /// - public static IEnumerable Validate(string dataType, IAppResources appResourceService, IDataModelAccessor dataModel, LayoutEvaluatorState evaluatorState, ILogger logger) - { - var rawValidationConfig = appResourceService.GetValidationConfiguration(dataType); - if (rawValidationConfig == null) - { - // No validation configuration exists for this data type - return new List(); - } - - var validationConfig = JsonDocument.Parse(rawValidationConfig).RootElement; - return Validate(validationConfig, dataModel, evaluatorState, logger); - } - - /// - public static IEnumerable Validate(JsonElement validationConfig, IDataModelAccessor dataModel, LayoutEvaluatorState evaluatorState, ILogger logger) - { - var validationIssues = new List(); - var expressionValidations = ParseExpressionValidationConfig(validationConfig, logger); - foreach (var validationObject in expressionValidations) - { - var baseField = validationObject.Key; - var resolvedFields = dataModel.GetResolvedKeys(baseField); - var validations = validationObject.Value; - foreach (var resolvedField in resolvedFields) - { - var positionalArguments = new[] { resolvedField }; - foreach (var validation in validations) - { - try - { - if (validation.Condition == null) - { - continue; - } - - var isInvalid = ExpressionEvaluator.EvaluateExpression(evaluatorState, validation.Condition, null, positionalArguments); - if (isInvalid is not bool) - { - throw new ArgumentException($"Validation condition for {resolvedField} did not evaluate to a boolean"); - } - if ((bool)isInvalid) - { - var validationIssue = new ValidationIssue - { - Field = resolvedField, - Severity = validation.Severity ?? ValidationIssueSeverity.Error, - CustomTextKey = validation.Message, - Code = validation.Message, - Source = ValidationIssueSources.Expression, - }; - validationIssues.Add(validationIssue); - } - } - catch(Exception e) - { - logger.LogError(e, "Error while evaluating expression validation for {resolvedField}", resolvedField); - throw; - } - } - } - } - - - return validationIssues; - } - - private static RawExpressionValidation? ResolveValidationDefinition(string name, JsonElement definition, Dictionary resolvedDefinitions, ILogger logger) - { - var resolvedDefinition = new RawExpressionValidation(); - var rawDefinition = definition.Deserialize(new JsonSerializerOptions - { - ReadCommentHandling = JsonCommentHandling.Skip, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }); - if (rawDefinition == null) - { - logger.LogError($"Validation definition {name} could not be parsed"); - return null; - } - if (rawDefinition.Ref != null) - { - var reference = resolvedDefinitions.GetValueOrDefault(rawDefinition.Ref); - if (reference == null) - { - logger.LogError($"Could not resolve reference {rawDefinition.Ref} for validation {name}"); - return null; - - } - resolvedDefinition.Message = reference.Message; - resolvedDefinition.Condition = reference.Condition; - resolvedDefinition.Severity = reference.Severity; - } - - if (rawDefinition.Message != null) - { - resolvedDefinition.Message = rawDefinition.Message; - } - - if (rawDefinition.Condition != null) - { - resolvedDefinition.Condition = rawDefinition.Condition; - } - - if (rawDefinition.Severity != null) - { - resolvedDefinition.Severity = rawDefinition.Severity; - } - - if (resolvedDefinition.Message == null) - { - logger.LogError($"Validation {name} is missing message"); - return null; - } - - if (resolvedDefinition.Condition == null) - { - logger.LogError($"Validation {name} is missing condition"); - return null; - } - - return resolvedDefinition; - } - - private static ExpressionValidation? ResolveExpressionValidation(string field, JsonElement definition, Dictionary resolvedDefinitions, ILogger logger) - { - - var rawExpressionValidatıon = new RawExpressionValidation(); - - if (definition.ValueKind == JsonValueKind.String) - { - var stringReference = definition.GetString(); - if (stringReference == null) - { - logger.LogError($"Could not resolve null reference for validation for field {field}"); - return null; - } - var reference = resolvedDefinitions.GetValueOrDefault(stringReference); - if (reference == null) - { - logger.LogError($"Could not resolve reference {stringReference} for validation for field {field}"); - return null; - } - rawExpressionValidatıon.Message = reference.Message; - rawExpressionValidatıon.Condition = reference.Condition; - rawExpressionValidatıon.Severity = reference.Severity; - } - else - { - var expressionDefinition = definition.Deserialize(new JsonSerializerOptions - { - ReadCommentHandling = JsonCommentHandling.Skip, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }); - if (expressionDefinition == null) - { - logger.LogError($"Validation for field {field} could not be parsed"); - return null; - } - - if (expressionDefinition.Ref != null) - { - var reference = resolvedDefinitions.GetValueOrDefault(expressionDefinition.Ref); - if (reference == null) - { - logger.LogError($"Could not resolve reference {expressionDefinition.Ref} for validation for field {field}"); - return null; - - } - rawExpressionValidatıon.Message = reference.Message; - rawExpressionValidatıon.Condition = reference.Condition; - rawExpressionValidatıon.Severity = reference.Severity; - } - - if (expressionDefinition.Message != null) - { - rawExpressionValidatıon.Message = expressionDefinition.Message; - } - - if (expressionDefinition.Condition != null) - { - rawExpressionValidatıon.Condition = expressionDefinition.Condition; - } - - if (expressionDefinition.Severity != null) - { - rawExpressionValidatıon.Severity = expressionDefinition.Severity; - } - } - - if (rawExpressionValidatıon.Message == null) - { - logger.LogError($"Validation for field {field} is missing message"); - return null; - } - - if (rawExpressionValidatıon.Condition == null) - { - logger.LogError($"Validation for field {field} is missing condition"); - return null; - } - - var expressionValidation = new ExpressionValidation - { - Message = rawExpressionValidatıon.Message, - Condition = rawExpressionValidatıon.Condition, - Severity = rawExpressionValidatıon.Severity ?? ValidationIssueSeverity.Error, - }; - - return expressionValidation; - } - - private static Dictionary> ParseExpressionValidationConfig(JsonElement expressionValidationConfig, ILogger logger) - { - var expressionValidationDefinitions = new Dictionary(); - JsonElement definitionsObject; - var hasDefinitions = expressionValidationConfig.TryGetProperty("definitions", out definitionsObject); - if (hasDefinitions) - { - foreach (var definitionObject in definitionsObject.EnumerateObject()) - { - var name = definitionObject.Name; - var definition = definitionObject.Value; - var resolvedDefinition = ResolveValidationDefinition(name, definition, expressionValidationDefinitions, logger); - if (resolvedDefinition == null) - { - logger.LogError($"Validation definition {name} could not be resolved"); - continue; - } - expressionValidationDefinitions[name] = resolvedDefinition; - } - } - var expressionValidations = new Dictionary>(); - JsonElement validationsObject; - var hasValidations = expressionValidationConfig.TryGetProperty("validations", out validationsObject); - if (hasValidations) - { - foreach (var validationArray in validationsObject.EnumerateObject()) - { - var field = validationArray.Name; - var validations = validationArray.Value; - foreach (var validation in validations.EnumerateArray()) - { - if (!expressionValidations.ContainsKey(field)) - { - expressionValidations[field] = new List(); - } - var resolvedExpressionValidation = ResolveExpressionValidation(field, validation, expressionValidationDefinitions, logger); - if (resolvedExpressionValidation == null) - { - logger.LogError($"Validation for field {field} could not be resolved"); - continue; - } - expressionValidations[field].Add(resolvedExpressionValidation); - } - } - } - return expressionValidations; - } - } -} diff --git a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs new file mode 100644 index 000000000..e5243b4bd --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs @@ -0,0 +1,108 @@ +using System.Diagnostics; +using System.Linq.Expressions; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation; + +/// +/// Simple wrapper for validation of form data that does the type checking for you. +/// +/// The type of the model this class will validate +public abstract class GenericFormDataValidator : IFormDataValidator +{ + /// + /// Constructor to force the DataType to be set. + /// + /// + protected GenericFormDataValidator(string dataType) + { + DataType = dataType; + } + /// + public string DataType { get; private init; } + + private readonly List _runForPrefixes = new List(); + // ReSharper disable once StaticMemberInGenericType + private static readonly AsyncLocal> ValidationIssues = new(); + + /// + /// Default implementation that respects the runFor prefixes. + /// + public bool ShouldRunForIncrementalValidation(List? changedFields = null) + { + if (changedFields == null) + { + return true; + } + + if (_runForPrefixes.Count == 0) + { + return true; + } + + foreach (var prefix in _runForPrefixes) + { + foreach (var changedField in changedFields) + { + if (IsMatch(changedField, prefix)) + { + return true; + } + } + } + + return false; + } + + private static bool IsMatch(string changedField, string prefix) + { + return changedField.StartsWith(prefix) || prefix.StartsWith(changedField); + } + + /// + /// Easy way to configure to only run for fields that start with the given prefix. + /// + /// A selector that will be translated into a prefix + /// The type of the selected element (only for making the compiler happy) + protected void RunFor(Expression> selector) + { + _runForPrefixes.Add(LinqExpressionHelpers.GetJsonPath(selector)); + } + + protected void CreateValidationIssue(Expression> selector, string textKey, ValidationIssueSeverity severity = ValidationIssueSeverity.Error) + { + Debug.Assert(ValidationIssues.Value is not null); + ValidationIssues.Value.Add( new ValidationIssue + { + Field = LinqExpressionHelpers.GetJsonPath(selector), + CustomTextKey = textKey, + Severity = severity + }); + } + + protected void AddValidationIssue(ValidationIssue issue) + { + Debug.Assert(ValidationIssues.Value is not null); + ValidationIssues.Value.Add(issue); + } + + public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) + { + if (data is not TModel model) + { + throw new ArgumentException($"Data is not of type {typeof(TModel)}"); + } + + ValidationIssues.Value = new List();; + await ValidateFormData(instance, dataElement, model); + return ValidationIssues.Value; + + } + + /// + /// Implement this method to validate the data. + /// + protected abstract Task ValidateFormData(Instance instance, DataElement dataElement, TModel data, List? changedFields = null); +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs new file mode 100644 index 000000000..221d82556 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs @@ -0,0 +1,156 @@ +using System.Collections; +using System.Text.Json.Serialization; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.AspNetCore.Mvc.ModelBinding; + +namespace Altinn.App.Core.Features.Validation.Helpers; + +public static class ModelStateHelpers +{ + public static List ModelStateToIssueList(ModelStateDictionary modelState, Instance instance, + DataElement dataElement, GeneralSettings generalSettings, Type objectType, string source) + { + var validationIssues = new List(); + + foreach (var modelKey in modelState.Keys) + { + modelState.TryGetValue(modelKey, out var entry); + + if (entry is { ValidationState: ModelValidationState.Invalid }) + { + foreach (var error in entry.Errors) + { + var severityAndMessage = GetSeverityFromMessage(error.ErrorMessage, generalSettings); + validationIssues.Add(new ValidationIssue + { + InstanceId = instance.Id, + DataElementId = dataElement.Id, + Source = source, + Code = severityAndMessage.Message, + Field = ModelKeyToField(modelKey, objectType)!, + Severity = severityAndMessage.Severity, + Description = severityAndMessage.Message + }); + } + } + } + + return validationIssues; + } + + private static (ValidationIssueSeverity Severity, string Message) GetSeverityFromMessage(string originalMessage, + GeneralSettings generalSettings) + { + if (originalMessage.StartsWith(generalSettings.SoftValidationPrefix)) + { + return (ValidationIssueSeverity.Warning, + originalMessage.Remove(0, generalSettings.SoftValidationPrefix.Length)); + } + + if (generalSettings.FixedValidationPrefix != null + && originalMessage.StartsWith(generalSettings.FixedValidationPrefix)) + { + return (ValidationIssueSeverity.Fixed, + originalMessage.Remove(0, generalSettings.FixedValidationPrefix.Length)); + } + + if (originalMessage.StartsWith(generalSettings.InfoValidationPrefix)) + { + return (ValidationIssueSeverity.Informational, + originalMessage.Remove(0, generalSettings.InfoValidationPrefix.Length)); + } + + if (originalMessage.StartsWith(generalSettings.SuccessValidationPrefix)) + { + return (ValidationIssueSeverity.Success, + originalMessage.Remove(0, generalSettings.SuccessValidationPrefix.Length)); + } + + return (ValidationIssueSeverity.Error, originalMessage); + } + + /// + /// Translate the ModelKey from validation to a field that respects [JsonPropertyName] annotations + /// + /// + /// Will be obsolete when updating to net70 or higher and activating + /// https://learn.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-7.0#use-json-property-names-in-validation-errors + /// + public static string? ModelKeyToField(string? modelKey, Type data) + { + var keyParts = modelKey?.Split('.', 2); + var keyWithIndex = keyParts?.ElementAtOrDefault(0)?.Split('[', 2); + var key = keyWithIndex?.ElementAtOrDefault(0); + var index = keyWithIndex?.ElementAtOrDefault(1); // with traling ']', eg: "3]" + var rest = keyParts?.ElementAtOrDefault(1); + + var property = data?.GetProperties()?.FirstOrDefault(p => p.Name == key); + var jsonPropertyName = property + ?.GetCustomAttributes(true) + .OfType() + .FirstOrDefault() + ?.Name; + if (jsonPropertyName is null) + { + jsonPropertyName = key; + } + + if (index is not null) + { + jsonPropertyName = jsonPropertyName + '[' + index; + } + + if (rest is null) + { + return jsonPropertyName; + } + + var childType = property?.PropertyType; + + // Get the Parameter of IEnumerable properties, if they are not string + if (childType is not null && childType != typeof(string) && childType.IsAssignableTo(typeof(IEnumerable))) + { + childType = childType.GetInterfaces() + .Where(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(IEnumerable<>)) + .Select(t => t.GetGenericArguments()[0]).FirstOrDefault(); + } + + if (childType is null) + { + // Give up and return rest, if the child type is not found. + return $"{jsonPropertyName}.{rest}"; + } + + return $"{jsonPropertyName}.{ModelKeyToField(rest, childType)}"; + } + + public static List MapModelStateToIssueList(ModelStateDictionary modelState, Instance instance, + GeneralSettings generalSettings) + { + var validationIssues = new List(); + + foreach (var modelKey in modelState.Keys) + { + modelState.TryGetValue(modelKey, out var entry); + + if (entry != null && entry.ValidationState == ModelValidationState.Invalid) + { + foreach (var error in entry.Errors) + { + var severityAndMessage = GetSeverityFromMessage(error.ErrorMessage, generalSettings); + validationIssues.Add(new ValidationIssue + { + InstanceId = instance.Id, + Code = severityAndMessage.Message, + Severity = severityAndMessage.Severity, + Description = severityAndMessage.Message + }); + } + } + } + + return validationIssues; + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs b/src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs new file mode 100644 index 000000000..ba4da23df --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs @@ -0,0 +1,38 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation; + +/// +/// Validator for data elements. +/// See for an alternative validator for data elements with app logic. +/// and that support incremental validation on save. +/// For validating the content of files, see and +/// +public interface IDataElementValidator +{ + /// + /// The data type that this validator should run for. This is the id of the data type from applicationmetadata.json + /// + /// + /// Used by default in . Overrides might ignore this. + /// + string DataType { get; } + + /// + /// Override this method to customize what data elements this validator should run for. + /// + bool CanValidateDataType(DataType dataType) + { + return DataType == dataType.Id; + } + + /// + /// Run validations for a data element. This is supposed to run quickly + /// + /// The instance to validate + /// + /// + /// + public Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType); +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/IValidation.cs b/src/Altinn.App.Core/Features/Validation/IValidation.cs deleted file mode 100644 index 78c54f791..000000000 --- a/src/Altinn.App.Core/Features/Validation/IValidation.cs +++ /dev/null @@ -1,28 +0,0 @@ -using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; - -namespace Altinn.App.Core.Features.Validation -{ - /// - /// Describes the public methods of a validation service - /// - public interface IValidation - { - /// - /// Validate an instance for a specified process step. - /// - /// The instance to validate - /// The task to validate the instance for. - /// A list of validation errors if any were found - Task> ValidateAndUpdateProcess(Instance instance, string taskId); - - /// - /// Validate a specific data element. - /// - /// The instance where the data element belong - /// The datatype describing the data element requirements - /// The metadata of a data element to validate - /// A list of validation errors if any were found - Task> ValidateDataElement(Instance instance, DataType dataType, DataElement dataElement); - } -} diff --git a/src/Altinn.App.Core/Features/Validation/IValidationService.cs b/src/Altinn.App.Core/Features/Validation/IValidationService.cs new file mode 100644 index 000000000..1f9ac0739 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/IValidationService.cs @@ -0,0 +1,53 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation; + +/// +/// Core interface for validation of instances. Only a single implementation of this interface should exist in the app. +/// +public interface IValidationService +{ + /// + /// Validates the instance with all data elements on the current task and ensures that the instance is read for process next. + /// + /// + /// This method executes validations in the following interfaces + /// * for the current task + /// * for all data elements on the current task + /// * for all data elements with app logic on the current task + /// + /// The instance to validate + /// instance.Process?.CurrentTask?.ElementId + /// List of validation issues for this data element + Task> ValidateInstanceAtTask(Instance instance, string taskId); + + /// + /// + /// + /// + /// This method executes validations in the following interfaces + /// * for all data elements on the current task + /// * for all data elements with app logic on the current task + /// + /// This method does not run task validations + /// + /// The instance to validate + /// The data element to run validations for + /// The data type (from applicationmetadata) that the element is an instance of + /// List of validation issues for this data element + Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType); + + /// + /// Validates a single data element. Used by frontend to continuously validate form data as it changes. + /// + /// + /// This method executes validations for + /// + /// The instance to validate + /// The data element to run validations for + /// The type of the data element + /// The data deserialized to the strongly typed object that represents the form data + /// List of json paths for the fields that have changed (used for incremental validation) + Task>> ValidateFormData(Instance instance, DataElement dataElement, DataType dataType, object data, List? changedFields = null); +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/NullInstanceValidator.cs b/src/Altinn.App.Core/Features/Validation/NullInstanceValidator.cs deleted file mode 100644 index 4e3919be4..000000000 --- a/src/Altinn.App.Core/Features/Validation/NullInstanceValidator.cs +++ /dev/null @@ -1,23 +0,0 @@ -using Altinn.Platform.Storage.Interface.Models; -using Microsoft.AspNetCore.Mvc.ModelBinding; - -namespace Altinn.App.Core.Features.Validation; - -/// -/// Default implementation of the IInstanceValidator interface. -/// This implementation does not do any validation and always returns true. -/// -public class NullInstanceValidator: IInstanceValidator -{ - /// - public async Task ValidateData(object data, ModelStateDictionary validationResults) - { - await Task.CompletedTask; - } - - /// - public async Task ValidateTask(Instance instance, string taskId, ModelStateDictionary validationResults) - { - await Task.CompletedTask; - } -} \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs b/src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs deleted file mode 100644 index cfb9028e1..000000000 --- a/src/Altinn.App.Core/Features/Validation/ValidationAppSI.cs +++ /dev/null @@ -1,433 +0,0 @@ -using Altinn.App.Core.Configuration; -using Altinn.App.Core.Helpers.DataModel; -using Altinn.App.Core.Helpers; -using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.AppModel; -using Altinn.App.Core.Internal.Data; -using Altinn.App.Core.Internal.Expressions; -using Altinn.App.Core.Internal.Instances; -using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Enums; -using Altinn.Platform.Storage.Interface.Models; -using Microsoft.AspNetCore.Http; -using Microsoft.AspNetCore.Mvc; -using Microsoft.AspNetCore.Mvc.Abstractions; -using Microsoft.AspNetCore.Mvc.ModelBinding; -using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Options; - -namespace Altinn.App.Core.Features.Validation -{ - /// - /// Represents a validation service for validating instances and their data elements - /// - public class ValidationAppSI : IValidation - { - private readonly ILogger _logger; - private readonly IDataClient _dataClient; - private readonly IInstanceClient _instanceClient; - private readonly IInstanceValidator _instanceValidator; - private readonly IAppModel _appModel; - private readonly IAppResources _appResourcesService; - private readonly IAppMetadata _appMetadata; - private readonly LayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; - private readonly IObjectModelValidator _objectModelValidator; - private readonly IHttpContextAccessor _httpContextAccessor; - private readonly GeneralSettings _generalSettings; - private readonly AppSettings _appSettings; - - /// - /// Initializes a new instance of the class. - /// - public ValidationAppSI( - ILogger logger, - IDataClient dataClient, - IInstanceClient instanceClient, - IInstanceValidator instanceValidator, - IAppModel appModel, - IAppResources appResourcesService, - IAppMetadata appMetadata, - IObjectModelValidator objectModelValidator, - LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, - IHttpContextAccessor httpContextAccessor, - IOptions generalSettings, - IOptions appSettings) - { - _logger = logger; - _dataClient = dataClient; - _instanceClient = instanceClient; - _instanceValidator = instanceValidator; - _appModel = appModel; - _appResourcesService = appResourcesService; - _appMetadata = appMetadata; - _objectModelValidator = objectModelValidator; - _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; - _httpContextAccessor = httpContextAccessor; - _generalSettings = generalSettings.Value; - _appSettings = appSettings.Value; - } - - /// - /// Validate an instance for a specified process step. - /// - /// The instance to validate - /// The task to validate the instance for. - /// A list of validation errors if any were found - public async Task> ValidateAndUpdateProcess(Instance instance, string taskId) - { - _logger.LogInformation("Validation of {instance.Id}", instance.Id); - - List messages = new List(); - - ModelStateDictionary validationResults = new ModelStateDictionary(); - await _instanceValidator.ValidateTask(instance, taskId, validationResults); - messages.AddRange(MapModelStateToIssueList(validationResults, instance)); - - Application application = await _appMetadata.GetApplicationMetadata(); - - foreach (DataType dataType in application.DataTypes.Where(et => et.TaskId == taskId)) - { - List elements = instance.Data.Where(d => d.DataType == dataType.Id).ToList(); - - if (dataType.MaxCount > 0 && dataType.MaxCount < elements.Count) - { - ValidationIssue message = new ValidationIssue - { - InstanceId = instance.Id, - Code = ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType, - Field = dataType.Id - }; - messages.Add(message); - } - - if (dataType.MinCount > 0 && dataType.MinCount > elements.Count) - { - ValidationIssue message = new ValidationIssue - { - InstanceId = instance.Id, - Code = ValidationIssueCodes.InstanceCodes.TooFewDataElementsOfType, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.InstanceCodes.TooFewDataElementsOfType, - Field = dataType.Id - }; - messages.Add(message); - } - - foreach (DataElement dataElement in elements) - { - messages.AddRange(await ValidateDataElement(instance, dataType, dataElement)); - } - } - - instance.Process.CurrentTask.Validated = new ValidationStatus - { - // The condition for completion is met if there are no errors (or other weirdnesses). - CanCompleteTask = messages.Count == 0 || - messages.All(m => m.Severity != ValidationIssueSeverity.Error && m.Severity != ValidationIssueSeverity.Unspecified), - Timestamp = DateTime.Now - }; - - await _instanceClient.UpdateProcess(instance); - return messages; - } - - /// - /// Validate a specific data element. - /// - /// The instance where the data element belong - /// The datatype describing the data element requirements - /// The metadata of a data element to validate - /// A list of validation errors if any were found - public async Task> ValidateDataElement(Instance instance, DataType dataType, DataElement dataElement) - { - _logger.LogInformation("Validation of data element {dataElement.Id} of instance {instance.Id}", dataElement.Id, instance.Id); - - List messages = new List(); - - if (dataElement.ContentType == null) - { - ValidationIssue message = new ValidationIssue - { - InstanceId = instance.Id, - Code = ValidationIssueCodes.DataElementCodes.MissingContentType, - DataElementId = dataElement.Id, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.MissingContentType - }; - messages.Add(message); - } - else - { - string contentTypeWithoutEncoding = dataElement.ContentType.Split(";")[0]; - - if (dataType.AllowedContentTypes != null && dataType.AllowedContentTypes.Count > 0 && dataType.AllowedContentTypes.All(ct => !ct.Equals(contentTypeWithoutEncoding, StringComparison.OrdinalIgnoreCase))) - { - ValidationIssue message = new ValidationIssue - { - InstanceId = instance.Id, - DataElementId = dataElement.Id, - Code = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, - Field = dataType.Id - }; - messages.Add(message); - } - } - - if (dataType.MaxSize.HasValue && dataType.MaxSize > 0 && (long)dataType.MaxSize * 1024 * 1024 < dataElement.Size) - { - ValidationIssue message = new ValidationIssue - { - InstanceId = instance.Id, - DataElementId = dataElement.Id, - Code = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, - Field = dataType.Id - }; - messages.Add(message); - } - - if (dataType.EnableFileScan && dataElement.FileScanResult == FileScanResult.Infected) - { - ValidationIssue message = new ValidationIssue() - { - InstanceId = instance.Id, - DataElementId = dataElement.Id, - Code = ValidationIssueCodes.DataElementCodes.DataElementFileInfected, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.DataElementFileInfected, - Field = dataType.Id - }; - messages.Add(message); - } - - if (dataType.EnableFileScan && dataType.ValidationErrorOnPendingFileScan && dataElement.FileScanResult == FileScanResult.Pending) - { - ValidationIssue message = new ValidationIssue() - { - InstanceId = instance.Id, - DataElementId = dataElement.Id, - Code = ValidationIssueCodes.DataElementCodes.DataElementFileScanPending, - Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.DataElementFileScanPending, - Field = dataType.Id - }; - messages.Add(message); - } - - if (dataType.AppLogic?.ClassRef != null) - { - Type modelType = _appModel.GetModelType(dataType.AppLogic.ClassRef); - Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); - string app = instance.AppId.Split("/")[1]; - int instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId); - object data = await _dataClient.GetFormData( - instanceGuid, modelType, instance.Org, app, instanceOwnerPartyId, Guid.Parse(dataElement.Id)); - - LayoutEvaluatorState? evaluationState = null; - - // Remove hidden data before validation - if (_appSettings.RequiredValidation || _appSettings.ExpressionValidation) - { - - var layoutSet = _appResourcesService.GetLayoutSetForTask(dataType.TaskId); - evaluationState = await _layoutEvaluatorStateInitializer.Init(instance, data, layoutSet?.Id); - LayoutEvaluator.RemoveHiddenData(evaluationState, RowRemovalOption.SetToNull); - } - - // Evaluate expressions in layout and validate that all required data is included and that maxLength - // is respected on groups - if (_appSettings.RequiredValidation) - { - var layoutErrors = LayoutEvaluator.RunLayoutValidationsForRequired(evaluationState!, dataElement.Id); - messages.AddRange(layoutErrors); - } - - // Run expression validations - if (_appSettings.ExpressionValidation) - { - var expressionErrors = ExpressionValidator.Validate(dataType.Id, _appResourcesService, new DataModel(data), evaluationState!, _logger); - messages.AddRange(expressionErrors); - } - - // Run Standard mvc validation using the System.ComponentModel.DataAnnotations - ModelStateDictionary dataModelValidationResults = new ModelStateDictionary(); - var actionContext = new ActionContext( - _httpContextAccessor.HttpContext, - new Microsoft.AspNetCore.Routing.RouteData(), - new ActionDescriptor(), - dataModelValidationResults); - ValidationStateDictionary validationState = new ValidationStateDictionary(); - _objectModelValidator.Validate(actionContext, validationState, null, data); - - if (!dataModelValidationResults.IsValid) - { - messages.AddRange(MapModelStateToIssueList(actionContext.ModelState, ValidationIssueSources.ModelState, instance, dataElement.Id, data.GetType())); - } - - // Call custom validation from the IInstanceValidator - ModelStateDictionary customValidationResults = new ModelStateDictionary(); - await _instanceValidator.ValidateData(data, customValidationResults); - - if (!customValidationResults.IsValid) - { - messages.AddRange(MapModelStateToIssueList(customValidationResults, ValidationIssueSources.Custom, instance, dataElement.Id, data.GetType())); - } - - } - - return messages; - } - - private List MapModelStateToIssueList( - ModelStateDictionary modelState, - string source, - Instance instance, - string dataElementId, - Type modelType) - { - List validationIssues = new List(); - - foreach (string modelKey in modelState.Keys) - { - modelState.TryGetValue(modelKey, out ModelStateEntry? entry); - - if (entry != null && entry.ValidationState == ModelValidationState.Invalid) - { - foreach (ModelError error in entry.Errors) - { - var severityAndMessage = GetSeverityFromMessage(error.ErrorMessage); - validationIssues.Add(new ValidationIssue - { - InstanceId = instance.Id, - DataElementId = dataElementId, - Source = source, - Code = severityAndMessage.Message, - Field = ModelKeyToField(modelKey, modelType)!, - Severity = severityAndMessage.Severity, - Description = severityAndMessage.Message - }); - } - } - } - - return validationIssues; - } - - /// - /// Translate the ModelKey from validation to a field that respects [JsonPropertyName] annotations - /// - /// - /// Will be obsolete when updating to net70 or higher and activating https://learn.microsoft.com/en-us/aspnet/core/mvc/models/validation?view=aspnetcore-7.0#use-json-property-names-in-validation-errors - /// - public static string? ModelKeyToField(string? modelKey, Type data) - { - var keyParts = modelKey?.Split('.', 2); - var keyWithIndex = keyParts?.ElementAtOrDefault(0)?.Split('[', 2); - var key = keyWithIndex?.ElementAtOrDefault(0); - var index = keyWithIndex?.ElementAtOrDefault(1); // with traling ']', eg: "3]" - var rest = keyParts?.ElementAtOrDefault(1); - - var property = data?.GetProperties()?.FirstOrDefault(p => p.Name == key); - var jsonPropertyName = property - ?.GetCustomAttributes(true) - .OfType() - .FirstOrDefault() - ?.Name; - if (jsonPropertyName is null) - { - jsonPropertyName = key; - } - - if (index is not null) - { - jsonPropertyName = jsonPropertyName + '[' + index; - } - - if (rest is null) - { - return jsonPropertyName; - } - - var childType = property?.PropertyType; - - // Get the Parameter of IEnumerable properties, if they are not string - if (childType is not null && childType != typeof(string) && childType.IsAssignableTo(typeof(System.Collections.IEnumerable))) - { - childType = childType.GetInterfaces() - .Where(t => t.IsGenericType && t.GetGenericTypeDefinition() == typeof(IEnumerable<>)) - .Select(t => t.GetGenericArguments()[0]).FirstOrDefault(); - } - - if (childType is null) - { - // Give up and return rest, if the child type is not found. - return $"{jsonPropertyName}.{rest}"; - } - - return $"{jsonPropertyName}.{ModelKeyToField(rest, childType)}"; - } - - private List MapModelStateToIssueList(ModelStateDictionary modelState, Instance instance) - { - List validationIssues = new List(); - - foreach (string modelKey in modelState.Keys) - { - modelState.TryGetValue(modelKey, out ModelStateEntry? entry); - - if (entry != null && entry.ValidationState == ModelValidationState.Invalid) - { - foreach (ModelError error in entry.Errors) - { - var severityAndMessage = GetSeverityFromMessage(error.ErrorMessage); - validationIssues.Add(new ValidationIssue - { - InstanceId = instance.Id, - Code = severityAndMessage.Message, - Severity = severityAndMessage.Severity, - Description = severityAndMessage.Message - }); - } - } - } - - return validationIssues; - } - - private (ValidationIssueSeverity Severity, string Message) GetSeverityFromMessage(string originalMessage) - { - if (originalMessage.StartsWith(_generalSettings.SoftValidationPrefix)) - { - return (ValidationIssueSeverity.Warning, - originalMessage.Remove(0, _generalSettings.SoftValidationPrefix.Length)); - } - - if (_generalSettings.FixedValidationPrefix != null - && originalMessage.StartsWith(_generalSettings.FixedValidationPrefix)) - { - return (ValidationIssueSeverity.Fixed, - originalMessage.Remove(0, _generalSettings.FixedValidationPrefix.Length)); - } - - if (originalMessage.StartsWith(_generalSettings.InfoValidationPrefix)) - { - return (ValidationIssueSeverity.Informational, - originalMessage.Remove(0, _generalSettings.InfoValidationPrefix.Length)); - } - - if (originalMessage.StartsWith(_generalSettings.SuccessValidationPrefix)) - { - return (ValidationIssueSeverity.Success, - originalMessage.Remove(0, _generalSettings.SuccessValidationPrefix.Length)); - } - - return (ValidationIssueSeverity.Error, originalMessage); - } - } -} diff --git a/src/Altinn.App.Core/Features/Validation/ValidationService.cs b/src/Altinn.App.Core/Features/Validation/ValidationService.cs new file mode 100644 index 000000000..4280cd386 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/ValidationService.cs @@ -0,0 +1,161 @@ +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.AppModel; +using Altinn.App.Core.Internal.Data; +using Altinn.App.Core.Internal.Process.Elements; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; + +namespace Altinn.App.Core.Features.Validation; + +/// +/// Main validation service that encapsulates all validation logic +/// +public class ValidationService : IValidationService +{ + private readonly IServiceProvider _serviceProvider; + private readonly IDataClient _dataClient; + private readonly IAppModel _appModel; + private readonly IAppMetadata _appMetadata; + private readonly ILogger _logger; + + /// + /// Constructor with DI serivces + /// + public ValidationService(IServiceProvider serviceProvider, IDataClient dataClient, IAppModel appModel, IAppMetadata appMetadata, ILogger logger) + { + _serviceProvider = serviceProvider; + _dataClient = dataClient; + _appModel = appModel; + _appMetadata = appMetadata; + _logger = logger; + } + + /// + public async Task> ValidateInstanceAtTask(Instance instance, string taskId) + { + ArgumentNullException.ThrowIfNull(instance); + ArgumentNullException.ThrowIfNull(taskId); + + var issues = new List(); + + // Run task validations + var taskValidators = _serviceProvider.GetServices() + .Where(tv => tv.TaskId == taskId) + .Concat(_serviceProvider.GetKeyedServices(taskId)) + .ToArray(); + + var taskIssuesTask = Task.WhenAll(taskValidators.Select(tv => + { + try + { + _logger.LogDebug("Start running validator {validatorName} on task {taskId} in instance {instanceId}", tv.GetType().Name, taskId, instance.Id); + return tv.ValidateTask(instance); + } + catch (Exception e) + { + _logger.LogError(e, "Error while running validator {validatorName} on task {taskId} in instance {instanceId}", tv.GetType().Name, taskId, instance.Id); + throw; + } + })); + + // Run validations for single data elements + var application = await _appMetadata.GetApplicationMetadata(); + var dataTypesForTask = application.DataTypes.Where(dt => dt.TaskId == taskId).ToArray(); + var dataElementsToValidate = instance.Data.Where(de => dataTypesForTask.Any(dt => dt.Id == de.DataType)).ToArray(); + var dataIssuesTask = Task.WhenAll(dataElementsToValidate.Select(dataElement=>ValidateDataElement(instance, dataElement, dataTypesForTask.First(dt=>dt.Id == dataElement.DataType) ))); + + return (await Task.WhenAll(taskIssuesTask, dataIssuesTask)).SelectMany(x=>x.SelectMany(y=>y)).ToList(); + } + + + /// + public async Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType) + { + ArgumentNullException.ThrowIfNull(instance); + ArgumentNullException.ThrowIfNull(dataElement); + ArgumentNullException.ThrowIfNull(dataElement.DataType); + + // Get both keyed and non-keyed validators for the data type + var validators = _serviceProvider.GetServices() + .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) + .Where(v => v.CanValidateDataType(dataType)); + + var dataElementsIssuesTask = Task.WhenAll(validators.Select(async v => + { + try + { + _logger.LogDebug("Start running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", v.GetType().Name, dataElement.DataType, dataElement.Id, instance.Id); + return await v.ValidateDataElement(instance, dataElement, dataType); + } + catch (Exception e) + { + _logger.LogError(e, "Error while running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", v.GetType().Name, dataElement.DataType, dataElement.Id, instance.Id); + throw; + } + })); + + // Run extra validation on form data elements with app logic + if(dataType.AppLogic?.ClassRef is not null) + { + Type modelType = _appModel.GetModelType(dataType.AppLogic.ClassRef); + + Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); + string app = instance.AppId.Split("/")[1]; + int instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId); + var data = await _dataClient.GetFormData(instanceGuid, modelType, instance.Org, app, instanceOwnerPartyId, Guid.Parse(dataElement.Id)); // TODO: Add method that accepts instance and dataElement + var formDataIssuesDictionary = await ValidateFormData(instance, dataElement, dataType, data, null); + + return (await dataElementsIssuesTask).SelectMany(x=>x) + .Concat(formDataIssuesDictionary.SelectMany(kv=>kv.Value)) + .ToList(); + }; + + return (await dataElementsIssuesTask).SelectMany(x=>x).ToList(); + } + + /// + public async Task>> ValidateFormData(Instance instance, + DataElement dataElement, DataType dataType, object data, List? changedFields = null) + { + ArgumentNullException.ThrowIfNull(instance); + ArgumentNullException.ThrowIfNull(dataElement); + ArgumentNullException.ThrowIfNull(dataElement.DataType); + ArgumentNullException.ThrowIfNull(data); + + // Locate the relevant data validator services from normal and keyed services + var dataValidators = _serviceProvider.GetServices() + .Where(dv => dv.CanValidateDataType(dataElement.DataType)) + .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) + .Where(dv => dv.ShouldRunForIncrementalValidation(changedFields)) + .ToArray(); + + if (dataValidators.Length > 0) + { + // TODO: Remove hidden data before validation + } + + var issuesLists = await Task.WhenAll(dataValidators.Select(async (v) => + { + try + { + _logger.LogDebug("Start running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", v.GetType().Name, dataElement.DataType, dataElement.Id, instance.Id); + var issues = await v.ValidateFormData(instance, dataElement, data, changedFields); + if (v.Code is not null) + { + issues.ForEach(i=>i.Code = v.Code);// Ensure that the code is set to the validator code + } + return issues; + } + catch (Exception e) + { + _logger.LogError(e, "Error while running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", v.GetType().Name, dataElement.DataType, dataElement.Id, instance.Id); + throw; + } + })); + + return dataValidators.Zip(issuesLists).ToDictionary(kv => kv.First.Code ?? string.Empty, kv => kv.Second); + } + +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Helpers/LinqExpressionHelpers.cs b/src/Altinn.App.Core/Helpers/LinqExpressionHelpers.cs new file mode 100644 index 000000000..406f815f4 --- /dev/null +++ b/src/Altinn.App.Core/Helpers/LinqExpressionHelpers.cs @@ -0,0 +1,83 @@ +using System.Linq.Expressions; +using System.Reflection; +using System.Text.Json.Serialization; + +namespace Altinn.App.Core.Helpers; + +/// +/// Utilities for working with +/// +public static class LinqExpressionHelpers +{ + /// + /// Gets the JSON path from an expression + /// + /// The expression + /// The JSON path + public static string GetJsonPath(Expression> expression) + { + return GetJsonPath_internal(expression); + } + + /// + /// Need a private method to avoid the generic type parameter for recursion + /// + private static string GetJsonPath_internal(Expression expression) + { + ArgumentNullException.ThrowIfNull(expression); + + var path = new List(); + Expression? current = expression; + while (current is not null) + { + switch (current) + { + case MemberExpression memberExpression: + path.Add(GetJsonPropertyName(memberExpression.Member)); + current = memberExpression.Expression; + break; + case LambdaExpression lambdaExpression: + current = lambdaExpression.Body; + break; + case ParameterExpression: + // We have reached the root of the expression + current = null; + break; + + // This is a special case for accessing a list item by index + case MethodCallExpression { Method.Name: "get_Item", Arguments: [ ConstantExpression { Value: Int32 index } ], Object: MemberExpression memberExpression }: + path.Add($"{GetJsonPropertyName(memberExpression.Member)}[{index}]"); + current = memberExpression.Expression; + break; + // This is a special case for accessing a list item by index in a variable + case MethodCallExpression { Method.Name: "get_Item", Arguments: [ MemberExpression { Expression: ConstantExpression constantExpression, Member: FieldInfo fieldInfo }], Object: MemberExpression memberExpression }: + // Evaluate the constant expression to get the index + var evaluatedIndex = fieldInfo.GetValue(constantExpression.Value); + path.Add($"{GetJsonPropertyName(memberExpression.Member)}[{evaluatedIndex}]"); + current = memberExpression.Expression; + break; + // This is a special case for selecting all childern of a list using Select + case MethodCallExpression { Method.Name: "Select" } methodCallExpression: + path.Add(GetJsonPath_internal(methodCallExpression.Arguments[1])); + current = methodCallExpression.Arguments[0]; + break; + default: + throw new ArgumentException($"Invalid expression {expression}. Failed reading {current}"); + } + } + + path.Reverse(); + return string.Join(".", path); + } + + private static string GetJsonPropertyName(MemberInfo memberExpressionMember) + { + var jsonPropertyAttribute = memberExpressionMember.GetCustomAttribute(); + if (jsonPropertyAttribute is not null) + { + return jsonPropertyAttribute.Name; + } + + return memberExpressionMember.Name; + } +} \ No newline at end of file diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs index 6d972cc31..8d191936d 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs @@ -102,7 +102,7 @@ public static void RemoveHiddenData(LayoutEvaluatorState state, RowRemovalOption /// /// Return a list of for the given state and dataElementId /// - public static IEnumerable RunLayoutValidationsForRequired(LayoutEvaluatorState state, string dataElementId) + public static List RunLayoutValidationsForRequired(LayoutEvaluatorState state, string dataElementId) { var validationIssues = new List(); diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs index cfc0d0162..7f7c2183d 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs @@ -179,6 +179,14 @@ public ComponentContext GetComponentContext(string pageName, string componentId, return _dataModel.GetModelData(key, context?.RowIndices); } + /// + /// Get all of the resoved keys (including all possible indexes) from a data model key + /// + public string[] GetResolvedKeys(string key) + { + return _dataModel.GetResolvedKeys(key); + } + /// /// Set the value of a field to null. /// diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index 389d2517f..db8a88c95 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -27,7 +27,7 @@ public LayoutEvaluatorStateInitializer(IAppResources appResources, IOptions /// Initialize LayoutEvaluatorState with given Instance, data object and layoutSetId /// - public Task Init(Instance instance, object data, string? layoutSetId, string? gatewayAction = null) + public virtual Task Init(Instance instance, object data, string? layoutSetId, string? gatewayAction = null) { var layouts = _appResources.GetLayoutModel(layoutSetId); return Task.FromResult(new LayoutEvaluatorState(new DataModel(data), layouts, _frontEndSettings, instance, gatewayAction)); diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs index c10b85366..2d516a073 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs @@ -1,3 +1,4 @@ +using System.Text.Json.Serialization; using Newtonsoft.Json; using Newtonsoft.Json.Converters; @@ -12,49 +13,58 @@ public class ValidationIssue /// The seriousness of the identified issue. /// [JsonProperty(PropertyName = "severity")] - [JsonConverter(typeof(StringEnumConverter))] + [Newtonsoft.Json.JsonConverter(typeof(StringEnumConverter))] + [JsonPropertyName("severity")] + [System.Text.Json.Serialization.JsonConverter(typeof(JsonStringEnumConverter))] public ValidationIssueSeverity Severity { get; set; } /// /// The unique id of the specific element with the identified issue. /// [JsonProperty(PropertyName = "instanceId")] + [JsonPropertyName("instanceId")] public string? InstanceId { get; set; } /// /// The uniqe id of the data element of a given instance with the identified issue. /// [JsonProperty(PropertyName = "dataElementId")] + [JsonPropertyName("dataElementId")] public string? DataElementId { get; set; } /// /// A reference to a property the issue is a bout. /// [JsonProperty(PropertyName = "field")] + [JsonPropertyName("field")] public string? Field { get; set; } /// /// A system readable identification of the type of issue. /// [JsonProperty(PropertyName = "code")] + [JsonPropertyName("code")] public string? Code { get; set; } /// /// A human readable description of the issue. /// [JsonProperty(PropertyName = "description")] + [JsonPropertyName("description")] public string? Description { get; set; } /// /// The validation source of the issue eg. File, Schema, Component /// [JsonProperty(PropertyName = "source")] + [JsonPropertyName("source")] public string? Source { get; set; } /// /// The custom text key to use for the localized text in the frontend. /// [JsonProperty(PropertyName = "customTextKey")] + [JsonPropertyName("customTextKey")] public string? CustomTextKey { get; set; } } } diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs index 09bf34d14..75e56fcfa 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs @@ -1,5 +1,6 @@ using System.Net; using Altinn.App.Api.Controllers; +using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation; using Altinn.App.Core.Helpers; using Altinn.App.Core.Internal.App; @@ -21,7 +22,7 @@ public async Task ValidateInstance_returns_NotFound_when_GetInstance_returns_nul // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -45,7 +46,7 @@ public async Task ValidateInstance_throws_ValidationException_when_Instance_Proc // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -77,7 +78,7 @@ public async Task ValidateInstance_throws_ValidationException_when_Instance_Proc // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -112,7 +113,7 @@ public async Task ValidateInstance_returns_OK_with_messages() // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -143,7 +144,7 @@ public async Task ValidateInstance_returns_OK_with_messages() instanceMock.Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) .Returns(Task.FromResult(instance)); - validationMock.Setup(v => v.ValidateAndUpdateProcess(instance, "dummy")) + validationMock.Setup(v => v.ValidateInstanceAtTask(instance, "dummy")) .Returns(Task.FromResult(validationResult)); // Act @@ -161,7 +162,7 @@ public async Task ValidateInstance_returns_403_when_not_authorized() // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -186,7 +187,7 @@ public async Task ValidateInstance_returns_403_when_not_authorized() instanceMock.Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) .Returns(Task.FromResult(instance)); - validationMock.Setup(v => v.ValidateAndUpdateProcess(instance, "dummy")) + validationMock.Setup(v => v.ValidateInstanceAtTask(instance, "dummy")) .Throws(exception); // Act @@ -204,7 +205,7 @@ public async Task ValidateInstance_throws_PlatformHttpException_when_not_403() // Arrange var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); const string org = "ttd"; const string app = "app"; @@ -229,7 +230,7 @@ public async Task ValidateInstance_throws_PlatformHttpException_when_not_403() instanceMock.Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) .Returns(Task.FromResult(instance)); - validationMock.Setup(v => v.ValidateAndUpdateProcess(instance, "dummy")) + validationMock.Setup(v => v.ValidateInstanceAtTask(instance, "dummy")) .Throws(exception); // Act diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs index 2e76cc67c..16a50bd7e 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs @@ -1,5 +1,6 @@ using System.Collections; using Altinn.App.Api.Controllers; +using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation; using Altinn.App.Core.Helpers; using Altinn.App.Core.Infrastructure.Clients; @@ -236,18 +237,18 @@ public async Task TestValidateData(ValidateDataTestScenario testScenario) private static ValidateController SetupController(string app, string org, int instanceOwnerId, ValidateDataTestScenario testScenario) { - (Mock instanceMock, Mock appResourceMock, Mock validationMock) = + (Mock instanceMock, Mock appResourceMock, Mock validationMock) = SetupMocks(app, org, instanceOwnerId, testScenario); return new ValidateController(instanceMock.Object, validationMock.Object, appResourceMock.Object); } - private static (Mock, Mock, Mock) SetupMocks(string app, string org, + private static (Mock, Mock, Mock) SetupMocks(string app, string org, int instanceOwnerId, ValidateDataTestScenario testScenario) { var instanceMock = new Mock(); var appMetadataMock = new Mock(); - var validationMock = new Mock(); + var validationMock = new Mock(); if (testScenario.ReceivedInstance != null) { instanceMock.Setup(i => i.GetInstance(app, org, instanceOwnerId, testScenario.InstanceId)) @@ -263,8 +264,8 @@ private static (Mock, Mock, Mock) Se { validationMock.Setup(v => v.ValidateDataElement( testScenario.ReceivedInstance, - testScenario.ReceivedApplication.DataTypes.First(), - testScenario.ReceivedInstance.Data.First())) + testScenario.ReceivedInstance.Data.First(), + testScenario.ReceivedApplication.DataTypes.First())) .Returns(Task.FromResult>(testScenario.ReceivedValidationIssues)); } diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs new file mode 100644 index 000000000..f8f1e0a70 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs @@ -0,0 +1,217 @@ +#nullable enable + +using System.ComponentModel.DataAnnotations; +using System.Text.Json; +using System.Text.Json.Serialization; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.AspNetCore.Builder; +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Xunit; + +namespace Altinn.App.Core.Tests.Features.Validators.Default; + +public class DataAnnotationValidatorTests : IClassFixture +{ + private readonly DataAnnotationValidator _validator; + + public DataAnnotationValidatorTests(DataAnnotationsTestFixture fixture) + { + _validator = fixture.App.Services.GetRequiredKeyedService(DataAnnotationsTestFixture.DataType); + } + + private class TestClass + { + [Required] + [JsonPropertyName("requiredProperty")] + public string? RequiredProperty { get; set; } + + [StringLength(5)] + [JsonPropertyName("stringLength")] + public string? StringLengthProperty { get; set; } + + [Range(1, 10)] + [JsonPropertyName("range")] + public int RangeProperty { get; set; } + + [RegularExpression("^[0-9]*$")] + [JsonPropertyName("regularExpression")] + public string? RegularExpressionProperty { get; set; } + + [EmailAddress] + public string? EmailAddressProperty { get; set; } + + public TestClass? NestedProperty { get; set; } + } + + [Fact] + public void CanValidateDataType() + { + // Act + var result = _validator.CanValidateDataType(DataAnnotationsTestFixture.DataType); + + // Assert + Assert.True(result); + } + + [Fact] + public void ShouldRunForIncrementalValidation() + { + // Act + var result = _validator.ShouldRunForIncrementalValidation(); + + // Assert + Assert.False(result); + } + + [Fact] + public async Task ValidateFormData() + { + // Arrange + var instance = new Instance(); + var dataElement = new DataElement(); + var data = new object(); + var changedFields = new List(); + + // Prepare + + // Act + var result = await _validator.ValidateFormData(instance, dataElement, data, changedFields); + + // Assert + Assert.NotNull(result); + } + + [Fact] + public async Task Validate_ValidFormData_NoErrors() + { + // Arrange + var instance = new Instance(); + var dataElement = new DataElement(); + var data = new TestClass() + { + RangeProperty = 3, + RequiredProperty = "test", + EmailAddressProperty = "test@altinn.no", + RegularExpressionProperty = "12345", + StringLengthProperty = "12345", + NestedProperty = new TestClass() + { + RangeProperty = 3, + RequiredProperty = "test", + EmailAddressProperty = "test@altinn.no", + RegularExpressionProperty = "12345", + StringLengthProperty = "12345", + } + }; + var changedFields = new List(); + + // Act + var result = await _validator.ValidateFormData(instance, dataElement, data, changedFields); + + // Assert + Assert.NotNull(result); + Assert.Empty(result); + } + + [Fact] + public async Task ValidateFormData_RequiredProperty() + { + // Arrange + var instance = new Instance(); + var dataElement = new DataElement(); + var data = new TestClass() + { + NestedProperty = new(), + }; + var changedFields = new List(); + + // Act + var result = await _validator.ValidateFormData(instance, dataElement, data, changedFields); + + // Assert + result.Should().NotBeNull(); + result.Should().BeEquivalentTo(JsonSerializer.Deserialize>(""" + [ + { + "severity": "Error", + "instanceId": null, + "dataElementId": null, + "field": "range", + "code": "The field RangeProperty must be between 1 and 10.", + "description": "The field RangeProperty must be between 1 and 10.", + "source": "ModelState", + "customTextKey": null + }, + { + "severity": "Error", + "instanceId": null, + "dataElementId": null, + "field": "requiredProperty", + "code": "The RequiredProperty field is required.", + "description": "The RequiredProperty field is required.", + "source": "ModelState", + "customTextKey": null + }, + { + "severity": "Error", + "instanceId": null, + "dataElementId": null, + "field": "NestedProperty.range", + "code": "The field RangeProperty must be between 1 and 10.", + "description": "The field RangeProperty must be between 1 and 10.", + "source": "ModelState", + "customTextKey": null + }, + { + "severity": "Error", + "instanceId": null, + "dataElementId": null, + "field": "NestedProperty.requiredProperty", + "code": "The RequiredProperty field is required.", + "description": "The RequiredProperty field is required.", + "source": "ModelState", + "customTextKey": null + } + ] + """)); + } +} + +/// +/// System.ComponentModel.DataAnnotations does not provide an easy way to run validations recursively in a unit test, +/// so we need to instantiate a WebApplication to get the IObjectModelValidator. +/// +/// A full WebApplicationFactory seemed a little overkill, so we just use a WebApplicationBuilder. +/// +public class DataAnnotationsTestFixture : IAsyncDisposable +{ + public const string DataType = "test"; + + private readonly DefaultHttpContext _httpContext = new DefaultHttpContext(); + + private readonly Mock _httpContextAccessor = new Mock(); + + public WebApplication App { get; } + + public DataAnnotationsTestFixture() + { + WebApplicationBuilder builder = WebApplication.CreateBuilder(); + builder.Services.AddMvc(); + builder.Services.AddKeyedTransient(DataType); + _httpContextAccessor.Setup(a => a.HttpContext).Returns(_httpContext); + builder.Services.AddSingleton(_httpContextAccessor.Object); + builder.Services.Configure(builder.Configuration.GetSection("GeneralSettings")); + App = builder.Build(); + } + + public ValueTask DisposeAsync() + { + return App.DisposeAsync(); + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs new file mode 100644 index 000000000..23a48df5a --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs @@ -0,0 +1,124 @@ +using System.Reflection; +using System.Text.Json; +using System.Text.Json.Nodes; +using System.Text.Json.Serialization; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.Expressions; +using Altinn.App.Core.Models.Layout; +using Altinn.App.Core.Models.Validation; +using Altinn.App.Core.Tests.Helpers; +using Altinn.App.Core.Tests.LayoutExpressions; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; +using Moq; +using Xunit; +using Xunit.Sdk; + +namespace Altinn.App.Core.Tests.Features.Validators.Default; + +public class ExpressionValidatorTests +{ + private const string DataType = "default"; + private readonly ExpressionValidator _validator; + private readonly Mock> _logger = new(); + private readonly Mock _appResources = new(MockBehavior.Strict); + private readonly IOptions _frontendSettings = Options.Create(new FrontEndSettings()); + private readonly Mock _layoutInitializer; + + public ExpressionValidatorTests() + { + _layoutInitializer = new(MockBehavior.Strict, _appResources.Object, _frontendSettings) { CallBase = false }; + _validator = + new ExpressionValidator(DataType, _logger.Object, _appResources.Object, _layoutInitializer.Object); + } + + [Theory] + [ExpressionTest] + public async Task RunExpressionValidationTest(ExpressionValidationTestModel testCase) + { + var instance = new Instance(); + var dataElement = new DataElement(); + + var dataModel = new JsonDataModel(testCase.FormData); + + var evaluatorState = new LayoutEvaluatorState(dataModel, testCase.Layouts, _frontendSettings.Value, instance); + _layoutInitializer + .Setup(init => init.Init(It.Is(i => i == instance), It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(evaluatorState); + _appResources + .Setup(ar => ar.GetValidationConfiguration(null)) + .Returns(JsonSerializer.Serialize(testCase.ValidationConfig)); + + LayoutEvaluator.RemoveHiddenData(evaluatorState, RowRemovalOption.SetToNull); + var validationIssues = await _validator.ValidateFormData(instance, dataElement, null!); + + var result = validationIssues.Select(i => new + { + Message = i.CustomTextKey, + Severity = i.Severity, + Field = i.Field, + }); + + var expected = testCase.Expects.Select(e => new + { + Message = e.Message, + Severity = e.Severity, + Field = e.Field, + }); + + result.Should().BeEquivalentTo(expected); + } +} + +public class ExpressionTestAttribute : DataAttribute +{ + public override IEnumerable GetData(MethodInfo methodInfo) + { + var files = Directory.GetFiles(Path.Join("Features", "Validators", "shared-expression-validation-tests")); + + foreach (var file in files) + { + var data = File.ReadAllText(file); + ExpressionValidationTestModel testCase = JsonSerializer.Deserialize( + data, + new JsonSerializerOptions + { + ReadCommentHandling = JsonCommentHandling.Skip, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + })!; + yield return new object[] { testCase }; + } + } +} + +public class ExpressionValidationTestModel +{ + public string Name { get; set; } + + public ExpectedObject[] Expects { get; set; } + + public JsonElement ValidationConfig { get; set; } + + public JsonObject FormData { get; set; } + + [JsonConverter(typeof(LayoutModelConverterFromObject))] + public LayoutModel Layouts { get; set; } + + public class ExpectedObject + { + public string Message { get; set; } + + [JsonConverter(typeof(FrontendSeverityConverter))] + public ValidationIssueSeverity Severity { get; set; } + + public string Field { get; set; } + + public string ComponentId { get; set; } + } +} diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs new file mode 100644 index 000000000..402e92015 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -0,0 +1,141 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.AspNetCore.Mvc.ModelBinding; +using Microsoft.Extensions.Options; +using Moq; +using Xunit; + +namespace Altinn.App.Core.Tests.Features.Validators.Default +{ + public class LegacyIValidationFormDataTests + { + private readonly LegacyIValidationFormDataValidator _validator; + private readonly Mock _instanceValidator = new(); + + public LegacyIValidationFormDataTests() + { + var generalSettings = new GeneralSettings(); + _validator = + new LegacyIValidationFormDataValidator(_instanceValidator.Object, Options.Create(generalSettings)); + } + + [Fact] + public async Task ValidateFormData_NoErrors() + { + // Arrange + var data = new object(); + var changedFields = new List(); + + var validator = new LegacyIValidationFormDataValidator(null, Options.Create(new GeneralSettings())); + validator.ShouldRunForIncrementalValidation(changedFields).Should().BeTrue(); + + // Act + var result = await validator.ValidateFormData(new Instance(), new DataElement(), data, changedFields); + + // Assert + Assert.Empty(result); + } + + [Fact] + public async Task ValidateFormData_WithErrors() + { + // Arrange + var data = new object(); + var changedFields = new List(); + + _instanceValidator + .Setup(iv => iv.ValidateData(It.IsAny(), It.IsAny())) + .Callback((object _, ModelStateDictionary modelState) => + { + modelState.AddModelError("test", "test"); + modelState.AddModelError("ddd", "*FIXED*test"); + }); + + // Act + var result = await _validator.ValidateFormData(new Instance(), new DataElement(), data, changedFields); + + // Assert + result.Should().BeEquivalentTo( + JsonSerializer.Deserialize>(""" + [ + { + "severity": "Fixed", + "instanceId": null, + "dataElementId": null, + "field": "ddd", + "code": "test", + "description": "test", + "source": "Custom", + "customTextKey": null + }, + { + "severity": "Error", + "instanceId": null, + "dataElementId": null, + "field": "test", + "code": "test", + "description": "test", + "source": "Custom", + "customTextKey": null + } + ] + """)); + } + + private class TestModel + { + [JsonPropertyName("test")] + public string Test { get; set; } + + public int IntegerWithout { get; set; } + + [JsonPropertyName("child")] + public TestModel Child { get; set; } + + [JsonPropertyName("children")] + public List TestList { get; set; } + } + + [Theory] + [InlineData("test", "test", "test with small case")] + [InlineData("Test", "test", "test with capital case gets rewritten")] + [InlineData("NotModelMatch", "NotModelMatch", "Error that does not mach model is kept as is")] + [InlineData("Child.TestList[2].child", "child.children[2].child", "TestList is renamed to children because of JsonPropertyName")] + [InlineData("test.children.child", "test.children.child", "valid JsonPropertyName based path is kept as is")] + public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string field, string errorMessage) + { + // Arrange + var data = new TestModel(); + var changedFields = new List(); + + _instanceValidator + .Setup(iv => iv.ValidateData(It.IsAny(), It.IsAny())) + .Callback((object _, ModelStateDictionary modelState) => + { + modelState.AddModelError(errorKey, errorMessage); + modelState.AddModelError(errorKey, "*FIXED*" + errorMessage + " Fixed"); + }); + + // Act + var result = await _validator.ValidateFormData(new Instance(), new DataElement(), data, changedFields); + + // Assert + result.Should().HaveCount(2); + var errorIssue = result.Should().ContainSingle(i => i.Severity == ValidationIssueSeverity.Error).Which; + errorIssue.Field.Should().Be(field); + errorIssue.Severity.Should().Be(ValidationIssueSeverity.Error); + errorIssue.Description.Should().Be(errorMessage); + + var fixedIssue = result.Should().ContainSingle(i => i.Severity == ValidationIssueSeverity.Fixed).Which; + fixedIssue.Field.Should().Be(field); + fixedIssue.Severity.Should().Be(ValidationIssueSeverity.Fixed); + fixedIssue.Description.Should().Be(errorMessage + " Fixed"); + } + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs index 19d23481d..c54a28792 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs @@ -3,6 +3,7 @@ using System.Text.Json.Nodes; using System.Text.Json.Serialization; using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Features.Validation.Default; using Altinn.App.Core.Helpers; using Altinn.App.Core.Internal.Expressions; using Altinn.App.Core.Models.Layout; @@ -23,12 +24,12 @@ public class ExpressionValidationTests [ExpressionTest] public void RunExpressionValidationTest(ExpressionValidationTestModel testCase) { - var logger = Mock.Of>(); + var logger = Mock.Of>(); var dataModel = new JsonDataModel(testCase.FormData); var evaluatorState = new LayoutEvaluatorState(dataModel, testCase.Layouts, new(), new()); LayoutEvaluator.RemoveHiddenData(evaluatorState, RowRemovalOption.SetToNull); - var validationIssues = ExpressionValidator.Validate(testCase.ValidationConfig, dataModel, evaluatorState, logger).ToArray(); + var validationIssues = ExpressionValidator.Validate(testCase.ValidationConfig, evaluatorState, logger).ToArray(); var result = validationIssues.Select(i => new { diff --git a/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs new file mode 100644 index 000000000..96f234de2 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs @@ -0,0 +1,83 @@ +#nullable enable +using System.Diagnostics.CodeAnalysis; +using System.Linq.Expressions; +using System.Text.Json.Serialization; +using Altinn.App.Core.Features.Validation; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Xunit; + +namespace Altinn.App.Core.Tests.Features.Validators; + +public class GenericValidatorTests +{ + private class MyModel + { + [JsonPropertyName("name")] + public string? Name { get; set; } + + [JsonPropertyName("age")] + public int? Age { get; set; } + + [JsonPropertyName("children")] + public List? Children { get; set; } + } + + private class TestValidator : GenericFormDataValidator + { + public TestValidator() : base("MyType") + { + } + + // Custom method to make the protected RunFor possible to call from the test + public void RunForExternal(Expression> selector) + { + RunFor(selector); + } + + protected override Task ValidateFormData(Instance instance, DataElement dataElement, MyModel data, List? changedFields = null) + { + throw new NotImplementedException(); + } + } + + [Fact] + public void TestShouldRun() + { + var testValidator = new TestValidator(); + testValidator.RunForExternal(m => m.Name); + testValidator.ShouldRunForIncrementalValidation().Should().BeTrue(); + testValidator.ShouldRunForIncrementalValidation(new List() { "name" }).Should().BeTrue(); + testValidator.ShouldRunForIncrementalValidation(new List() { "age" }).Should().BeFalse(); + } + + [Theory] + [InlineData("name", false)] + [InlineData("age", false)] + [InlineData("children", true)] + [InlineData("children[0]", true)] + [InlineData("children[0].age", false)] + [InlineData("children[2]", false)] + public void TestShouldRunWithIndexedRow(string changedField, bool shouldBe) + { + var testValidator = new TestValidator(); + testValidator.RunForExternal(m => m.Children![0].Name); + testValidator.ShouldRunForIncrementalValidation(new List() { changedField }).Should().Be(shouldBe); + } + + [Theory] + [InlineData("name", false)] + [InlineData("age", false)] + [InlineData("children", true)] + + // [InlineData("children[0]", true)] //TODO:Fix + [InlineData("children[0].age", false)] + [InlineData("children[2]", false)] + public void TestShouldRunWithSelectAllRow(string changedField, bool shouldBe) + { + var testValidator = new TestValidator(); + testValidator.RunForExternal(m => m.Children!.Select(c => c.Name)); + testValidator.ShouldRunForIncrementalValidation(new List() { changedField }).Should().Be(shouldBe); + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationAppSITests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs similarity index 51% rename from test/Altinn.App.Core.Tests/Features/Validators/ValidationAppSITests.cs rename to test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs index 83b38a9dc..4dae865de 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationAppSITests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs @@ -2,17 +2,21 @@ using System.Text.Json.Serialization; using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Features.Validation.Helpers; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Expressions; using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Enums; using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; +using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -20,21 +24,54 @@ namespace Altinn.App.Core.Tests.Features.Validators; -public class ValidationAppSITests +public class ValidationServiceOldTests { + private readonly Mock> _loggerMock = new(); + private readonly Mock _dataClientMock = new(); + private readonly Mock _appModelMock = new(); + private readonly Mock _appMetadataMock = new(); + private readonly ServiceCollection _serviceCollection = new(); + + private ApplicationMetadata _applicationMetadata = new("tdd/test") + { + DataTypes = new List() + { + new DataType() + { + Id = "test", + TaskId = "Task_1", + EnableFileScan = false, + ValidationErrorOnPendingFileScan = false, + } + } + }; + + public ValidationServiceOldTests() + { + _serviceCollection.AddSingleton(_loggerMock.Object); + _serviceCollection.AddSingleton(_dataClientMock.Object); + _serviceCollection.AddSingleton(); + _serviceCollection.AddSingleton(_appModelMock.Object); + _serviceCollection.AddSingleton(_appMetadataMock.Object); + _serviceCollection.AddSingleton(); + _appMetadataMock.Setup(am => am.GetApplicationMetadata()).ReturnsAsync(_applicationMetadata); + } + [Fact] public async Task FileScanEnabled_VirusFound_ValidationShouldFail() { - ValidationAppSI validationAppSI = ConfigureMockServicesForValidation(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + IValidationService validationService = serviceProvider.GetRequiredService(); var instance = new Instance(); var dataType = new DataType() { EnableFileScan = true }; var dataElement = new DataElement() { + DataType = "test", FileScanResult = FileScanResult.Infected }; - List validationIssues = await validationAppSI.ValidateDataElement(instance, dataType, dataElement); + List validationIssues = await validationService.ValidateDataElement(instance, dataElement, dataType); validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileInfected").Should().NotBeNull(); } @@ -42,16 +79,21 @@ public async Task FileScanEnabled_VirusFound_ValidationShouldFail() [Fact] public async Task FileScanEnabled_PendingScanNotEnabled_ValidationShouldNotFail() { - ValidationAppSI validationAppSI = ConfigureMockServicesForValidation(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + IValidationService validationService = serviceProvider.GetRequiredService(); - var instance = new Instance(); - var dataType = new DataType() { EnableFileScan = true }; + var dataType = new DataType() + { Id = "test", TaskId = "Task_1", AppLogic = null, EnableFileScan = true }; + var instance = new Instance() + { + }; var dataElement = new DataElement() { - FileScanResult = FileScanResult.Pending + DataType = "test", + FileScanResult = FileScanResult.Pending, }; - List validationIssues = await validationAppSI.ValidateDataElement(instance, dataType, dataElement); + List validationIssues = await validationService.ValidateDataElement(instance, dataElement, dataType); validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileScanPending").Should().BeNull(); } @@ -59,16 +101,18 @@ public async Task FileScanEnabled_PendingScanNotEnabled_ValidationShouldNotFail( [Fact] public async Task FileScanEnabled_PendingScanEnabled_ValidationShouldNotFail() { - ValidationAppSI validationAppSI = ConfigureMockServicesForValidation(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + IValidationService validationService = serviceProvider.GetRequiredService(); var instance = new Instance(); var dataType = new DataType() { EnableFileScan = true, ValidationErrorOnPendingFileScan = true }; var dataElement = new DataElement() { + DataType = "test", FileScanResult = FileScanResult.Pending }; - List validationIssues = await validationAppSI.ValidateDataElement(instance, dataType, dataElement); + List validationIssues = await validationService.ValidateDataElement(instance, dataElement, dataType); validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileScanPending").Should().NotBeNull(); } @@ -76,153 +120,124 @@ public async Task FileScanEnabled_PendingScanEnabled_ValidationShouldNotFail() [Fact] public async Task FileScanEnabled_Clean_ValidationShouldNotFail() { - ValidationAppSI validationAppSI = ConfigureMockServicesForValidation(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + IValidationService validationService = serviceProvider.GetRequiredService(); var instance = new Instance(); var dataType = new DataType() { EnableFileScan = true, ValidationErrorOnPendingFileScan = true }; var dataElement = new DataElement() { - FileScanResult = FileScanResult.Clean + DataType = "test", + FileScanResult = FileScanResult.Clean, }; - List validationIssues = await validationAppSI.ValidateDataElement(instance, dataType, dataElement); + List validationIssues = await validationService.ValidateDataElement(instance, dataElement, dataType); validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileInfected").Should().BeNull(); validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileScanPending").Should().BeNull(); } - private static ValidationAppSI ConfigureMockServicesForValidation() - { - Mock> loggerMock = new(); - var dataMock = new Mock(); - var instanceMock = new Mock(); - var instanceValidator = new Mock(); - var appModelMock = new Mock(); - var appResourcesMock = new Mock(); - var appMetadataMock = new Mock(); - var objectModelValidatorMock = new Mock(); - var layoutEvaluatorStateInitializer = new LayoutEvaluatorStateInitializer(appResourcesMock.Object, Microsoft.Extensions.Options.Options.Create(new Configuration.FrontEndSettings())); - var httpContextAccessorMock = new Mock(); - var generalSettings = Microsoft.Extensions.Options.Options.Create(new Configuration.GeneralSettings()); - var appSettings = Microsoft.Extensions.Options.Options.Create(new Configuration.AppSettings()); - - var validationAppSI = new ValidationAppSI( - loggerMock.Object, - dataMock.Object, - instanceMock.Object, - instanceValidator.Object, - appModelMock.Object, - appResourcesMock.Object, - appMetadataMock.Object, - objectModelValidatorMock.Object, - layoutEvaluatorStateInitializer, - httpContextAccessorMock.Object, - generalSettings, - appSettings); - return validationAppSI; - } - [Fact] public void ModelKeyToField_NullInputWithoutType_ReturnsNull() { - ValidationAppSI.ModelKeyToField(null, null!).Should().BeNull(); + ModelStateHelpers.ModelKeyToField(null, null!).Should().BeNull(); } [Fact] public void ModelKeyToField_StringInputWithoutType_ReturnsSameString() { - ValidationAppSI.ModelKeyToField("null", null!).Should().Be("null"); + ModelStateHelpers.ModelKeyToField("null", null!).Should().Be("null"); } [Fact] public void ModelKeyToField_NullInput_ReturnsNull() { - ValidationAppSI.ModelKeyToField(null, typeof(TestModel)).Should().BeNull(); + ModelStateHelpers.ModelKeyToField(null, typeof(TestModel)).Should().BeNull(); } [Fact] public void ModelKeyToField_StringInput_ReturnsSameString() { - ValidationAppSI.ModelKeyToField("null", typeof(TestModel)).Should().Be("null"); + ModelStateHelpers.ModelKeyToField("null", typeof(TestModel)).Should().Be("null"); } [Fact] public void ModelKeyToField_StringInputWithAttr_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("FirstLevelProp", typeof(TestModel)).Should().Be("level1"); + ModelStateHelpers.ModelKeyToField("FirstLevelProp", typeof(TestModel)).Should().Be("level1"); } [Fact] public void ModelKeyToField_SubModel_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModel.DecimalNumber", typeof(TestModel)).Should().Be("sub.decimal"); + ModelStateHelpers.ModelKeyToField("SubTestModel.DecimalNumber", typeof(TestModel)).Should().Be("sub.decimal"); } [Fact] public void ModelKeyToField_SubModelNullable_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModel.StringNullable", typeof(TestModel)).Should().Be("sub.nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModel.StringNullable", typeof(TestModel)).Should().Be("sub.nullableString"); } [Fact] public void ModelKeyToField_SubModelWithSubmodel_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModel.StringNullable", typeof(TestModel)).Should().Be("sub.nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModel.StringNullable", typeof(TestModel)).Should().Be("sub.nullableString"); } [Fact] public void ModelKeyToField_SubModelNull_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModelNull.DecimalNumber", typeof(TestModel)).Should().Be("subnull.decimal"); + ModelStateHelpers.ModelKeyToField("SubTestModelNull.DecimalNumber", typeof(TestModel)).Should().Be("subnull.decimal"); } [Fact] public void ModelKeyToField_SubModelNullNullable_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModelNull.StringNullable", typeof(TestModel)).Should().Be("subnull.nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModelNull.StringNullable", typeof(TestModel)).Should().Be("subnull.nullableString"); } [Fact] public void ModelKeyToField_SubModelNullWithSubmodel_ReturnsMappedString() { - ValidationAppSI.ModelKeyToField("SubTestModelNull.StringNullable", typeof(TestModel)).Should().Be("subnull.nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModelNull.StringNullable", typeof(TestModel)).Should().Be("subnull.nullableString"); } // Test lists [Fact] public void ModelKeyToField_List_IgnoresMissingIndex() { - ValidationAppSI.ModelKeyToField("SubTestModelList.StringNullable", typeof(TestModel)).Should().Be("subList.nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModelList.StringNullable", typeof(TestModel)).Should().Be("subList.nullableString"); } [Fact] public void ModelKeyToField_List_ProxiesIndex() { - ValidationAppSI.ModelKeyToField("SubTestModelList[123].StringNullable", typeof(TestModel)).Should().Be("subList[123].nullableString"); + ModelStateHelpers.ModelKeyToField("SubTestModelList[123].StringNullable", typeof(TestModel)).Should().Be("subList[123].nullableString"); } [Fact] public void ModelKeyToField_ListOfList_ProxiesIndex() { - ValidationAppSI.ModelKeyToField("SubTestModelList[123].ListOfDecimal[5]", typeof(TestModel)).Should().Be("subList[123].decimalList[5]"); + ModelStateHelpers.ModelKeyToField("SubTestModelList[123].ListOfDecimal[5]", typeof(TestModel)).Should().Be("subList[123].decimalList[5]"); } [Fact] public void ModelKeyToField_ListOfList_IgnoresMissing() { - ValidationAppSI.ModelKeyToField("SubTestModelList[123].ListOfDecimal", typeof(TestModel)).Should().Be("subList[123].decimalList"); + ModelStateHelpers.ModelKeyToField("SubTestModelList[123].ListOfDecimal", typeof(TestModel)).Should().Be("subList[123].decimalList"); } [Fact] public void ModelKeyToField_ListOfListNullable_IgnoresMissing() { - ValidationAppSI.ModelKeyToField("SubTestModelList[123].ListOfNullableDecimal", typeof(TestModel)).Should().Be("subList[123].nullableDecimalList"); + ModelStateHelpers.ModelKeyToField("SubTestModelList[123].ListOfNullableDecimal", typeof(TestModel)).Should().Be("subList[123].nullableDecimalList"); } [Fact] public void ModelKeyToField_ListOfListOfListNullable_IgnoresMissingButPropagatesOthers() { - ValidationAppSI.ModelKeyToField("SubTestModelList[123].SubTestModelList.ListOfNullableDecimal[123456]", typeof(TestModel)).Should().Be("subList[123].subList.nullableDecimalList[123456]"); + ModelStateHelpers.ModelKeyToField("SubTestModelList[123].SubTestModelList.ListOfNullableDecimal[123456]", typeof(TestModel)).Should().Be("subList[123].subList.nullableDecimalList[123456]"); } public class TestModel @@ -257,4 +272,4 @@ public class SubTestModel [JsonPropertyName("subList")] public List SubTestModelList { get; set; } = default!; } -} \ No newline at end of file +} diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs new file mode 100644 index 000000000..37afdd1b2 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -0,0 +1,115 @@ +#nullable enable +using System.Text.Json.Serialization; +using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.AppModel; +using Altinn.App.Core.Internal.Data; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Altinn.App.Core.Tests.Features.Validators; + +public class ValidationServiceTests +{ + private class MyModel + { + [JsonPropertyName("name")] + public string? Name { get; set; } + + [JsonPropertyName("age")] + public int? Age { get; set; } + } + + private static readonly DataElement DefaultDataElement = new() + { + DataType = "MyType", + }; + + private readonly Mock> _loggerMock = new(); + private readonly Mock _dataClientMock = new(); + private readonly Mock _appModelMock = new(); + private readonly Mock _appMetadataMock = new(); + private readonly ServiceCollection _serviceCollection = new(); + + public ValidationServiceTests() + { + _serviceCollection.AddSingleton(_loggerMock.Object); + _serviceCollection.AddSingleton(_dataClientMock.Object); + _serviceCollection.AddSingleton(); + _serviceCollection.AddSingleton(_appModelMock.Object); + _serviceCollection.AddSingleton(_appMetadataMock.Object); + } + + private class MyNameValidator : GenericFormDataValidator + { + public MyNameValidator() : base(DefaultDataElement.DataType) + { + RunFor(m => m.Name); + } + + protected override async Task ValidateFormData(Instance instance, DataElement dataElement, MyModel data, List? changedFields = null) + { + if (data.Name != "Ola") + { + CreateValidationIssue(m => m.Name, "NameNotOla"); + } + } + } + + [Fact] + public async Task ValidateFormData_WithNoValidators_ReturnsNoErrors() + { + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + + var validatorService = serviceProvider.GetRequiredService(); + var data = new MyModel { Name = "Ola" }; + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + result.Should().BeEmpty(); + } + + [Fact] + public async Task ValidateFormData_WithMyNameValidator_ReturnsNoErrorsWhenNameIsOla() + { + _serviceCollection.AddSingleton(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + + var validatorService = serviceProvider.GetRequiredService(); + var data = new MyModel { Name = "Ola" }; + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + result.Should().ContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator-MyType").WhoseValue.Should().HaveCount(0); + result.Should().HaveCount(1); + } + + [Fact] + public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKari() + { + _serviceCollection.AddSingleton(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + + var validatorService = serviceProvider.GetRequiredService(); + var data = new MyModel { Name = "Kari" }; + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + result.Should().ContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator-MyType").WhoseValue.Should().ContainSingle().Which.CustomTextKey.Should().Be("NameNotOla"); + result.Should().HaveCount(1); + } + + [Fact] + public async Task ValidateFormData_WithMyNameValidator_ReturnsNoErrorsWhenOnlyAgeIsSoupposedlyChanged() + { + _serviceCollection.AddSingleton(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); + + var validatorService = serviceProvider.GetRequiredService(); + var data = new MyModel { Name = "Kari" }; + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data, new List { "age" }); + result.Should() + .NotContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator"); + result.Should().HaveCount(0); + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Helpers/LinqExpressionHelpersTests.cs b/test/Altinn.App.Core.Tests/Helpers/LinqExpressionHelpersTests.cs new file mode 100644 index 000000000..cd2152ff2 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Helpers/LinqExpressionHelpersTests.cs @@ -0,0 +1,68 @@ +#nullable enable +using System.Text.Json.Serialization; +using Altinn.App.Core.Helpers; +using FluentAssertions; +using Xunit; + +namespace Altinn.App.Core.Tests.Helpers; + +public class LinqExpressionHelpersTests +{ + public class MyModel + { + [JsonPropertyName("name")] + public string? Name { get; set; } + + [JsonPropertyName("age")] + public int? Age { get; set; } + + public List? Children { get; set; } + } + + [Fact] + public void GetJsonPath_OneLevelDeep() + { + var propertyName = LinqExpressionHelpers.GetJsonPath(m => m.Name); + propertyName.Should().Be("name"); + } + + [Fact] + public void GetJsonPath_TwoLevelsDeep() + { + var propertyName = LinqExpressionHelpers.GetJsonPath(m => m.Children![0].Age); + propertyName.Should().Be("Children[0].age"); + } + + [Fact()] + public void GetJsonPath_TwoLevelsDeepUsingFirst() + { + var propertyName = LinqExpressionHelpers.GetJsonPath>(m => m.Children!.Select(c => c.Age)); + propertyName.Should().Be("Children.age"); + } + + [Fact] + public void GetJsonPath_ManyLevelsDeep() + { + var propertyName = LinqExpressionHelpers.GetJsonPath>(m => m.Children![0].Children![2].Children!.Select(c => c.Children![44].Age)); + propertyName.Should().Be("Children[0].Children[2].Children.Children[44].age"); + } + + [Fact] + public void GetJsonPath_IndexInVariable() + { + var index = 123; + var propertyName = LinqExpressionHelpers.GetJsonPath(m => m.Children![index].Age); + propertyName.Should().Be("Children[123].age"); + } + + [Fact] + public void GetJsonPath_IndexInVariableLoop() + { + for (var i = 0; i < 10; i++) + { + var index = i; // Needed to avoid "Access to modified closure" error + var propertyName = LinqExpressionHelpers.GetJsonPath(m => m.Children![index].Age); + propertyName.Should().Be($"Children[{index}].age"); + } + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Implementation/NullInstanceValidatorTests.cs b/test/Altinn.App.Core.Tests/Implementation/NullInstanceValidatorTests.cs deleted file mode 100644 index a421a8281..000000000 --- a/test/Altinn.App.Core.Tests/Implementation/NullInstanceValidatorTests.cs +++ /dev/null @@ -1,38 +0,0 @@ -using Altinn.App.Core.Features.Validation; -using Altinn.App.PlatformServices.Tests.Implementation.TestResources; -using Altinn.Platform.Storage.Interface.Models; -using Microsoft.AspNetCore.Mvc.ModelBinding; -using Xunit; - -namespace Altinn.App.PlatformServices.Tests.Implementation; - -public class NullInstanceValidatorTests -{ - [Fact] - public async void NullInstanceValidator_ValidateData_does_not_add_to_ValidationResults() - { - // Arrange - var instanceValidator = new NullInstanceValidator(); - ModelStateDictionary validationResults = new ModelStateDictionary(); - - // Act - await instanceValidator.ValidateData(new DummyModel(), validationResults); - - // Assert - Assert.Empty(validationResults); - } - - [Fact] - public async void NullInstanceValidator_ValidateTask_does_not_add_to_ValidationResults() - { - // Arrange - var instanceValidator = new NullInstanceValidator(); - ModelStateDictionary validationResults = new ModelStateDictionary(); - - // Act - await instanceValidator.ValidateTask(new Instance(), "task0", validationResults); - - // Assert - Assert.Empty(validationResults); - } -} \ No newline at end of file From 088c12529b0c8f93b1b7603c32a549c5a966c39e Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Wed, 3 Jan 2024 13:32:12 +0100 Subject: [PATCH 2/7] Fix sonar cloud issues --- .../Default/DefaultDataElementValidation.cs | 6 +- .../Default/DefaultTaskValidator.cs | 4 ++ .../Validation/Default/ExpressionValidator.cs | 56 +++++++++---------- .../LegacyIValidationFormDataValidator.cs | 6 +- .../Validation/Default/RequiredValidator.cs | 6 ++ .../Validation/GenericFormDataValidator.cs | 14 ++++- .../Validation/Helpers/ModelStateHelpers.cs | 21 ++++++- .../Features/Validation/ValidationService.cs | 7 --- .../Default/LegacyIValidationFormDataTests.cs | 2 +- .../Validators/ExpressionValidationTests.cs | 28 +++++----- .../Validators/ValidationServiceOldTests.cs | 2 +- 11 files changed, 93 insertions(+), 59 deletions(-) diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs index 1cb99af02..2c4febe47 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs @@ -9,13 +9,15 @@ namespace Altinn.App.Core.Features.Validation.Default; /// public class DefaultDataElementValidation : IDataElementValidator { - public string DataType { get; } + /// + public string DataType { get; } = "*"; /// /// Runs on all data elements to validate metadata and file scan results. /// public bool CanValidateDataType(DataType dataType) => true; + /// public Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType) { var issues = new List(); @@ -35,7 +37,7 @@ public Task> ValidateDataElement(Instance instance, DataEl var contentTypeWithoutEncoding = dataElement.ContentType.Split(";")[0]; if (dataType.AllowedContentTypes != null && dataType.AllowedContentTypes.Count > 0 && - dataType.AllowedContentTypes.All(ct => + dataType.AllowedContentTypes.TrueForAll(ct => !ct.Equals(contentTypeWithoutEncoding, StringComparison.OrdinalIgnoreCase))) { issues.Add( new ValidationIssue diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs index 5916f2b62..9293062a8 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs @@ -13,6 +13,9 @@ public class DefaultTaskValidator : ITaskValidator { private readonly IAppMetadata _appMetadata; + /// + /// Initializes a new instance of the class. + /// public DefaultTaskValidator([ServiceKey] string taskId, IAppMetadata appMetadata) { TaskId = taskId; @@ -22,6 +25,7 @@ public DefaultTaskValidator([ServiceKey] string taskId, IAppMetadata appMetadata /// public string TaskId { get; } + /// public async Task> ValidateTask(Instance instance) { var messages = new List(); diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs index 104a236bb..0d454e2d0 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -19,8 +19,14 @@ public class ExpressionValidator : IFormDataValidator private readonly IAppResources _appResourceService; private readonly LayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; + private static readonly JsonSerializerOptions _jsonOptions = new() + { + ReadCommentHandling = JsonCommentHandling.Skip, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + }; + /// - /// Constructor + /// Constructor for the expression validator /// public ExpressionValidator([ServiceKey] string dataType, ILogger logger, IAppResources appResourceService, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer) { @@ -37,6 +43,7 @@ public ExpressionValidator([ServiceKey] string dataType, ILogger public bool ShouldRunForIncrementalValidation(List? changedFields = null) => true; + /// public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) { var rawValidationConfig = _appResourceService.GetValidationConfiguration(dataElement.DataType); @@ -52,8 +59,7 @@ public async Task> ValidateFormData(Instance instance, Dat } - - public static List Validate(JsonElement validationConfig, LayoutEvaluatorState evaluatorState, ILogger logger) + internal static List Validate(JsonElement validationConfig, LayoutEvaluatorState evaluatorState, ILogger logger) { var validationIssues = new List(); var expressionValidations = ParseExpressionValidationConfig(validationConfig, logger); @@ -108,14 +114,11 @@ public static List Validate(JsonElement validationConfig, Layou private static RawExpressionValidation? ResolveValidationDefinition(string name, JsonElement definition, Dictionary resolvedDefinitions, ILogger logger) { var resolvedDefinition = new RawExpressionValidation(); - var rawDefinition = definition.Deserialize(new JsonSerializerOptions - { - ReadCommentHandling = JsonCommentHandling.Skip, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }); + + var rawDefinition = definition.Deserialize(_jsonOptions); if (rawDefinition == null) { - logger.LogError($"Validation definition {name} could not be parsed"); + logger.LogError("Validation definition {name} could not be parsed", name); return null; } if (rawDefinition.Ref != null) @@ -123,7 +126,7 @@ public static List Validate(JsonElement validationConfig, Layou var reference = resolvedDefinitions.GetValueOrDefault(rawDefinition.Ref); if (reference == null) { - logger.LogError($"Could not resolve reference {rawDefinition.Ref} for validation {name}"); + logger.LogError("Could not resolve reference {rawDefinitionRef} for validation {name}", rawDefinition.Ref, name); return null; } @@ -149,13 +152,13 @@ public static List Validate(JsonElement validationConfig, Layou if (resolvedDefinition.Message == null) { - logger.LogError($"Validation {name} is missing message"); + logger.LogError("Validation {name} is missing message", name); return null; } if (resolvedDefinition.Condition == null) { - logger.LogError($"Validation {name} is missing condition"); + logger.LogError("Validation {name} is missing condition", name); return null; } @@ -172,13 +175,13 @@ public static List Validate(JsonElement validationConfig, Layou var stringReference = definition.GetString(); if (stringReference == null) { - logger.LogError($"Could not resolve null reference for validation for field {field}"); + logger.LogError("Could not resolve null reference for validation for field {field}", field); return null; } var reference = resolvedDefinitions.GetValueOrDefault(stringReference); if (reference == null) { - logger.LogError($"Could not resolve reference {stringReference} for validation for field {field}"); + logger.LogError("Could not resolve reference {stringReference} for validation for field {field}", stringReference, field); return null; } rawExpressionValidatıon.Message = reference.Message; @@ -187,14 +190,10 @@ public static List Validate(JsonElement validationConfig, Layou } else { - var expressionDefinition = definition.Deserialize(new JsonSerializerOptions - { - ReadCommentHandling = JsonCommentHandling.Skip, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - }); + var expressionDefinition = definition.Deserialize(_jsonOptions); if (expressionDefinition == null) { - logger.LogError($"Validation for field {field} could not be parsed"); + logger.LogError("Validation for field {field} could not be parsed", field); return null; } @@ -203,7 +202,7 @@ public static List Validate(JsonElement validationConfig, Layou var reference = resolvedDefinitions.GetValueOrDefault(expressionDefinition.Ref); if (reference == null) { - logger.LogError($"Could not resolve reference {expressionDefinition.Ref} for validation for field {field}"); + logger.LogError("Could not resolve reference {expressionDefinitionRef} for validation for field {field}", expressionDefinition.Ref, field); return null; } @@ -230,13 +229,13 @@ public static List Validate(JsonElement validationConfig, Layou if (rawExpressionValidatıon.Message == null) { - logger.LogError($"Validation for field {field} is missing message"); + logger.LogError("Validation for field {field} is missing message", field); return null; } if (rawExpressionValidatıon.Condition == null) { - logger.LogError($"Validation for field {field} is missing condition"); + logger.LogError("Validation for field {field} is missing condition", field); return null; } @@ -264,7 +263,7 @@ private static Dictionary> ParseExpressionVal var resolvedDefinition = ResolveValidationDefinition(name, definition, expressionValidationDefinitions, logger); if (resolvedDefinition == null) { - logger.LogError($"Validation definition {name} could not be resolved"); + logger.LogError("Validation definition {name} could not be resolved", name); continue; } expressionValidationDefinitions[name] = resolvedDefinition; @@ -281,17 +280,18 @@ private static Dictionary> ParseExpressionVal var validations = validationArray.Value; foreach (var validation in validations.EnumerateArray()) { - if (!expressionValidations.ContainsKey(field)) + if (!expressionValidations.TryGetValue(field, out var expressionValidation)) { - expressionValidations[field] = new List(); + expressionValidation = new List(); + expressionValidations[field] = expressionValidation; } var resolvedExpressionValidation = ResolveExpressionValidation(field, validation, expressionValidationDefinitions, logger); if (resolvedExpressionValidation == null) { - logger.LogError($"Validation for field {field} could not be resolved"); + logger.LogError("Validation for field {field} could not be resolved", field); continue; } - expressionValidations[field].Add(resolvedExpressionValidation); + expressionValidation.Add(resolvedExpressionValidation); } } } diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs index aea7d7e78..b856327fc 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs @@ -27,14 +27,14 @@ public LegacyIValidationFormDataValidator(IInstanceValidator? instanceValidator, } /// - /// + /// The legacy validator should run for all data types /// - public string DataType { get; } = "AnyType"; + public string DataType { get; } = "*"; /// /// Always run for incremental validation /// - public bool ShouldRunForIncrementalValidation(List? changedFields = null) => true; + public bool ShouldRunForIncrementalValidation(List? changedFields = null) => _instanceValidator is not null; /// diff --git a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs index 77bd010b2..5811b1337 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs @@ -6,12 +6,18 @@ namespace Altinn.App.Core.Features.Validation.Default; +/// +/// Validator that runs the required rules in the layout +/// public class RequiredLayoutValidator : IFormDataValidator { private readonly LayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; private readonly IAppResources _appResourcesService; private readonly IAppMetadata _appMetadata; + /// + /// Initializes a new instance of the class. + /// public RequiredLayoutValidator([ServiceKey] string dataType, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, IAppResources appResourcesService, IAppMetadata appMetadata) { DataType = dataType; diff --git a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs index e5243b4bd..b9102c0e7 100644 --- a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs @@ -71,10 +71,13 @@ protected void RunFor(Expression> selector) _runForPrefixes.Add(LinqExpressionHelpers.GetJsonPath(selector)); } + /// + /// Convenience method to create a validation issue for a field using a linq expression instead of a key + /// protected void CreateValidationIssue(Expression> selector, string textKey, ValidationIssueSeverity severity = ValidationIssueSeverity.Error) { Debug.Assert(ValidationIssues.Value is not null); - ValidationIssues.Value.Add( new ValidationIssue + AddValidationIssue(new ValidationIssue { Field = LinqExpressionHelpers.GetJsonPath(selector), CustomTextKey = textKey, @@ -82,12 +85,19 @@ protected void CreateValidationIssue(Expression> selector, str }); } + /// + /// Allows inheriting classes to add validation issues. + /// protected void AddValidationIssue(ValidationIssue issue) { Debug.Assert(ValidationIssues.Value is not null); ValidationIssues.Value.Add(issue); } + /// + /// Implementation of the generic interface to call the corretly typed + /// validation method implemented by the inheriting class. + /// public async Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null) { if (data is not TModel model) @@ -95,7 +105,7 @@ public async Task> ValidateFormData(Instance instance, Dat throw new ArgumentException($"Data is not of type {typeof(TModel)}"); } - ValidationIssues.Value = new List();; + ValidationIssues.Value = new List(); await ValidateFormData(instance, dataElement, model); return ValidationIssues.Value; diff --git a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs index 221d82556..c478c1a8b 100644 --- a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs +++ b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs @@ -1,14 +1,28 @@ using System.Collections; using System.Text.Json.Serialization; using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features.Validation.Default; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Mvc.ModelBinding; namespace Altinn.App.Core.Features.Validation.Helpers; +/// +/// Static helpers to make map from to list of +/// public static class ModelStateHelpers { + /// + /// Get a list of issues from a + /// + /// + /// The instance used for populating issue.InstanceId + /// Data element for populating issue.DataElementId + /// General settings to get *Fixed* prefixes + /// Type of the object to map ModelStateDictionary key to the json path field (might be different) + /// issue.Source + /// A list of the issues as our standard ValidationIssue public static List ModelStateToIssueList(ModelStateDictionary modelState, Instance instance, DataElement dataElement, GeneralSettings generalSettings, Type objectType, string source) { @@ -49,8 +63,7 @@ private static (ValidationIssueSeverity Severity, string Message) GetSeverityFro originalMessage.Remove(0, generalSettings.SoftValidationPrefix.Length)); } - if (generalSettings.FixedValidationPrefix != null - && originalMessage.StartsWith(generalSettings.FixedValidationPrefix)) + if (originalMessage.StartsWith(generalSettings.FixedValidationPrefix)) { return (ValidationIssueSeverity.Fixed, originalMessage.Remove(0, generalSettings.FixedValidationPrefix.Length)); @@ -126,6 +139,10 @@ private static (ValidationIssueSeverity Severity, string Message) GetSeverityFro return $"{jsonPropertyName}.{ModelKeyToField(rest, childType)}"; } + /// + /// Same as , but without information about a specific field + /// used by + /// public static List MapModelStateToIssueList(ModelStateDictionary modelState, Instance instance, GeneralSettings generalSettings) { diff --git a/src/Altinn.App.Core/Features/Validation/ValidationService.cs b/src/Altinn.App.Core/Features/Validation/ValidationService.cs index 4280cd386..df12f1d4a 100644 --- a/src/Altinn.App.Core/Features/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Features/Validation/ValidationService.cs @@ -38,8 +38,6 @@ public async Task> ValidateInstanceAtTask(Instance instanc ArgumentNullException.ThrowIfNull(instance); ArgumentNullException.ThrowIfNull(taskId); - var issues = new List(); - // Run task validations var taskValidators = _serviceProvider.GetServices() .Where(tv => tv.TaskId == taskId) @@ -131,11 +129,6 @@ public async Task>> ValidateFormData(In .Where(dv => dv.ShouldRunForIncrementalValidation(changedFields)) .ToArray(); - if (dataValidators.Length > 0) - { - // TODO: Remove hidden data before validation - } - var issuesLists = await Task.WhenAll(dataValidators.Select(async (v) => { try diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs index 402e92015..a2e397484 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -33,7 +33,7 @@ public async Task ValidateFormData_NoErrors() var changedFields = new List(); var validator = new LegacyIValidationFormDataValidator(null, Options.Create(new GeneralSettings())); - validator.ShouldRunForIncrementalValidation(changedFields).Should().BeTrue(); + validator.ShouldRunForIncrementalValidation(changedFields).Should().BeFalse(); // Act var result = await validator.ValidateFormData(new Instance(), new DataElement(), data, changedFields); diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs index c54a28792..a8bd5f92c 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ExpressionValidationTests.cs @@ -53,20 +53,22 @@ public class ExpressionTestAttribute : DataAttribute { public override IEnumerable GetData(MethodInfo methodInfo) { - var files = Directory.GetFiles(Path.Join("Features", "Validators", "shared-expression-validation-tests")); - - foreach (var file in files) - { - var data = File.ReadAllText(file); - ExpressionValidationTestModel testCase = JsonSerializer.Deserialize( - data, - new JsonSerializerOptions + return Directory + .GetFiles(Path.Join("Features", "Validators", "shared-expression-validation-tests")) + .Select(file => + { + var data = File.ReadAllText(file); + return new object[] { - ReadCommentHandling = JsonCommentHandling.Skip, - PropertyNamingPolicy = JsonNamingPolicy.CamelCase, - })!; - yield return new object[] { testCase }; - } + JsonSerializer.Deserialize( + data, + new JsonSerializerOptions + { + ReadCommentHandling = JsonCommentHandling.Skip, + PropertyNamingPolicy = JsonNamingPolicy.CamelCase, + })! + }; + }); } } diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs index 4dae865de..28746fb27 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs @@ -32,7 +32,7 @@ public class ValidationServiceOldTests private readonly Mock _appMetadataMock = new(); private readonly ServiceCollection _serviceCollection = new(); - private ApplicationMetadata _applicationMetadata = new("tdd/test") + private readonly ApplicationMetadata _applicationMetadata = new("tdd/test") { DataTypes = new List() { From d9026e9e2cb018420c151aad20ecb19c94d94ced Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 4 Jan 2024 10:47:31 +0100 Subject: [PATCH 3/7] Rename to get more consistent naming --- .../Features/{Validation => }/IDataElementValidator.cs | 0 ...aElementValidation.cs => DefaultDataElementValidator.cs} | 2 +- ...ator.cs => LegacyIInstanceValidatorFormDataValidator.cs} | 4 ++-- ...alidator.cs => LegacyIInstanceValidatorTaskValidator.cs} | 4 ++-- .../Features/Validation/Helpers/ModelStateHelpers.cs | 2 +- .../Validators/Default/LegacyIValidationFormDataTests.cs | 6 +++--- .../Features/Validators/ValidationServiceOldTests.cs | 2 +- 7 files changed, 10 insertions(+), 10 deletions(-) rename src/Altinn.App.Core/Features/{Validation => }/IDataElementValidator.cs (100%) rename src/Altinn.App.Core/Features/Validation/Default/{DefaultDataElementValidation.cs => DefaultDataElementValidator.cs} (98%) rename src/Altinn.App.Core/Features/Validation/Default/{LegacyIValidationFormDataValidator.cs => LegacyIInstanceValidatorFormDataValidator.cs} (88%) rename src/Altinn.App.Core/Features/Validation/Default/{LegacyIValidationTaskValidator.cs => LegacyIInstanceValidatorTaskValidator.cs} (86%) diff --git a/src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs b/src/Altinn.App.Core/Features/IDataElementValidator.cs similarity index 100% rename from src/Altinn.App.Core/Features/Validation/IDataElementValidator.cs rename to src/Altinn.App.Core/Features/IDataElementValidator.cs diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs similarity index 98% rename from src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs rename to src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs index 2c4febe47..ec19ca681 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidation.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs @@ -7,7 +7,7 @@ namespace Altinn.App.Core.Features.Validation.Default; /// /// Default validations that run on all data elements to validate metadata and file scan results. /// -public class DefaultDataElementValidation : IDataElementValidator +public class DefaultDataElementValidator : IDataElementValidator { /// public string DataType { get; } = "*"; diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs similarity index 88% rename from src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs rename to src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index b856327fc..dc574d412 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -12,7 +12,7 @@ namespace Altinn.App.Core.Features.Validation.Default; /// /// /// -public class LegacyIValidationFormDataValidator : IFormDataValidator +public class LegacyIInstanceValidatorFormDataValidator : IFormDataValidator { private readonly IInstanceValidator? _instanceValidator; private readonly GeneralSettings _generalSettings; @@ -20,7 +20,7 @@ public class LegacyIValidationFormDataValidator : IFormDataValidator /// /// constructor /// - public LegacyIValidationFormDataValidator(IInstanceValidator? instanceValidator, IOptions generalSettings) + public LegacyIInstanceValidatorFormDataValidator(IInstanceValidator? instanceValidator, IOptions generalSettings) { _instanceValidator = instanceValidator; _generalSettings = generalSettings.Value; diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs similarity index 86% rename from src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs rename to src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs index 02c18c0ec..3efd67b34 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIValidationTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -12,7 +12,7 @@ namespace Altinn.App.Core.Features.Validation.Default; /// /// Ensures that the old extention hook is still supported. /// -public class LegacyIValidationTaskValidator : ITaskValidator +public class LegacyIInstanceValidatorTaskValidator : ITaskValidator { private readonly IInstanceValidator? _instanceValidator; private readonly GeneralSettings _generalSettings; @@ -20,7 +20,7 @@ public class LegacyIValidationTaskValidator : ITaskValidator /// /// Constructor /// - public LegacyIValidationTaskValidator([ServiceKey] string taskId, IInstanceValidator? instanceValidator, IOptions generalSettings) + public LegacyIInstanceValidatorTaskValidator([ServiceKey] string taskId, IInstanceValidator? instanceValidator, IOptions generalSettings) { TaskId = taskId; _instanceValidator = instanceValidator; diff --git a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs index c478c1a8b..6bad70a2b 100644 --- a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs +++ b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs @@ -141,7 +141,7 @@ private static (ValidationIssueSeverity Severity, string Message) GetSeverityFro /// /// Same as , but without information about a specific field - /// used by + /// used by /// public static List MapModelStateToIssueList(ModelStateDictionary modelState, Instance instance, GeneralSettings generalSettings) diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs index a2e397484..e52206f96 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -15,14 +15,14 @@ namespace Altinn.App.Core.Tests.Features.Validators.Default { public class LegacyIValidationFormDataTests { - private readonly LegacyIValidationFormDataValidator _validator; + private readonly LegacyIInstanceValidatorFormDataValidator _validator; private readonly Mock _instanceValidator = new(); public LegacyIValidationFormDataTests() { var generalSettings = new GeneralSettings(); _validator = - new LegacyIValidationFormDataValidator(_instanceValidator.Object, Options.Create(generalSettings)); + new LegacyIInstanceValidatorFormDataValidator(_instanceValidator.Object, Options.Create(generalSettings)); } [Fact] @@ -32,7 +32,7 @@ public async Task ValidateFormData_NoErrors() var data = new object(); var changedFields = new List(); - var validator = new LegacyIValidationFormDataValidator(null, Options.Create(new GeneralSettings())); + var validator = new LegacyIInstanceValidatorFormDataValidator(null, Options.Create(new GeneralSettings())); validator.ShouldRunForIncrementalValidation(changedFields).Should().BeFalse(); // Act diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs index 28746fb27..e67d28ff5 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs @@ -53,7 +53,7 @@ public ValidationServiceOldTests() _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(_appModelMock.Object); _serviceCollection.AddSingleton(_appMetadataMock.Object); - _serviceCollection.AddSingleton(); + _serviceCollection.AddSingleton(); _appMetadataMock.Setup(am => am.GetApplicationMetadata()).ReturnsAsync(_applicationMetadata); } From 50f56fe722a949b6b2765e18b91b36bd515b657c Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 4 Jan 2024 16:30:17 +0100 Subject: [PATCH 4/7] Clanup DataType/TaskId to allow * Also comment out KeyedService fetching --- .../Features/IDataElementValidator.cs | 10 +--------- .../Features/IFormDataValidator.cs | 12 ++++-------- .../Validation/Default/DataAnnotationValidator.cs | 5 ++--- .../Default/DefaultDataElementValidator.cs | 2 +- .../Validation/Default/DefaultTaskValidator.cs | 5 ++--- .../Validation/Default/ExpressionValidator.cs | 6 +++--- .../LegacyIInstanceValidatorFormDataValidator.cs | 2 +- .../LegacyIInstanceValidatorTaskValidator.cs | 7 +++---- .../Validation/Default/RequiredValidator.cs | 10 ++++++---- .../Features/Validation/ValidationService.cs | 15 ++++++++------- .../Default/ExpressionValidatorTests.cs | 3 +-- .../Features/Validators/ValidationServiceTests.cs | 13 +++++++++---- 12 files changed, 41 insertions(+), 49 deletions(-) diff --git a/src/Altinn.App.Core/Features/IDataElementValidator.cs b/src/Altinn.App.Core/Features/IDataElementValidator.cs index ba4da23df..2f98f11b4 100644 --- a/src/Altinn.App.Core/Features/IDataElementValidator.cs +++ b/src/Altinn.App.Core/Features/IDataElementValidator.cs @@ -15,18 +15,10 @@ public interface IDataElementValidator /// The data type that this validator should run for. This is the id of the data type from applicationmetadata.json /// /// - /// Used by default in . Overrides might ignore this. + /// /// string DataType { get; } - /// - /// Override this method to customize what data elements this validator should run for. - /// - bool CanValidateDataType(DataType dataType) - { - return DataType == dataType.Id; - } - /// /// Run validations for a data element. This is supposed to run quickly /// diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs index 0d7d97e1e..524942d35 100644 --- a/src/Altinn.App.Core/Features/IFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -13,16 +13,11 @@ public interface IFormDataValidator /// /// The data type this validator is for. Typically either hard coded by implementation or /// or set by constructor using a and a keyed service. + /// + /// To validate all types, just use a "*" as value /// string DataType { get; } - /// - /// Extension point if the validator should run for multiple data types. - /// Typical users will just set the property. - /// - /// The ID of the data type that might be validated - bool CanValidateDataType(string dataType) => DataType == dataType; - /// /// Used for partial validation to ensure that the validator only runs when relevant fields have changed. /// @@ -45,5 +40,6 @@ public interface IFormDataValidator /// /// /// List of validation issues - Task> ValidateFormData(Instance instance, DataElement dataElement, object data, List? changedFields = null); + Task> ValidateFormData(Instance instance, DataElement dataElement, object data, + List? changedFields = null); } \ No newline at end of file diff --git a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs index c3f268b47..47a6c60e2 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs @@ -24,9 +24,8 @@ public class DataAnnotationValidator : IFormDataValidator /// /// Constructor /// - public DataAnnotationValidator([ServiceKey] string dataType, IHttpContextAccessor httpContextAccessor, IObjectModelValidator objectModelValidator, IOptions generalSettings) + public DataAnnotationValidator(IHttpContextAccessor httpContextAccessor, IObjectModelValidator objectModelValidator, IOptions generalSettings) { - DataType = dataType; _httpContextAccessor = httpContextAccessor; _objectModelValidator = objectModelValidator; _generalSettings = generalSettings.Value; @@ -35,7 +34,7 @@ public DataAnnotationValidator([ServiceKey] string dataType, IHttpContextAccesso /// /// Dummy implementation to satisfy interface, We use instead /// - public string DataType { get; } + public string DataType => "*"; /// /// Run validator for all data types. diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs index ec19ca681..43ec72b49 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs @@ -10,7 +10,7 @@ namespace Altinn.App.Core.Features.Validation.Default; public class DefaultDataElementValidator : IDataElementValidator { /// - public string DataType { get; } = "*"; + public string DataType => "*"; /// /// Runs on all data elements to validate metadata and file scan results. diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs index 9293062a8..bfc26730e 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultTaskValidator.cs @@ -16,14 +16,13 @@ public class DefaultTaskValidator : ITaskValidator /// /// Initializes a new instance of the class. /// - public DefaultTaskValidator([ServiceKey] string taskId, IAppMetadata appMetadata) + public DefaultTaskValidator(IAppMetadata appMetadata) { - TaskId = taskId; _appMetadata = appMetadata; } /// - public string TaskId { get; } + public string TaskId => "*"; /// public async Task> ValidateTask(Instance instance) diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs index 0d454e2d0..0bd5f3716 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -28,16 +28,16 @@ public class ExpressionValidator : IFormDataValidator /// /// Constructor for the expression validator /// - public ExpressionValidator([ServiceKey] string dataType, ILogger logger, IAppResources appResourceService, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer) + public ExpressionValidator(ILogger logger, IAppResources appResourceService, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer) { - DataType = dataType; _logger = logger; _appResourceService = appResourceService; _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; } /// - public string DataType { get; } + public string DataType => "*"; + /// /// Expression validations should always run (they're likely quicker to run than to figure out if relevant fields changed) /// diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index dc574d412..7dd916c92 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -29,7 +29,7 @@ public LegacyIInstanceValidatorFormDataValidator(IInstanceValidator? instanceVal /// /// The legacy validator should run for all data types /// - public string DataType { get; } = "*"; + public string DataType => "*"; /// /// Always run for incremental validation diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs index 3efd67b34..79984ebad 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -20,17 +20,16 @@ public class LegacyIInstanceValidatorTaskValidator : ITaskValidator /// /// Constructor /// - public LegacyIInstanceValidatorTaskValidator([ServiceKey] string taskId, IInstanceValidator? instanceValidator, IOptions generalSettings) + public LegacyIInstanceValidatorTaskValidator(IInstanceValidator? instanceValidator, IOptions generalSettings) { - TaskId = taskId; _instanceValidator = instanceValidator; _generalSettings = generalSettings.Value; } /// - /// The task id this validator is registrered for. + /// Run the legacy validator for all tasks /// - public string TaskId { get; } + public string TaskId => "*"; /// public async Task> ValidateTask(Instance instance) diff --git a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs index 5811b1337..f59bace83 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs @@ -18,15 +18,17 @@ public class RequiredLayoutValidator : IFormDataValidator /// /// Initializes a new instance of the class. /// - public RequiredLayoutValidator([ServiceKey] string dataType, LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, IAppResources appResourcesService, IAppMetadata appMetadata) + public RequiredLayoutValidator(LayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, IAppResources appResourcesService, IAppMetadata appMetadata) { - DataType = dataType; _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; _appResourcesService = appResourcesService; _appMetadata = appMetadata; } - /// - public string DataType { get; } + + /// + /// Run for all data types + /// + public string DataType => "*"; /// /// Required validator should always run for incremental validation, as they're almost quicker to run than to verify. diff --git a/src/Altinn.App.Core/Features/Validation/ValidationService.cs b/src/Altinn.App.Core/Features/Validation/ValidationService.cs index df12f1d4a..b6c6345c3 100644 --- a/src/Altinn.App.Core/Features/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Features/Validation/ValidationService.cs @@ -21,7 +21,7 @@ public class ValidationService : IValidationService private readonly ILogger _logger; /// - /// Constructor with DI serivces + /// Constructor with DI services /// public ValidationService(IServiceProvider serviceProvider, IDataClient dataClient, IAppModel appModel, IAppMetadata appMetadata, ILogger logger) { @@ -40,8 +40,8 @@ public async Task> ValidateInstanceAtTask(Instance instanc // Run task validations var taskValidators = _serviceProvider.GetServices() - .Where(tv => tv.TaskId == taskId) - .Concat(_serviceProvider.GetKeyedServices(taskId)) + .Where(tv => tv.TaskId == "*" || tv.TaskId == taskId) + // .Concat(_serviceProvider.GetKeyedServices(taskId)) .ToArray(); var taskIssuesTask = Task.WhenAll(taskValidators.Select(tv => @@ -77,8 +77,9 @@ public async Task> ValidateDataElement(Instance instance, // Get both keyed and non-keyed validators for the data type var validators = _serviceProvider.GetServices() - .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) - .Where(v => v.CanValidateDataType(dataType)); + .Where(v => v.DataType == "*" || v.DataType == dataType.Id) + // .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) + .ToArray(); var dataElementsIssuesTask = Task.WhenAll(validators.Select(async v => { @@ -124,8 +125,8 @@ public async Task>> ValidateFormData(In // Locate the relevant data validator services from normal and keyed services var dataValidators = _serviceProvider.GetServices() - .Where(dv => dv.CanValidateDataType(dataElement.DataType)) - .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) + .Where(dv => dv.DataType == "*" || dv.DataType == dataType.Id) + // .Concat(_serviceProvider.GetKeyedServices(dataElement.DataType)) .Where(dv => dv.ShouldRunForIncrementalValidation(changedFields)) .ToArray(); diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs index 23a48df5a..1719a6c68 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs @@ -24,7 +24,6 @@ namespace Altinn.App.Core.Tests.Features.Validators.Default; public class ExpressionValidatorTests { - private const string DataType = "default"; private readonly ExpressionValidator _validator; private readonly Mock> _logger = new(); private readonly Mock _appResources = new(MockBehavior.Strict); @@ -35,7 +34,7 @@ public ExpressionValidatorTests() { _layoutInitializer = new(MockBehavior.Strict, _appResources.Object, _frontendSettings) { CallBase = false }; _validator = - new ExpressionValidator(DataType, _logger.Object, _appResources.Object, _layoutInitializer.Object); + new ExpressionValidator(_logger.Object, _appResources.Object, _layoutInitializer.Object); } [Theory] diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 37afdd1b2..ac5940563 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -31,6 +31,11 @@ private class MyModel DataType = "MyType", }; + private static readonly DataType DefaultDataType = new() + { + Id = "MyType", + }; + private readonly Mock> _loggerMock = new(); private readonly Mock _dataClientMock = new(); private readonly Mock _appModelMock = new(); @@ -69,7 +74,7 @@ public async Task ValidateFormData_WithNoValidators_ReturnsNoErrors() var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Ola" }; - var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, DefaultDataType, data); result.Should().BeEmpty(); } @@ -81,7 +86,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsNoErrorsWhenNameIs var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Ola" }; - var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, DefaultDataType, data); result.Should().ContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator-MyType").WhoseValue.Should().HaveCount(0); result.Should().HaveCount(1); } @@ -94,7 +99,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Kari" }; - var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data); + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, DefaultDataType, data); result.Should().ContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator-MyType").WhoseValue.Should().ContainSingle().Which.CustomTextKey.Should().Be("NameNotOla"); result.Should().HaveCount(1); } @@ -107,7 +112,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsNoErrorsWhenOnlyAg var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Kari" }; - var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, null!, data, new List { "age" }); + var result = await validatorService.ValidateFormData(new Instance(), DefaultDataElement, DefaultDataType, data, new List { "age" }); result.Should() .NotContainKey("Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+MyNameValidator"); result.Should().HaveCount(0); From df7acc859e858d3dc1d87552a380b1294449ca75 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 4 Jan 2024 22:36:17 +0100 Subject: [PATCH 5/7] Fix more sonarcloud issues --- src/Altinn.App.Core/Features/IDataElementValidator.cs | 3 ++- .../Features/Validation/ValidationService.cs | 6 +++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/src/Altinn.App.Core/Features/IDataElementValidator.cs b/src/Altinn.App.Core/Features/IDataElementValidator.cs index 2f98f11b4..95e5a1c37 100644 --- a/src/Altinn.App.Core/Features/IDataElementValidator.cs +++ b/src/Altinn.App.Core/Features/IDataElementValidator.cs @@ -1,3 +1,4 @@ +using Altinn.App.Core.Features.FileAnalysis; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; @@ -7,7 +8,7 @@ namespace Altinn.App.Core.Features.Validation; /// Validator for data elements. /// See for an alternative validator for data elements with app logic. /// and that support incremental validation on save. -/// For validating the content of files, see and +/// For validating the content of files, see and /// public interface IDataElementValidator { diff --git a/src/Altinn.App.Core/Features/Validation/ValidationService.cs b/src/Altinn.App.Core/Features/Validation/ValidationService.cs index b6c6345c3..c34d78acb 100644 --- a/src/Altinn.App.Core/Features/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Features/Validation/ValidationService.cs @@ -60,8 +60,8 @@ public async Task> ValidateInstanceAtTask(Instance instanc // Run validations for single data elements var application = await _appMetadata.GetApplicationMetadata(); - var dataTypesForTask = application.DataTypes.Where(dt => dt.TaskId == taskId).ToArray(); - var dataElementsToValidate = instance.Data.Where(de => dataTypesForTask.Any(dt => dt.Id == de.DataType)).ToArray(); + var dataTypesForTask = application.DataTypes.Where(dt => dt.TaskId == taskId).ToList(); + var dataElementsToValidate = instance.Data.Where(de => dataTypesForTask.Exists(dt => dt.Id == de.DataType)).ToArray(); var dataIssuesTask = Task.WhenAll(dataElementsToValidate.Select(dataElement=>ValidateDataElement(instance, dataElement, dataTypesForTask.First(dt=>dt.Id == dataElement.DataType) ))); return (await Task.WhenAll(taskIssuesTask, dataIssuesTask)).SelectMany(x=>x.SelectMany(y=>y)).ToList(); @@ -109,7 +109,7 @@ public async Task> ValidateDataElement(Instance instance, return (await dataElementsIssuesTask).SelectMany(x=>x) .Concat(formDataIssuesDictionary.SelectMany(kv=>kv.Value)) .ToList(); - }; + } return (await dataElementsIssuesTask).SelectMany(x=>x).ToList(); } From 698500c4639e55bc01690ea6f76c8f96fc32db32 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 4 Jan 2024 22:46:39 +0100 Subject: [PATCH 6/7] Registrer new validators in DI container --- .../Extensions/ServiceCollectionExtensions.cs | 15 ++++++++++++++- .../Features/Validation/ValidationService.cs | 1 - 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs index addd6d21b..038dba4a0 100644 --- a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs @@ -9,6 +9,7 @@ using Altinn.App.Core.Features.PageOrder; using Altinn.App.Core.Features.Pdf; using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Features.Validation.Default; using Altinn.App.Core.Implementation; using Altinn.App.Core.Infrastructure.Clients.Authentication; using Altinn.App.Core.Infrastructure.Clients.Authorization; @@ -133,7 +134,7 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati { // Services for Altinn App services.TryAddTransient(); - services.TryAddTransient(); + AddValidationServices(services); services.TryAddTransient(); services.TryAddTransient(); services.TryAddSingleton(); @@ -177,6 +178,18 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati } } + private static void AddValidationServices(IServiceCollection services) + { + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + services.TryAddTransient(); + } + /// /// Checks if a service is already added to the collection. /// diff --git a/src/Altinn.App.Core/Features/Validation/ValidationService.cs b/src/Altinn.App.Core/Features/Validation/ValidationService.cs index c34d78acb..0a3dc802e 100644 --- a/src/Altinn.App.Core/Features/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Features/Validation/ValidationService.cs @@ -1,7 +1,6 @@ using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; -using Altinn.App.Core.Internal.Process.Elements; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.DependencyInjection; From 92bc3f964a693e692aac63681ab6beea65e13c0f Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 4 Jan 2024 22:53:27 +0100 Subject: [PATCH 7/7] Remove dead code --- .../Validation/Default/DataAnnotationValidator.cs | 7 +------ .../Validation/Default/DefaultDataElementValidator.cs | 7 ++----- .../Validators/Default/DataAnnotationValidatorTests.cs | 10 ---------- 3 files changed, 3 insertions(+), 21 deletions(-) diff --git a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs index 47a6c60e2..1929d48e6 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs @@ -32,15 +32,10 @@ public DataAnnotationValidator(IHttpContextAccessor httpContextAccessor, IObject } /// - /// Dummy implementation to satisfy interface, We use instead + /// Run Data annotation validation on all data types with app logic /// public string DataType => "*"; - /// - /// Run validator for all data types. - /// - public bool CanValidateDataType(string dataType) => true; - /// /// Disable incremental validation for this validator. /// diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs index 43ec72b49..841a2c16b 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs @@ -9,13 +9,10 @@ namespace Altinn.App.Core.Features.Validation.Default; /// public class DefaultDataElementValidator : IDataElementValidator { - /// - public string DataType => "*"; - /// - /// Runs on all data elements to validate metadata and file scan results. + /// Run validations on all data elements /// - public bool CanValidateDataType(DataType dataType) => true; + public string DataType => "*"; /// public Task> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType) diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs index f8f1e0a70..84f5cedc3 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs @@ -49,16 +49,6 @@ private class TestClass public TestClass? NestedProperty { get; set; } } - [Fact] - public void CanValidateDataType() - { - // Act - var result = _validator.CanValidateDataType(DataAnnotationsTestFixture.DataType); - - // Assert - Assert.True(result); - } - [Fact] public void ShouldRunForIncrementalValidation() {