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

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<ProcessController> _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<ProcessController> logger,
             IInstanceClient instanceClient,
             IProcessClient processClient,
-            IValidation validationService,
+            IValidationService validationService,
             IAuthorizationService authorization,
             IProcessReader processReader,
             IProcessEngine processEngine)
@@ -202,15 +203,15 @@ public async Task<ActionResult<List<string>>> GetNextElements(
             }
         }
 
-        private async Task<bool> CanTaskBeEnded(Instance instance, string currentElementId)
+        private async Task<bool> CanTaskBeEnded(Instance instance, string currentTaskId)
         {
             List<ValidationIssue> validationIssues = new List<ValidationIssue>();
 
             bool canEndTask;
 
             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);
             }

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;
 
         /// <summary>
         /// Initialises a new instance of the <see cref="ValidateController"/> class
         /// </summary>
         public ValidateController(
             IInstanceClient instanceClient,
-            IValidation validationService,
+            IValidationService validationService,
             IAppMetadata appMetadata)
         {
             _instanceClient = instanceClient;
@@ -66,7 +67,7 @@ public async Task<IActionResult> ValidateInstance(
 
             try 
             {
-                List<ValidationIssue> messages = await _validationService.ValidateAndUpdateProcess(instance, taskId);
+                List<ValidationIssue> messages = await _validationService.ValidateInstanceAtTask(instance, taskId);
                 return Ok(messages);
             } 
             catch (PlatformHttpException exception) 
@@ -130,9 +131,11 @@ public async Task<IActionResult> 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

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<IPDP, PDPAppSI>();
-            services.TryAddTransient<IValidation, ValidationAppSI>();
+            services.TryAddTransient<IValidationService, ValidationService>();
             services.TryAddTransient<IPrefill, PrefillSI>();
             services.TryAddTransient<ISigningCredentialsResolver, SigningCredentialsResolver>();
             services.TryAddSingleton<IAppResources, AppResourcesSI>();
@@ -146,7 +146,6 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati
 #pragma warning restore CS0618, CS0612 // Type or member is obsolete
             services.TryAddTransient<IInstantiationProcessor, NullInstantiationProcessor>();
             services.TryAddTransient<IInstantiationValidator, NullInstantiationValidator>();
-            services.TryAddTransient<IInstanceValidator, NullInstanceValidator>();
             services.TryAddTransient<IAppModel, DefaultAppModel>();
             services.TryAddTransient<DataListsFactory>();
             services.TryAddTransient<InstanceDataListsFactory>();

src/Altinn.App.Core/Features/FileAnalyzis/FileAnalysisResult.cs

@@ -37,7 +37,7 @@ public FileAnalysisResult(string analyserId)
         public string? MimeType { get; set; }
 
         /// <summary>
-        /// Key/Value pairs contaning fining from the analysis.
+        /// Key/Value pairs containing findings from the analysis.
         /// </summary>
         public IDictionary<string, string> Metadata { get; private set; } = new Dictionary<string, string>();        
     }

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;
+
+/// <summary>
+/// Interface for handling validation of form data. 
+/// (i.e. dataElements with AppLogic defined 
+/// </summary>
+public interface IFormDataValidator
+{
+    /// <summary>
+    /// The data type this validator is for. Typically either hard coded by implementation or
+    /// or set by constructor using a <see cref="ServiceKeyAttribute" /> and a keyed service.
+    /// </summary>
+    string DataType { get; }
+
+    /// <summary>
+    /// Extension point if the validator should run for multiple data types.
+    /// Typical users will just set the <see cref="DataType"/> property.
+    /// </summary>
+    /// <param name="dataType">The ID of the data type that might be validated</param>
+    bool CanValidateDataType(string dataType) => DataType == dataType;
+
+    /// <summary>
+    /// Used for partial validation to ensure that the validator only runs when relevant fields have changed.
+    /// </summary>
+    /// <param name="changedFields">List of the json path to all changed fields for incremental validation</param>
+    bool ShouldRunForIncrementalValidation(List<string>? changedFields = null);
+
+    /// <summary>
+    /// Returns the group id of the validator. This is used to run partial validations on the backend.
+    /// </summary>
+    /// <remarks>
+    /// The default implementation should work for most cases.
+    /// </remarks>
+    public string? Code => $"{this.GetType().FullName}-{DataType}";
+
+    /// <summary>
+    ///
+    /// </summary>
+    /// <param name="instance"></param>
+    /// <param name="dataElement"></param>
+    /// <param name="data"></param>
+    /// <param name="changedFields"></param>
+    /// <returns>List of validation issues</returns>
+    Task<List<ValidationIssue>> ValidateFormData(Instance instance, DataElement dataElement, object data, List<string>? changedFields = null);
+}

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;
 /// <summary>
 /// IInstanceValidator defines the methods that are used to validate data and tasks
 /// </summary>
+[Obsolete($"Use {nameof(ITaskValidator)}, {nameof(IDataElementValidator)} or {nameof(IFormDataValidator)} instead")]
 public interface IInstanceValidator
 {
     /// <summary>

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;
+
+/// <summary>
+/// Interface for handling validation of tasks.
+/// </summary>
+public interface ITaskValidator
+{
+    /// <summary>
+    /// The task id this validator is for. Typically either hard coded by implementation or
+    /// or set by constructor using a <see cref="ServiceKeyAttribute" /> and a keyed service.
+    /// </summary>
+    /// <example>
+    /// <code>
+    /// string TaskId { get; init; }
+    /// // constructor
+    /// public MyTaskValidator([ServiceKey] string taskId)
+    /// {
+    ///     TaskId = taskId;
+    /// }
+    /// </code>
+    /// </example>
+    string TaskId { get; }
+
+    /// <summary>
+    /// Unique code for the validator. Used to run partial validations on the backend.
+    /// </summary>
+    public string Code => this.GetType().FullName ?? string.Empty;
+
+    /// <summary>
+    /// Actual validation logic for the task
+    /// </summary>
+    /// <param name="instance">The instance to validate</param>
+    /// <returns>List of validation issues to add to this task validation</returns>
+    Task<List<ValidationIssue>> ValidateTask(Instance instance);
+}

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;
+
+/// <summary>
+/// Runs <see cref="System.ComponentModel.DataAnnotations"/> validation on the data object.
+/// </summary>
+public class DataAnnotationValidator : IFormDataValidator
+{
+    private readonly IHttpContextAccessor _httpContextAccessor;
+    private readonly IObjectModelValidator _objectModelValidator;
+    private readonly GeneralSettings _generalSettings;
+
+    /// <summary>
+    /// Constructor
+    /// </summary>
+    public DataAnnotationValidator([ServiceKey] string dataType, IHttpContextAccessor httpContextAccessor, IObjectModelValidator objectModelValidator, IOptions<GeneralSettings> generalSettings)
+    {
+        DataType = dataType;
+        _httpContextAccessor = httpContextAccessor;
+        _objectModelValidator = objectModelValidator;
+        _generalSettings = generalSettings.Value;
+    }
+
+    /// <summary>
+    /// Dummy implementation to satisfy interface, We use <see cref="CanValidateDataType" /> instead
+    /// </summary>
+    public string DataType { get; }
+
+    /// <summary>
+    /// Run validator for all data types.
+    /// </summary>
+    public bool CanValidateDataType(string dataType) => true;
+
+    /// <summary>
+    /// Disable incremental validation for this validator.
+    /// </summary>
+    public bool ShouldRunForIncrementalValidation(List<string>? changedFields = null) => false;
+
+    /// <inheritdoc />
+    public Task<List<ValidationIssue>> ValidateFormData(Instance instance, DataElement dataElement, object data, List<string>? 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<List<ValidationIssue>>(e);
+        }
+    }
+}

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;
+
+/// <summary>
+/// Default validations that run on all data elements to validate metadata and file scan results.
+/// </summary>
+public class DefaultDataElementValidation : IDataElementValidator
+{
+    public string DataType { get; }
+
+    /// <summary>
+    /// Runs on all data elements to validate metadata and file scan results.
+    /// </summary>
+    public bool CanValidateDataType(DataType dataType) => true;
+
+    public Task<List<ValidationIssue>> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType)
+    {
+        var issues = new List<ValidationIssue>();
+        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);
+    }
+}