Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Start work on new validation #374

Closed
wants to merge 7 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/Altinn.App.Api/Controllers/ProcessController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -46,7 +47,7 @@ public ProcessController(
ILogger<ProcessController> logger,
IInstanceClient instanceClient,
IProcessClient processClient,
IValidation validationService,
IValidationService validationService,
IAuthorizationService authorization,
IProcessReader processReader,
IProcessEngine processEngine)
Expand Down Expand Up @@ -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);
}
Expand Down
11 changes: 7 additions & 4 deletions src/Altinn.App.Api/Controllers/ValidateController.cs
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
16 changes: 14 additions & 2 deletions src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -133,7 +134,7 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati
{
// Services for Altinn App
services.TryAddTransient<IPDP, PDPAppSI>();
services.TryAddTransient<IValidation, ValidationAppSI>();
AddValidationServices(services);
services.TryAddTransient<IPrefill, PrefillSI>();
services.TryAddTransient<ISigningCredentialsResolver, SigningCredentialsResolver>();
services.TryAddSingleton<IAppResources, AppResourcesSI>();
Expand All @@ -146,7 +147,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>();
Expand Down Expand Up @@ -178,6 +178,18 @@ public static void AddAppServices(this IServiceCollection services, IConfigurati
}
}

private static void AddValidationServices(IServiceCollection services)
{
services.TryAddTransient<IValidationService, ValidationService>();
services.TryAddTransient<IFormDataValidator, RequiredLayoutValidator>();
services.TryAddTransient<IFormDataValidator, ExpressionValidator>();
services.TryAddTransient<IFormDataValidator, DataAnnotationValidator>();
services.TryAddTransient<IFormDataValidator, LegacyIInstanceValidatorFormDataValidator>();
services.TryAddTransient<IDataElementValidator, DefaultDataElementValidator>();
services.TryAddTransient<ITaskValidator, LegacyIInstanceValidatorTaskValidator>();
services.TryAddTransient<ITaskValidator, DefaultTaskValidator>();
}

/// <summary>
/// Checks if a service is already added to the collection.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>();
}
Expand Down
31 changes: 31 additions & 0 deletions src/Altinn.App.Core/Features/IDataElementValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
using Altinn.App.Core.Features.FileAnalysis;
using Altinn.App.Core.Models.Validation;
using Altinn.Platform.Storage.Interface.Models;

namespace Altinn.App.Core.Features.Validation;

/// <summary>
/// Validator for data elements.
/// See <see cref="IFormDataValidator"/> for an alternative validator for data elements with app logic.
/// and that support incremental validation on save.
/// For validating the content of files, see <see cref="IFileAnalyser"/> and <see cref="IFileValidator"/>
/// </summary>
public interface IDataElementValidator
{
/// <summary>
/// The data type that this validator should run for. This is the id of the data type from applicationmetadata.json
/// </summary>
/// <remarks>
///
/// </remarks>
string DataType { get; }

/// <summary>
/// Run validations for a data element. This is supposed to run quickly
/// </summary>
/// <param name="instance">The instance to validate</param>
/// <param name="dataElement"></param>
/// <param name="dataType"></param>
/// <returns></returns>
public Task<List<ValidationIssue>> ValidateDataElement(Instance instance, DataElement dataElement, DataType dataType);
}
45 changes: 45 additions & 0 deletions src/Altinn.App.Core/Features/IFormDataValidator.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
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.
///
/// To validate all types, just use a "*" as value
/// </summary>
string DataType { get; }

/// <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);
}
2 changes: 2 additions & 0 deletions src/Altinn.App.Core/Features/IInstanceValidator.cs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
using Altinn.App.Core.Features.Validation;
using Altinn.Platform.Storage.Interface.Models;
using Microsoft.AspNetCore.Mvc.ModelBinding;

Expand All @@ -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>
Expand Down
39 changes: 39 additions & 0 deletions src/Altinn.App.Core/Features/ITaskValidator.cs
Original file line number Diff line number Diff line change
@@ -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);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
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(IHttpContextAccessor httpContextAccessor, IObjectModelValidator objectModelValidator, IOptions<GeneralSettings> generalSettings)
{
_httpContextAccessor = httpContextAccessor;
_objectModelValidator = objectModelValidator;
_generalSettings = generalSettings.Value;
}

/// <summary>
/// Run Data annotation validation on all data types with app logic
/// </summary>
public string DataType => "*";

/// <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);
}
Comment on lines +60 to +63

Check notice

Code scanning / CodeQL

Generic catch clause Note

Generic catch clause.
}
}
Loading
Loading