diff --git a/src/Altinn.App.Api/Controllers/ActionsController.cs b/src/Altinn.App.Api/Controllers/ActionsController.cs index 7f1684461..a1eeaed01 100644 --- a/src/Altinn.App.Api/Controllers/ActionsController.cs +++ b/src/Altinn.App.Api/Controllers/ActionsController.cs @@ -5,6 +5,7 @@ using Altinn.App.Core.Features.Action; 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.Instances; using Altinn.App.Core.Internal.Validation; @@ -33,23 +34,19 @@ public class ActionsController : ControllerBase private readonly IValidationService _validationService; private readonly IDataClient _dataClient; private readonly IAppMetadata _appMetadata; + private readonly IAppModel _appModel; /// /// Create new instance of the class /// - /// The authorization service - /// The instance client - /// The user action service - /// Service for performing validations of user data - /// Client for accessing data in storage - /// Service for getting application metadata public ActionsController( IAuthorizationService authorization, IInstanceClient instanceClient, UserActionService userActionService, IValidationService validationService, IDataClient dataClient, - IAppMetadata appMetadata + IAppMetadata appMetadata, + IAppModel appModel ) { _authorization = authorization; @@ -58,6 +55,7 @@ IAppMetadata appMetadata _validationService = validationService; _dataClient = dataClient; _appMetadata = appMetadata; + _appModel = appModel; } /// @@ -162,29 +160,40 @@ public async Task> Perform( ); } + var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel); + Dictionary>>? validationIssues = null; + if (result.UpdatedDataModels is { Count: > 0 }) { - await SaveChangedModels(instance, result.UpdatedDataModels); + var changes = await SaveChangedModels(instance, dataAccessor, result.UpdatedDataModels); + + validationIssues = await GetValidations( + instance, + dataAccessor, + changes, + actionRequest.IgnoredValidators, + language + ); } - return new OkObjectResult( + return Ok( new UserActionResponse() { ClientActions = result.ClientActions, UpdatedDataModels = result.UpdatedDataModels, - UpdatedValidationIssues = await GetValidations( - instance, - result.UpdatedDataModels, - actionRequest.IgnoredValidators, - language - ), + UpdatedValidationIssues = validationIssues, RedirectUrl = result.RedirectUrl, } ); } - private async Task SaveChangedModels(Instance instance, Dictionary resultUpdatedDataModels) + private async Task> SaveChangedModels( + Instance instance, + CachedInstanceDataAccessor dataAccessor, + Dictionary resultUpdatedDataModels + ) { + var changes = new List(); var instanceIdentifier = new InstanceIdentifier(instance); foreach (var (elementId, newModel) in resultUpdatedDataModels) { @@ -192,11 +201,12 @@ private async Task SaveChangedModels(Instance instance, Dictionary d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase)); + var previousData = await dataAccessor.Get(dataElement); ObjectUtils.InitializeAltinnRowId(newModel); ObjectUtils.PrepareModelForXmlStorage(newModel); - var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase)); await _dataClient.UpdateData( newModel, instanceIdentifier.InstanceGuid, @@ -206,61 +216,65 @@ await _dataClient.UpdateData( instanceIdentifier.InstanceOwnerPartyId, Guid.Parse(dataElement.Id) ); + // update dataAccessor to use the changed data + dataAccessor.Set(dataElement, newModel); + // add change to list + changes.Add( + new DataElementChange + { + DataElement = dataElement, + PreviousValue = previousData, + CurrentValue = newModel, + } + ); } + return changes; } - private async Task>>?> GetValidations( + private async Task>>?> GetValidations( Instance instance, - Dictionary? resultUpdatedDataModels, + IInstanceDataAccessor dataAccessor, + List changes, List? ignoredValidators, string? language ) { - if (resultUpdatedDataModels is null || resultUpdatedDataModels.Count < 1) - { - return null; - } - - var instanceIdentifier = new InstanceIdentifier(instance); - var application = await _appMetadata.GetApplicationMetadata(); + var taskId = instance.Process.CurrentTask.ElementId; + var validationIssues = await _validationService.ValidateIncrementalFormData( + instance, + taskId, + changes, + dataAccessor, + ignoredValidators, + language + ); - var updatedValidationIssues = new Dictionary>>(); + // For historical reasons the validation issues from actions controller is separated per data element + // The easiest way was to keep this behaviour to improve compatibility with older frontend versions + return PartitionValidationIssuesByDataElement(validationIssues); + } - // TODO: Consider validating models in parallel - foreach (var (elementId, newModel) in resultUpdatedDataModels) + private Dictionary< + string, + Dictionary> + > PartitionValidationIssuesByDataElement(Dictionary> validationIssues) + { + var updatedValidationIssues = new Dictionary>>(); + foreach (var (validationSource, issuesFromSource) in validationIssues) { - if (newModel is null) - { - continue; - } - - var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase)); - var dataType = application.DataTypes.First(d => - d.Id.Equals(dataElement.DataType, StringComparison.OrdinalIgnoreCase) - ); - - // TODO: Consider rewriting so that we get the original data the IUserAction have requested instead of fetching it again - var oldData = await _dataClient.GetFormData( - instanceIdentifier.InstanceGuid, - newModel.GetType(), - instance.Org, - instance.AppId.Split('/')[1], - instanceIdentifier.InstanceOwnerPartyId, - Guid.Parse(dataElement.Id) - ); - - var validationIssues = await _validationService.ValidateFormData( - instance, - dataElement, - dataType, - newModel, - oldData, - ignoredValidators, - language - ); - if (validationIssues.Count > 0) + foreach (var issue in issuesFromSource) { - updatedValidationIssues.Add(elementId, validationIssues); + if (!updatedValidationIssues.TryGetValue(issue.DataElementId ?? "", out var elementIssues)) + { + elementIssues = new Dictionary>(); + updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues; + } + if (!elementIssues.TryGetValue(validationSource, out var sourceIssues)) + { + sourceIssues = new List(); + elementIssues[validationSource] = sourceIssues; + } + sourceIssues.Add(issue); } } diff --git a/src/Altinn.App.Api/Controllers/DataController.cs b/src/Altinn.App.Api/Controllers/DataController.cs index 958853a33..02a009c21 100644 --- a/src/Altinn.App.Api/Controllers/DataController.cs +++ b/src/Altinn.App.Api/Controllers/DataController.cs @@ -452,6 +452,53 @@ public async Task> PatchFormData( [FromBody] DataPatchRequest dataPatchRequest, [FromQuery] string? language = null ) + { + var request = new DataPatchRequestMultiple() + { + Patches = new() { [dataGuid] = dataPatchRequest.Patch }, + IgnoredValidators = dataPatchRequest.IgnoredValidators + }; + var response = await PatchFormDataMultiple(org, app, instanceOwnerPartyId, instanceGuid, request, language); + + if (response.Result is OkObjectResult { Value: DataPatchResponseMultiple newResponse }) + { + // Map the new response to the old response + return Ok( + new DataPatchResponse() + { + ValidationIssues = newResponse.ValidationIssues, + NewDataModel = newResponse.NewDataModels[dataGuid], + } + ); + } + + // Return the error object unchanged + return response.Result ?? throw new InvalidOperationException("Response is null"); + } + + /// + /// Updates an existing form data element with a patch of changes. + /// + /// unique identfier of the organisation responsible for the app + /// application identifier which is unique within an organisation + /// unique id of the party that is the owner of the instance + /// unique id to identify the instance + /// Container object for the and list of ignored validators + /// The language selected by the user. + /// A response object with the new full model and validation issues from all the groups that run + [Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)] + [HttpPatch("")] + [ProducesResponseType(typeof(DataPatchResponseMultiple), 200)] + [ProducesResponseType(typeof(ProblemDetails), 409)] + [ProducesResponseType(typeof(ProblemDetails), 422)] + public async Task> PatchFormDataMultiple( + [FromRoute] string org, + [FromRoute] string app, + [FromRoute] int instanceOwnerPartyId, + [FromRoute] Guid instanceGuid, + [FromBody] DataPatchRequestMultiple dataPatchRequest, + [FromQuery] string? language = null + ) { try { @@ -464,44 +511,59 @@ public async Task> PatchFormData( ); } - var dataElement = instance.Data.First(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal)); + CachedInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor( + instance, + _dataClient, + _appMetadata, + _appModel + ); - if (dataElement == null) + foreach (Guid dataGuid in dataPatchRequest.Patches.Keys) { - return NotFound("Did not find data element"); - } + var dataElement = instance.Data.First(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal)); - var dataType = await GetDataType(dataElement); + if (dataElement == null) + { + return NotFound("Did not find data element"); + } - if (dataType?.AppLogic?.ClassRef is null) - { - _logger.LogError( - "Could not determine if {dataType} requires app logic for application {org}/{app}", - dataType, - org, - app - ); - return BadRequest($"Could not determine if data type {dataType?.Id} requires application logic."); + var dataType = await GetDataType(dataElement); + + if (dataType?.AppLogic?.ClassRef is null) + { + _logger.LogError( + "Could not determine if {dataType} requires app logic for application {org}/{app}", + dataType, + org, + app + ); + return BadRequest($"Could not determine if data type {dataType?.Id} requires application logic."); + } } - ServiceResult res = await _patchService.ApplyPatch( + ServiceResult res = await _patchService.ApplyPatches( instance, - dataType, - dataElement, - dataPatchRequest.Patch, + dataPatchRequest.Patches, language, dataPatchRequest.IgnoredValidators ); if (res.Success) { - await UpdateDataValuesOnInstance(instance, dataType.Id, res.Ok.NewDataModel); - await UpdatePresentationTextsOnInstance(instance, dataType.Id, res.Ok.NewDataModel); + foreach (var dataGuid in dataPatchRequest.Patches.Keys) + { + await UpdateDataValuesOnInstance(instance, dataGuid.ToString(), res.Ok.NewDataModels[dataGuid]); + await UpdatePresentationTextsOnInstance( + instance, + dataGuid.ToString(), + res.Ok.NewDataModels[dataGuid] + ); + } return Ok( - new DataPatchResponse + new DataPatchResponseMultiple() { - NewDataModel = res.Ok.NewDataModel, + NewDataModels = res.Ok.NewDataModels, ValidationIssues = res.Ok.ValidationIssues } ); @@ -513,7 +575,7 @@ public async Task> PatchFormData( { return HandlePlatformHttpException( e, - $"Unable to update data element {dataGuid} for instance {instanceOwnerPartyId}/{instanceGuid}" + $"Unable to update data element {string.Join(", ", dataPatchRequest.Patches.Keys)} for instance {instanceOwnerPartyId}/{instanceGuid}" ); } } diff --git a/src/Altinn.App.Api/Controllers/ProcessController.cs b/src/Altinn.App.Api/Controllers/ProcessController.cs index 1881becd2..f8b4b537a 100644 --- a/src/Altinn.App.Api/Controllers/ProcessController.cs +++ b/src/Altinn.App.Api/Controllers/ProcessController.cs @@ -4,6 +4,9 @@ using Altinn.App.Api.Models; using Altinn.App.Core.Constants; 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.Instances; using Altinn.App.Core.Internal.Process; using Altinn.App.Core.Internal.Process.Elements; @@ -38,6 +41,9 @@ public class ProcessController : ControllerBase private readonly IAuthorizationService _authorization; private readonly IProcessEngine _processEngine; private readonly IProcessReader _processReader; + private readonly IDataClient _dataClient; + private readonly IAppMetadata _appMetadata; + private readonly IAppModel _appModel; /// /// Initializes a new instance of the @@ -49,7 +55,10 @@ public ProcessController( IValidationService validationService, IAuthorizationService authorization, IProcessReader processReader, - IProcessEngine processEngine + IProcessEngine processEngine, + IDataClient dataClient, + IAppMetadata appMetadata, + IAppModel appModel ) { _logger = logger; @@ -59,6 +68,9 @@ IProcessEngine processEngine _authorization = authorization; _processReader = processReader; _processEngine = processEngine; + _dataClient = dataClient; + _appMetadata = appMetadata; + _appModel = appModel; } /// @@ -237,7 +249,13 @@ [FromRoute] Guid instanceGuid string? language ) { - var validationIssues = await _validationService.ValidateInstanceAtTask(instance, currentTaskId, language); + var dataAcceesor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel); + var validationIssues = await _validationService.ValidateInstanceAtTask( + instance, + currentTaskId, + dataAcceesor, + language + ); var success = validationIssues.TrueForAll(v => v.Severity != ValidationIssueSeverity.Error); if (!success) diff --git a/src/Altinn.App.Api/Controllers/ValidateController.cs b/src/Altinn.App.Api/Controllers/ValidateController.cs index 260432607..e8f231ff5 100644 --- a/src/Altinn.App.Api/Controllers/ValidateController.cs +++ b/src/Altinn.App.Api/Controllers/ValidateController.cs @@ -1,5 +1,7 @@ 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.Instances; using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models.Validation; @@ -17,6 +19,8 @@ namespace Altinn.App.Api.Controllers; public class ValidateController : ControllerBase { private readonly IInstanceClient _instanceClient; + private readonly IDataClient _dataClient; + private readonly IAppModel _appModel; private readonly IAppMetadata _appMetadata; private readonly IValidationService _validationService; @@ -26,12 +30,16 @@ public class ValidateController : ControllerBase public ValidateController( IInstanceClient instanceClient, IValidationService validationService, - IAppMetadata appMetadata + IAppMetadata appMetadata, + IDataClient dataClient, + IAppModel appModel ) { _instanceClient = instanceClient; _validationService = validationService; _appMetadata = appMetadata; + _dataClient = dataClient; + _appModel = appModel; } /// @@ -45,6 +53,7 @@ IAppMetadata appMetadata /// The currently used language by the user (or null if not available) [HttpGet] [Route("{org}/{app}/instances/{instanceOwnerPartyId:int}/{instanceGuid:guid}/validate")] + [ProducesResponseType(typeof(ValidationIssueWithSource), 200)] public async Task ValidateInstance( [FromRoute] string org, [FromRoute] string app, @@ -67,9 +76,11 @@ public async Task ValidateInstance( try { - List messages = await _validationService.ValidateInstanceAtTask( + var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel); + List messages = await _validationService.ValidateInstanceAtTask( instance, taskId, + dataAccessor, language ); return Ok(messages); @@ -95,6 +106,9 @@ public async Task ValidateInstance( /// Unique id identifying specific data element /// The currently used language by the user (or null if not available) [HttpGet] + [Obsolete( + "There is no longer any concept of validating a single data element. Use the /validate endpoint instead." + )] [Route("{org}/{app}/instances/{instanceOwnerId:int}/{instanceId:guid}/data/{dataGuid:guid}/validate")] public async Task ValidateData( [FromRoute] string org, @@ -116,7 +130,7 @@ public async Task ValidateData( throw new ValidationException("Unable to validate instance without a started process."); } - List messages = new List(); + List messages = new List(); DataElement? element = instance.Data.FirstOrDefault(d => d.Id == dataGuid.ToString()); @@ -134,22 +148,29 @@ public async Task ValidateData( throw new ValidationException("Unknown element type."); } - messages.AddRange(await _validationService.ValidateDataElement(instance, element, dataType, language)); + var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel); + + // TODO: Consider filtering so that only relevant issues are reported. + messages.AddRange( + await _validationService.ValidateInstanceAtTask(instance, dataType.TaskId, dataAccessor, language) + ); string taskId = instance.Process.CurrentTask.ElementId; // Should this be a BadRequest instead? if (!dataType.TaskId.Equals(taskId, StringComparison.OrdinalIgnoreCase)) { - ValidationIssue message = new ValidationIssue - { - Code = ValidationIssueCodes.DataElementCodes.DataElementValidatedAtWrongTask, - Severity = ValidationIssueSeverity.Warning, - DataElementId = element.Id, - Description = $"Data element for task {dataType.TaskId} validated while currentTask is {taskId}", - CustomTextKey = ValidationIssueCodes.DataElementCodes.DataElementValidatedAtWrongTask, - CustomTextParams = new List() { dataType.TaskId, taskId }, - }; + ValidationIssueWithSource message = + new() + { + Code = ValidationIssueCodes.DataElementCodes.DataElementValidatedAtWrongTask, + Severity = ValidationIssueSeverity.Warning, + DataElementId = element.Id, + Description = $"Data element for task {dataType.TaskId} validated while currentTask is {taskId}", + CustomTextKey = ValidationIssueCodes.DataElementCodes.DataElementValidatedAtWrongTask, + CustomTextParams = new List() { dataType.TaskId, taskId }, + Source = GetType().FullName ?? String.Empty + }; messages.Add(message); } diff --git a/src/Altinn.App.Api/Models/DataPatchRequestMultiple.cs b/src/Altinn.App.Api/Models/DataPatchRequestMultiple.cs new file mode 100644 index 000000000..75a6061d5 --- /dev/null +++ b/src/Altinn.App.Api/Models/DataPatchRequestMultiple.cs @@ -0,0 +1,26 @@ +using System.Text.Json.Serialization; +using Altinn.App.Api.Controllers; +using Altinn.Platform.Storage.Interface.Models; +using Json.Patch; + +namespace Altinn.App.Api.Models; + +/// +/// Represents the request to patch data on the in the +/// version that supports multiple data models in the same request. +/// +public class DataPatchRequestMultiple +{ + /// + /// The Patch operation to perform in a dictionary keyed on the . + /// + [JsonPropertyName("patches")] + public required Dictionary Patches { get; init; } + + /// + /// List of validators to ignore during the patch operation. + /// Issues from these validators will not be run during the save operation, but the validator will run on process/next + /// + [JsonPropertyName("ignoredValidators")] + public required List? IgnoredValidators { get; init; } +} diff --git a/src/Altinn.App.Api/Models/DataPatchResponse.cs b/src/Altinn.App.Api/Models/DataPatchResponse.cs index 97fe3e0cf..0d453caed 100644 --- a/src/Altinn.App.Api/Models/DataPatchResponse.cs +++ b/src/Altinn.App.Api/Models/DataPatchResponse.cs @@ -11,7 +11,7 @@ public class DataPatchResponse /// /// The validation issues that were found during the patch operation. /// - public required Dictionary> ValidationIssues { get; init; } + public required Dictionary> ValidationIssues { get; init; } /// /// The current data model after the patch operation. diff --git a/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs b/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs new file mode 100644 index 000000000..dd64c5352 --- /dev/null +++ b/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs @@ -0,0 +1,20 @@ +using Altinn.App.Api.Controllers; +using Altinn.App.Core.Models.Validation; + +namespace Altinn.App.Api.Models; + +/// +/// Represents the response from a data patch operation on the . +/// +public class DataPatchResponseMultiple +{ + /// + /// The validation issues that were found during the patch operation. + /// + public required Dictionary> ValidationIssues { get; init; } + + /// + /// The current data in all data models updated by the patch operation. + /// + public required Dictionary NewDataModels { get; init; } +} diff --git a/src/Altinn.App.Api/Models/UserActionResponse.cs b/src/Altinn.App.Api/Models/UserActionResponse.cs index 2eb23150c..a03224fd3 100644 --- a/src/Altinn.App.Api/Models/UserActionResponse.cs +++ b/src/Altinn.App.Api/Models/UserActionResponse.cs @@ -20,7 +20,10 @@ public class UserActionResponse /// Validators that are not listed in the dictionary are assumed to have not been executed /// [JsonPropertyName("updatedValidationIssues")] - public Dictionary>>? UpdatedValidationIssues { get; set; } + public Dictionary< + string, + Dictionary> + >? UpdatedValidationIssues { get; set; } /// /// Actions the client should perform after action has been performed backend diff --git a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs index 94f526c9e..4d0e3dd2e 100644 --- a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs @@ -205,7 +205,6 @@ IWebHostEnvironment env private static void AddValidationServices(IServiceCollection services, IConfiguration configuration) { - CachedFormDataAccessor.Register(services); services.AddTransient(); services.AddScoped(); if (configuration.GetSection("AppSettings").Get()?.RequiredValidation == true) @@ -218,9 +217,7 @@ private static void AddValidationServices(IServiceCollection services, IConfigur services.AddTransient(); } services.AddTransient(); - services.AddTransient(); services.AddTransient(); - services.AddTransient(); services.AddTransient(); } diff --git a/src/Altinn.App.Core/Features/ITaskValidator.cs b/src/Altinn.App.Core/Features/ITaskValidator.cs index 667e1164d..1cf2c6abf 100644 --- a/src/Altinn.App.Core/Features/ITaskValidator.cs +++ b/src/Altinn.App.Core/Features/ITaskValidator.cs @@ -10,26 +10,18 @@ namespace Altinn.App.Core.Features; 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. + /// The task id this validator is for, or "*" if relevant for all tasks. /// - /// - /// - /// string TaskId { get; init; } - /// // constructor - /// public MyTaskValidator([ServiceKey] string taskId) - /// { - /// TaskId = taskId; - /// } - /// - /// string TaskId { get; } /// - /// Returns the group id of the validator. - /// The default is based on the FullName and TaskId fields, and should not need customization + /// Returns the name to be used in the "Source" of property in all + /// 's created by the validator. /// - string ValidationSource => $"{this.GetType().FullName}-{TaskId}"; + /// + /// The default is based on the FullName and TaskId fields, and should not need customization + /// + string ValidationSource => $"{GetType().FullName}-{TaskId}"; /// /// Actual validation logic for the task diff --git a/src/Altinn.App.Core/Features/IValidator.cs b/src/Altinn.App.Core/Features/IValidator.cs new file mode 100644 index 000000000..17daffdf3 --- /dev/null +++ b/src/Altinn.App.Core/Features/IValidator.cs @@ -0,0 +1,86 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features; + +/// +/// Main interface for validation of instances +/// +public interface IValidator +{ + /// + /// The task id for the task that the validator is associated with or "*" if the validator should run for all tasks. + /// + public string TaskId { get; } + + /// + /// Unique string that identifies the source of the validation issues from this validator + /// Used for incremental validation. Default implementation should typically work. + /// + public string ValidationSource => $"{GetType().FullName}-{TaskId}"; + + /// + /// + /// + /// The instance to validate + /// The current task. + /// Language for messages, if the messages are too dynamic for the translation system + /// Use this to access data from other data elements + /// + public Task> Validate( + Instance instance, + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor + ); + + /// + /// For patch requests we typically don't run all validators, because some validators will predictably produce the same issues as previously. + /// This method is used to determine if the validator has relevant changes, or if the cached issues list can be used. + /// + /// The instance to validate + /// The current task ID + /// List of changed data elements with current and previous value + /// Use this to access data from other data elements + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ); +} + +/// +/// Represents a change in a data element with current and previous deserialized data +/// +public class DataElementChange +{ + /// + /// The data element the change is related to + /// + public required DataElement DataElement { get; init; } + + /// + /// The state of the data element before the change + /// + public required object PreviousValue { get; init; } + + /// + /// The state of the data element after the change + /// + public required object CurrentValue { get; init; } +} + +/// +/// Service for accessing data from other data elements in the +/// +public interface IInstanceDataAccessor +{ + /// + /// Get the actual data represented in the data element. + /// + /// The data element to retrieve. Must be from the instance that is currently active + /// The deserialized data model for this data element or a stream for binary elements + Task Get(DataElement dataElement); +} diff --git a/src/Altinn.App.Core/Features/Telemetry.Data.cs b/src/Altinn.App.Core/Features/Telemetry.Data.cs index a049d74e2..b17b57708 100644 --- a/src/Altinn.App.Core/Features/Telemetry.Data.cs +++ b/src/Altinn.App.Core/Features/Telemetry.Data.cs @@ -32,6 +32,12 @@ internal void DataPatched(PatchResult result) => return activity; } + internal Activity? StartDataProcessWriteActivity(IDataProcessor dataProcessor) + { + var activity = ActivitySource.StartActivity($"{Prefix}.ProcessWrite.{dataProcessor.GetType().Name}"); + return activity; + } + internal static class Data { internal const string Prefix = "Data"; diff --git a/src/Altinn.App.Core/Features/Telemetry.Validation.cs b/src/Altinn.App.Core/Features/Telemetry.Validation.cs index 9ef4c4763..b4ac4eb84 100644 --- a/src/Altinn.App.Core/Features/Telemetry.Validation.cs +++ b/src/Altinn.App.Core/Features/Telemetry.Validation.cs @@ -18,52 +18,27 @@ private void InitValidation(InitContext context) { } return activity; } - internal Activity? StartRunTaskValidatorActivity(ITaskValidator validator) + internal Activity? StartValidateIncrementalActivity( + Instance instance, + string taskId, + List changes + ) { - var activity = ActivitySource.StartActivity($"{Prefix}.RunTaskValidator"); - - activity?.SetTag(InternalLabels.ValidatorType, validator.GetType().Name); - activity?.SetTag(InternalLabels.ValidatorSource, validator.ValidationSource); - - return activity; - } - - internal Activity? StartValidateDataElementActivity(Instance instance, DataElement dataElement) - { - var activity = ActivitySource.StartActivity($"{Prefix}.ValidateDataElement"); - activity?.SetInstanceId(instance); - activity?.SetDataElementId(dataElement); - return activity; - } - - internal Activity? StartRunDataElementValidatorActivity(IDataElementValidator validator) - { - var activity = ActivitySource.StartActivity($"{Prefix}.RunDataElementValidator"); - - activity?.SetTag(InternalLabels.ValidatorType, validator.GetType().Name); - activity?.SetTag(InternalLabels.ValidatorSource, validator.ValidationSource); - - return activity; - } - - internal Activity? StartValidateFormDataActivity(Instance instance, DataElement dataElement) - { - var activity = ActivitySource.StartActivity($"{Prefix}.ValidateFormData"); + ArgumentException.ThrowIfNullOrWhiteSpace(taskId); + ArgumentNullException.ThrowIfNull(changes); + var activity = ActivitySource.StartActivity($"{Prefix}.ValidateIncremental"); + activity?.SetTaskId(taskId); activity?.SetInstanceId(instance); - activity?.SetDataElementId(dataElement); + // TODO: record the guid for the changed elements in a sensible list return activity; } - internal Activity? StartRunFormDataValidatorActivity(IFormDataValidator validator) - { - var activity = ActivitySource.StartActivity($"{Prefix}.RunFormDataValidator"); - - activity?.SetTag(InternalLabels.ValidatorType, validator.GetType().Name); - activity?.SetTag(InternalLabels.ValidatorSource, validator.ValidationSource); - - return activity; - } + internal Activity? StartRunValidatorActivity(IValidator validator) => + ActivitySource + .StartActivity($"{Prefix}.RunValidator") + ?.SetTag(InternalLabels.ValidatorType, validator.GetType().Name) + .SetTag(InternalLabels.ValidatorSource, validator.ValidationSource); internal static class Validation { diff --git a/src/Altinn.App.Core/Features/Telemetry.cs b/src/Altinn.App.Core/Features/Telemetry.cs index d389fb79c..587009854 100644 --- a/src/Altinn.App.Core/Features/Telemetry.cs +++ b/src/Altinn.App.Core/Features/Telemetry.cs @@ -180,6 +180,7 @@ internal static class InternalLabels internal const string AuthorizerTaskId = "authorization.authorizer.task.id"; internal const string ValidatorType = "validator.type"; internal const string ValidatorSource = "validator.source"; + internal const string ValidatorRelevantChanges = "validator.relevant_changes"; } private void InitMetricCounter(InitContext context, string name, Action> init) diff --git a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs index 210afb25b..277a18e1b 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DataAnnotationValidator.cs @@ -42,7 +42,7 @@ IOptions generalSettings /// /// This validator has the code "DataAnnotations" and this is known by the frontend, who may request this validator to not run for incremental validation. /// - public string ValidationSource => "DataAnnotations"; + public string ValidationSource => ValidationIssueSources.DataAnnotations; /// /// We don't know which fields are relevant for data annotation validation, so we always run it. @@ -83,8 +83,7 @@ public Task> ValidateFormData( instance, dataElement, _generalSettings, - data.GetType(), - ValidationIssueSources.ModelState + data.GetType() ) ); } diff --git a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs index 999ebb644..34c7f34a2 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/DefaultDataElementValidator.cs @@ -53,7 +53,8 @@ public Task> ValidateDataElement( DataElementId = dataElement.Id, Code = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, Severity = ValidationIssueSeverity.Error, - Description = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, + Description = + $"ContentType {contentTypeWithoutEncoding} not allowed for {string.Join(",", dataType.AllowedContentTypes)}", Field = dataType.Id } ); diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs index 8b8fca981..1f72203c9 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -42,7 +42,7 @@ ILayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer /// /// This validator has the code "Expression" and this is known by the frontend, who may request this validator to not run for incremental validation. /// - public string ValidationSource => "Expression"; + public string ValidationSource => ValidationIssueSources.Expression; /// /// We don't have an efficient way to figure out if changes to the model results in different validations, and frontend ignores this anyway @@ -121,7 +121,6 @@ public async Task> ValidateFormData( Severity = validation.Severity ?? ValidationIssueSeverity.Error, CustomTextKey = validation.Message, Code = validation.Message, - Source = ValidationIssueSources.Expression, }; validationIssues.Add(validationIssue); diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index 5533a391f..34617f3cd 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -2,6 +2,7 @@ using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features.Validation.Helpers; +using Altinn.App.Core.Internal.App; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -12,9 +13,10 @@ namespace Altinn.App.Core.Features.Validation.Default; /// /// This validator is used to run the legacy IInstanceValidator.ValidateData method /// -public class LegacyIInstanceValidatorFormDataValidator : IFormDataValidator +public class LegacyIInstanceValidatorFormDataValidator : IValidator { - private readonly IInstanceValidator? _instanceValidator; + private readonly IInstanceValidator _instanceValidator; + private readonly IAppMetadata _appMetadata; private readonly GeneralSettings _generalSettings; /// @@ -22,17 +24,19 @@ public class LegacyIInstanceValidatorFormDataValidator : IFormDataValidator /// public LegacyIInstanceValidatorFormDataValidator( IOptions generalSettings, - IInstanceValidator? instanceValidator = null + IInstanceValidator instanceValidator, + IAppMetadata appMetadata ) { _instanceValidator = instanceValidator; + _appMetadata = appMetadata; _generalSettings = generalSettings.Value; } /// - /// The legacy validator should run for all data types + /// The legacy validator should run for all tasks, because there is no way to specify task for the legacy validator /// - public string DataType => _instanceValidator is null ? "" : "*"; + public string TaskId => "*"; /// > public string ValidationSource @@ -45,33 +49,46 @@ public string ValidationSource } } - /// - /// Always run for incremental validation (if it exists) - /// - public bool HasRelevantChanges(object current, object previous) => _instanceValidator is not null; - /// - public async Task> ValidateFormData( + public async Task> Validate( Instance instance, - DataElement dataElement, - object data, - string? language + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor ) { - if (_instanceValidator is null) + var issues = new List(); + var appMetadata = await _appMetadata.GetApplicationMetadata(); + var dataTypes = appMetadata.DataTypes.Where(d => d.TaskId == taskId).Select(d => d.Id).ToList(); + foreach (var dataElement in instance.Data.Where(d => dataTypes.Contains(d.DataType))) { - return new List(); + var data = await instanceDataAccessor.Get(dataElement); + var modelState = new ModelStateDictionary(); + await _instanceValidator.ValidateData(data, modelState); + issues.AddRange( + ModelStateHelpers.ModelStateToIssueList( + modelState, + instance, + dataElement, + _generalSettings, + data.GetType() + ) + ); } - var modelState = new ModelStateDictionary(); - await _instanceValidator.ValidateData(data, modelState); - return ModelStateHelpers.ModelStateToIssueList( - modelState, - instance, - dataElement, - _generalSettings, - data.GetType(), - ValidationIssueSources.Custom - ); + return issues; + } + + /// + /// Always run for incremental validation, because the legacy validator don't have a way to know when changes are relevant + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ) + { + return Task.FromResult(true); } } diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs index e1c48343e..5920289ed 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -12,9 +12,9 @@ namespace Altinn.App.Core.Features.Validation.Default; /// /// Ensures that the old extension hook is still supported. /// -public class LegacyIInstanceValidatorTaskValidator : ITaskValidator +public class LegacyIInstanceValidatorTaskValidator : IValidator { - private readonly IInstanceValidator? _instanceValidator; + private readonly IInstanceValidator _instanceValidator; private readonly GeneralSettings _generalSettings; /// @@ -22,7 +22,7 @@ public class LegacyIInstanceValidatorTaskValidator : ITaskValidator /// public LegacyIInstanceValidatorTaskValidator( IOptions generalSettings, - IInstanceValidator? instanceValidator = null + IInstanceValidator instanceValidator ) { _instanceValidator = instanceValidator; @@ -39,22 +39,35 @@ public string ValidationSource { get { - var type = _instanceValidator?.GetType() ?? GetType(); + var type = _instanceValidator.GetType(); Debug.Assert(type.FullName is not null, "FullName does not return null on class/struct types"); return type.FullName; } } /// - public async Task> ValidateTask(Instance instance, string taskId, string? language) + public async Task> Validate( + Instance instance, + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor + ) { - if (_instanceValidator is null) - { - return new List(); - } - var modelState = new ModelStateDictionary(); await _instanceValidator.ValidateTask(instance, taskId, modelState); return ModelStateHelpers.MapModelStateToIssueList(modelState, instance, _generalSettings); } + + /// + /// Don't run the legacy Instance validator for incremental validation (it was not running before) + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ) + { + return Task.FromResult(false); + } } diff --git a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs index 22c2db084..ff50289d6 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs @@ -27,7 +27,7 @@ public RequiredLayoutValidator(ILayoutEvaluatorStateInitializer layoutEvaluatorS /// /// This validator has the code "Required" and this is known by the frontend, who may request this validator to not run for incremental validation. /// - public string ValidationSource => "Required"; + public string ValidationSource => ValidationIssueSources.Required; /// /// We don't have an efficient way to figure out if changes to the model results in different validations, and frontend ignores this anyway diff --git a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs index a22871460..c30d67f5e 100644 --- a/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs +++ b/src/Altinn.App.Core/Features/Validation/Helpers/ModelStateHelpers.cs @@ -21,15 +21,13 @@ public static class ModelStateHelpers /// 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 + Type objectType ) { var validationIssues = new List(); @@ -47,7 +45,6 @@ string source new ValidationIssue { DataElementId = dataElement.Id, - Source = source, Code = severityAndMessage.Message, Field = ModelKeyToField(modelKey, objectType), Severity = severityAndMessage.Severity, diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs new file mode 100644 index 000000000..090730823 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs @@ -0,0 +1,79 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation.Wrappers; + +/// +/// Wrap the old interface to the new interface. +/// +internal class DataElementValidatorWrapper : IValidator +{ + private readonly IDataElementValidator _dataElementValidator; + private readonly string _taskId; + private readonly List _dataTypes; + + public DataElementValidatorWrapper( + IDataElementValidator dataElementValidator, + string taskId, + List dataTypes + ) + { + _dataElementValidator = dataElementValidator; + _taskId = taskId; + _dataTypes = dataTypes; + } + + /// + public string TaskId => _taskId; + + /// + public string ValidationSource => _dataElementValidator.ValidationSource; + + /// + /// Run all legacy instances for the given . + /// + public async Task> Validate( + Instance instance, + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor + ) + { + var issues = new List(); + var validateAllElements = _dataElementValidator.DataType == "*"; + foreach (var dataElement in instance.Data) + { + if (validateAllElements || _dataElementValidator.DataType == dataElement.DataType) + { + var dataType = _dataTypes.Find(d => d.Id == dataElement.DataType); + if (dataType is null) + { + throw new InvalidOperationException( + $"DataType {dataElement.DataType} not found in dataTypes from applicationmetadata" + ); + } + var dataElementValidationResult = await _dataElementValidator.ValidateDataElement( + instance, + dataElement, + dataType, + language + ); + issues.AddRange(dataElementValidationResult); + } + } + + return issues; + } + + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ) + { + // DataElementValidator did not previously implement incremental validation, so we always return false + return Task.FromResult(false); + } +} diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs new file mode 100644 index 000000000..2e85bbd25 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs @@ -0,0 +1,88 @@ +namespace Altinn.App.Core.Features.Validation.Wrappers; + +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +/// +/// Wrap the old interface to the new interface. +/// +internal class FormDataValidatorWrapper : IValidator +{ + private readonly IFormDataValidator _formDataValidator; + private readonly string _taskId; + private readonly List _dataTypes; + + public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string taskId, List dataTypes) + { + _formDataValidator = formDataValidator; + _taskId = taskId; + _dataTypes = dataTypes; + } + + /// + public string TaskId => _taskId; + + /// + public string ValidationSource => _formDataValidator.ValidationSource; + + /// + /// Run all legacy instances for the given . + /// + public async Task> Validate( + Instance instance, + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor + ) + { + var issues = new List(); + var validateAllElements = _formDataValidator.DataType == "*"; + foreach (var dataElement in instance.Data) + { + if (!validateAllElements && _formDataValidator.DataType != dataElement.DataType) + { + continue; + } + + var data = await instanceDataAccessor.Get(dataElement); + var dataElementValidationResult = await _formDataValidator.ValidateFormData( + instance, + dataElement, + data, + language + ); + issues.AddRange(dataElementValidationResult); + } + + return issues; + } + + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ) + { + try + { + foreach (var change in changes) + { + if ( + (_formDataValidator.DataType == "*" || _formDataValidator.DataType == change.DataElement.DataType) + && _formDataValidator.HasRelevantChanges(change.CurrentValue, change.PreviousValue) + ) + { + return Task.FromResult(true); + } + } + + return Task.FromResult(false); + } + catch (Exception e) + { + return Task.FromException(e); + } + } +} diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs new file mode 100644 index 000000000..3b0221443 --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs @@ -0,0 +1,49 @@ +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation.Wrappers; + +/// +/// Wrap the old interface to the new interface. +/// +internal class TaskValidatorWrapper : IValidator +{ + private readonly ITaskValidator _taskValidator; + + /// + /// Constructor that wraps an + /// + public TaskValidatorWrapper(ITaskValidator taskValidator) + { + _taskValidator = taskValidator; + } + + /// + public string TaskId => _taskValidator.TaskId; + + /// + public string ValidationSource => _taskValidator.ValidationSource; + + /// + public Task> Validate( + Instance instance, + string taskId, + string? language, + IInstanceDataAccessor instanceDataAccessor + ) + { + return _taskValidator.ValidateTask(instance, taskId, language); + } + + /// + public Task HasRelevantChanges( + Instance instance, + string taskId, + List changes, + IInstanceDataAccessor instanceDataAccessor + ) + { + // TaskValidator did not previously implement incremental validation, so we always return false + return Task.FromResult(false); + } +} diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/TextClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/TextClient.cs index d35eba899..31b6e06fd 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/TextClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/TextClient.cs @@ -14,7 +14,7 @@ namespace Altinn.App.Core.Infrastructure.Clients.Storage; /// -/// A client forretrieving text resources from Altinn Platform. +/// A client for retrieving text resources from Altinn Platform. /// [Obsolete("Use IAppResources.GetTexts() instead")] public class TextClient : IText diff --git a/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs index 61d472aa4..7680f6bba 100644 --- a/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs @@ -1,51 +1,47 @@ using System.Collections.Concurrent; using System.Globalization; +using Altinn.App.Core.Features; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; using Altinn.Platform.Storage.Interface.Models; -using Microsoft.AspNetCore.Http; -using Microsoft.Extensions.DependencyInjection; namespace Altinn.App.Core.Internal.Data; /// /// Class that caches form data to avoid multiple calls to the data service for a single validation /// -/// Must be registered as a scoped service in DI container +/// Do not add this to the DI container, as it should only be created explicitly because of data leak potential. /// -internal sealed class CachedFormDataAccessor : ICachedFormDataAccessor +internal sealed class CachedInstanceDataAccessor : IInstanceDataAccessor { + private readonly string _org; + private readonly string _app; + private readonly Guid _instanceGuid; + private readonly int _instanceOwnerPartyId; private readonly IDataClient _dataClient; private readonly IAppMetadata _appMetadata; private readonly IAppModel _appModel; - private readonly IHttpContextAccessor _contextAccessor; - private readonly string _requestIdentifier; private readonly LazyCache _cache = new(); - public CachedFormDataAccessor( + public CachedInstanceDataAccessor( + Instance instance, IDataClient dataClient, IAppMetadata appMetadata, - IAppModel appModel, - IHttpContextAccessor contextAccessor + IAppModel appModel ) { + _org = instance.Org; + _app = instance.AppId.Split("/")[1]; + _instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); + _instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture); _dataClient = dataClient; _appMetadata = appMetadata; _appModel = appModel; - _contextAccessor = contextAccessor; - ArgumentNullException.ThrowIfNull(_contextAccessor.HttpContext); - _requestIdentifier = _contextAccessor.HttpContext.TraceIdentifier; } /// - public async Task Get(Instance instance, DataElement dataElement) + public async Task Get(DataElement dataElement) { - // Be completly sure that the cache is only used in a single http request - if (_requestIdentifier != _contextAccessor.HttpContext?.TraceIdentifier) - { - throw new Exception("Cache can only be used in a single http request"); - } - return await _cache.GetOrCreate( dataElement.Id, async _ => @@ -59,15 +55,19 @@ public async Task Get(Instance instance, DataElement dataElement) if (dataType.AppLogic?.ClassRef != null) { - return await GetFormData(instance, dataElement, dataType); + return await GetFormData(dataElement, dataType); } - return await GetBinaryData(instance, dataElement); + return await GetBinaryData(dataElement); } ); } - /// + /// + /// Add data to the cache, so that it won't be fetched again + /// + /// + /// public void Set(DataElement dataElement, object data) { _cache.Set(dataElement.Id, data); @@ -86,64 +86,53 @@ private sealed class LazyCache public async Task GetOrCreate(TKey key, Func> valueFactory) { - return await _cache.GetOrAdd(key, innerKey => new Lazy>(() => valueFactory(innerKey))).Value; + Task task; + lock (_cache) + { + task = _cache.GetOrAdd(key, innerKey => new Lazy>(() => valueFactory(innerKey))).Value; + } + ; + return await task; } public void Set(TKey key, TValue data) { - if (!_cache.TryAdd(key, new Lazy>(Task.FromResult(data)))) + lock (_cache) { - var existing = _cache[key]; - if ( - existing.IsValueCreated - && existing.Value.IsCompletedSuccessfully - && data.Equals(existing.Value.Result) - ) - { - // We are trying to set the same value again, so we can just ignore this - return; - } - - throw new InvalidOperationException($"Key {key} already exists in cache"); + _cache.AddOrUpdate( + key, + _ => new Lazy>(Task.FromResult(data)), + (_, _) => new Lazy>(Task.FromResult(data)) + ); } } } - private async Task GetBinaryData(Instance instance, DataElement dataElement) + private async Task GetBinaryData(DataElement dataElement) { - var instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); - var app = instance.AppId.Split("/")[1]; - var instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture); + ; var data = await _dataClient.GetBinaryData( - instance.Org, - app, - instanceOwnerPartyId, - instanceGuid, + _org, + _app, + _instanceOwnerPartyId, + _instanceGuid, Guid.Parse(dataElement.Id) ); return data; } - private async Task GetFormData(Instance instance, DataElement dataElement, DataType dataType) + private async Task GetFormData(DataElement dataElement, DataType dataType) { var modelType = _appModel.GetModelType(dataType.AppLogic.ClassRef); - var instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); - var app = instance.AppId.Split("/")[1]; - var instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture); var data = await _dataClient.GetFormData( - instanceGuid, + _instanceGuid, modelType, - instance.Org, - app, - instanceOwnerPartyId, + _org, + _app, + _instanceOwnerPartyId, Guid.Parse(dataElement.Id) ); return data; } - - internal static void Register(IServiceCollection services) - { - services.AddScoped(); - } } diff --git a/src/Altinn.App.Core/Internal/Data/ICachedFormDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/ICachedFormDataAccessor.cs deleted file mode 100644 index c2a9d09d4..000000000 --- a/src/Altinn.App.Core/Internal/Data/ICachedFormDataAccessor.cs +++ /dev/null @@ -1,21 +0,0 @@ -using Altinn.Platform.Storage.Interface.Models; - -namespace Altinn.App.Core.Internal.Data; - -/// -/// Use this in your validators, dataProcessors to get form data from the cache -/// -/// Note that this is a scoped service and can't be used in singleton or transient services -/// -public interface ICachedFormDataAccessor -{ - /// - /// Get the deserialized data for a given data element - /// - Task Get(Instance instance, DataElement dataElement); - - /// - /// In PATCH requests we need to use the new object for the uploaded data element, instead of fetching from - /// - void Set(DataElement dataElement, object data); -} diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs index ab613598f..49ecb4e1d 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluator.cs @@ -171,7 +171,6 @@ ComponentContext context Field = field.Field, Description = $"{field.Field} is required in component with id {context.Component.Id}", Code = "required", - Source = ValidationIssueSources.Required } ); } diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index e4217b455..2ee87794d 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -1,5 +1,6 @@ using System.Diagnostics; using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; using Altinn.App.Core.Helpers.DataModel; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.Data; @@ -16,7 +17,7 @@ public class LayoutEvaluatorStateInitializer : ILayoutEvaluatorStateInitializer // Dependency injection properties (set in ctor) private readonly IAppResources _appResources; private readonly FrontEndSettings _frontEndSettings; - private readonly ICachedFormDataAccessor _dataAccessor; + private readonly IInstanceDataAccessor _dataAccessor; /// /// Constructor with services from dependency injection @@ -24,7 +25,7 @@ public class LayoutEvaluatorStateInitializer : ILayoutEvaluatorStateInitializer public LayoutEvaluatorStateInitializer( IAppResources appResources, IOptions frontEndSettings, - ICachedFormDataAccessor dataAccessor + IInstanceDataAccessor dataAccessor ) { _appResources = appResources; @@ -80,9 +81,7 @@ public async Task Init( dataTasks.AddRange( instance .Data.Where(dataElement => dataElement.DataType == dataType) - .Select(async dataElement => - KeyValuePair.Create(dataElement, await _dataAccessor.Get(instance, dataElement)) - ) + .Select(async dataElement => KeyValuePair.Create(dataElement, await _dataAccessor.Get(dataElement))) ); } diff --git a/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs b/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs index f434d7698..bdcc8f34b 100644 --- a/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs +++ b/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs @@ -10,10 +10,10 @@ public class DataPatchResult /// /// The validation issues that were found during the patch operation. /// - public required Dictionary> ValidationIssues { get; init; } + public required Dictionary> ValidationIssues { get; init; } /// /// The current data model after the patch operation. /// - public required object NewDataModel { get; init; } + public required Dictionary NewDataModels { get; init; } } diff --git a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs b/src/Altinn.App.Core/Internal/Patch/IPatchService.cs index 6a0e74869..2e860b7c6 100644 --- a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/IPatchService.cs @@ -14,18 +14,14 @@ public interface IPatchService /// Applies a patch to a Form Data element /// /// - /// - /// - /// + /// /// /// /// - public Task> ApplyPatch( + Task> ApplyPatches( Instance instance, - DataType dataType, - DataElement dataElement, - JsonPatch jsonPatch, + Dictionary patches, string? language, - List? ignoredValidators = null + List? ignoredValidators ); } diff --git a/src/Altinn.App.Core/Internal/Patch/PatchService.cs b/src/Altinn.App.Core/Internal/Patch/PatchService.cs index ec15c56dd..1e01ed0bd 100644 --- a/src/Altinn.App.Core/Internal/Patch/PatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/PatchService.cs @@ -18,7 +18,7 @@ namespace Altinn.App.Core.Internal.Patch; /// /// Service for applying patches to form data elements /// -public class PatchService : IPatchService +internal class PatchService : IPatchService { private readonly IAppMetadata _appMetadata; private readonly IDataClient _dataClient; @@ -33,12 +33,6 @@ public class PatchService : IPatchService /// /// Creates a new instance of the class /// - /// - /// - /// - /// - /// - /// public PatchService( IAppMetadata appMetadata, IDataClient dataClient, @@ -57,96 +51,121 @@ public PatchService( } /// - public async Task> ApplyPatch( + public async Task> ApplyPatches( Instance instance, - DataType dataType, - DataElement dataElement, - JsonPatch jsonPatch, + Dictionary patches, string? language, - List? ignoredValidators = null + List? ignoredValidators ) { using var activity = _telemetry?.StartDataPatchActivity(instance); - InstanceIdentifier instanceIdentifier = new InstanceIdentifier(instance); + InstanceIdentifier instanceIdentifier = new(instance); AppIdentifier appIdentifier = (await _appMetadata.GetApplicationMetadata()).AppIdentifier; - var modelType = _appModel.GetModelType(dataType.AppLogic.ClassRef); - var oldModel = await _dataClient.GetFormData( - instanceIdentifier.InstanceGuid, - modelType, - appIdentifier.Org, - appIdentifier.App, - instanceIdentifier.InstanceOwnerPartyId, - Guid.Parse(dataElement.Id) - ); - var oldModelNode = JsonSerializer.SerializeToNode(oldModel); - var patchResult = jsonPatch.Apply(oldModelNode); - var telemetryPatchResult = ( - patchResult.IsSuccess ? Telemetry.Data.PatchResult.Success : Telemetry.Data.PatchResult.Error - ); - activity?.SetTag(InternalLabels.Result, telemetryPatchResult.ToStringFast()); - _telemetry?.DataPatched(telemetryPatchResult); + var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel); + var changes = new List(); - if (!patchResult.IsSuccess) + foreach (var (dataElementId, jsonPatch) in patches) { - bool testOperationFailed = patchResult.Error.Contains("is not equal to the indicated value."); - return new DataPatchError() + var dataElement = instance.Data.Find(d => d.Id == dataElementId.ToString()); + if (dataElement is null) { - Title = testOperationFailed ? "Precondition in patch failed" : "Patch Operation Failed", - Detail = patchResult.Error, - ErrorType = testOperationFailed - ? DataPatchErrorType.PatchTestFailed - : DataPatchErrorType.DeserializationFailed, - Extensions = new Dictionary() + return new DataPatchError() { - { "previousModel", oldModel }, - { "patchOperationIndex", patchResult.Operation }, - } - }; - } + Title = "Unknown data element to patch", + Detail = $"Data element with id {dataElementId} not found in instance", + }; + } - var result = DeserializeModel(oldModel.GetType(), patchResult.Result); - if (!result.Success) - { - return new DataPatchError() + var oldModel = await dataAccessor.Get(dataElement); + var oldModelNode = JsonSerializer.SerializeToNode(oldModel); + var patchResult = jsonPatch.Apply(oldModelNode); + + if (!patchResult.IsSuccess) { - Title = "Patch operation did not deserialize", - Detail = result.Error, - ErrorType = DataPatchErrorType.DeserializationFailed - }; - } - Guid dataElementId = Guid.Parse(dataElement.Id); - foreach (var dataProcessor in _dataProcessors) - { - await dataProcessor.ProcessDataWrite(instance, dataElementId, result.Ok, oldModel, language); - } + bool testOperationFailed = patchResult.Error.Contains("is not equal to the indicated value."); + return new DataPatchError() + { + Title = testOperationFailed ? "Precondition in patch failed" : "Patch Operation Failed", + Detail = patchResult.Error, + ErrorType = testOperationFailed + ? DataPatchErrorType.PatchTestFailed + : DataPatchErrorType.DeserializationFailed, + Extensions = new Dictionary() + { + { "previousModel", oldModel }, + { "patchOperationIndex", patchResult.Operation }, + } + }; + } + + var newModelResult = DeserializeModel(oldModel.GetType(), patchResult.Result); + if (!newModelResult.Success) + { + return new DataPatchError() + { + Title = "Patch operation did not deserialize", + Detail = newModelResult.Error, + ErrorType = DataPatchErrorType.DeserializationFailed + }; + } + var newModel = newModelResult.Ok; - ObjectUtils.InitializeAltinnRowId(result.Ok); - ObjectUtils.PrepareModelForXmlStorage(result.Ok); + foreach (var dataProcessor in _dataProcessors) + { + using var processWriteActivity = _telemetry?.StartDataProcessWriteActivity(dataProcessor); + try + { + // TODO: Create new dataProcessor interface that takes multiple models at the same time. + await dataProcessor.ProcessDataWrite(instance, dataElementId, newModel, oldModel, language); + } + catch (Exception e) + { + processWriteActivity?.Errored(e); + throw; + } + } + ObjectUtils.InitializeAltinnRowId(newModel); + ObjectUtils.PrepareModelForXmlStorage(newModel); + changes.Add( + new DataElementChange + { + DataElement = dataElement, + PreviousValue = oldModel, + CurrentValue = newModel, + } + ); + + // save form data to storage + await _dataClient.UpdateData( + newModel, + instanceIdentifier.InstanceGuid, + newModel.GetType(), + appIdentifier.Org, + appIdentifier.App, + instanceIdentifier.InstanceOwnerPartyId, + dataElementId + ); + + // Ensure that validation runs on the modified model. + dataAccessor.Set(dataElement, newModel); + } - var validationIssues = await _validationService.ValidateFormData( + var validationIssues = await _validationService.ValidateIncrementalFormData( instance, - dataElement, - dataType, - result.Ok, - oldModel, + instance.Process.CurrentTask.ElementId, + changes, + dataAccessor, ignoredValidators, language ); - // Save Formdata to database - await _dataClient.UpdateData( - result.Ok, - instanceIdentifier.InstanceGuid, - modelType, - appIdentifier.Org, - appIdentifier.App, - instanceIdentifier.InstanceOwnerPartyId, - dataElementId - ); - - return new DataPatchResult { NewDataModel = result.Ok, ValidationIssues = validationIssues }; + return new DataPatchResult + { + NewDataModels = changes.ToDictionary(c => Guid.Parse(c.DataElement.Id), c => c.CurrentValue), + ValidationIssues = validationIssues + }; } private static ServiceResult DeserializeModel(Type type, JsonNode? patchResult) diff --git a/src/Altinn.App.Core/Internal/Validation/IValidationService.cs b/src/Altinn.App.Core/Internal/Validation/IValidationService.cs index f63ee5c14..77cb50be8 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidationService.cs @@ -20,52 +20,31 @@ public interface IValidationService /// /// The instance to validate /// instance.Process?.CurrentTask?.ElementId + /// Accessor for instance data to be validated /// The language to run validations in /// List of validation issues for this data element - Task> ValidateInstanceAtTask(Instance instance, string taskId, string? language); - - /// - /// Validate a single data element regardless of whether it has AppLogic (eg. datamodel) or not. - /// - /// - /// 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 - /// The language to run validations in - /// List of validation issues for this data element - Task> ValidateDataElement( + Task> ValidateInstanceAtTask( Instance instance, - DataElement dataElement, - DataType dataType, + string taskId, + IInstanceDataAccessor dataAccessor, string? language ); /// - /// 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 - /// The previous data so that validators can know if they need to run again with - /// List validators that should not be run (for incremental validation). Typically known validators that frontend knows how to replicate - /// The language to run validations in - /// A dictionary containing lists of validation issues grouped by and/or - Task>> ValidateFormData( + /// + /// + /// + /// List of changed with both previous and next + /// + /// + /// + public Task>> ValidateIncrementalFormData( Instance instance, - DataElement dataElement, - DataType dataType, - object data, - object? previousData, + string taskId, + List changes, + IInstanceDataAccessor dataAccessor, List? ignoredValidators, string? language ); diff --git a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs index 1a677b199..c469b7b12 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs @@ -1,4 +1,10 @@ +using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Features.Validation.Wrappers; +using Altinn.App.Core.Internal.App; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.Options; namespace Altinn.App.Core.Internal.Validation; @@ -10,17 +16,7 @@ public interface IValidatorFactory /// /// Gets all task validators for a given task. /// - public IEnumerable GetTaskValidators(string taskId); - - /// - /// Gets all data element validators for a given data element. - /// - public IEnumerable GetDataElementValidators(string dataTypeId); - - /// - /// Gets all form data validators for a given data element. - /// - public IEnumerable GetFormDataValidators(string dataTypeId); + public IEnumerable GetValidators(string taskId); } /// @@ -29,38 +25,124 @@ public interface IValidatorFactory public class ValidatorFactory : IValidatorFactory { private readonly IEnumerable _taskValidators; + private readonly IOptions _generalSettings; private readonly IEnumerable _dataElementValidators; private readonly IEnumerable _formDataValidators; + private readonly IEnumerable _validators; +#pragma warning disable CS0618 // Type or member is obsolete + private readonly IEnumerable _instanceValidators; +#pragma warning restore CS0618 // Type or member is obsolete + private readonly IAppMetadata _appMetadata; /// /// Initializes a new instance of the class. /// public ValidatorFactory( IEnumerable taskValidators, + IOptions generalSettings, IEnumerable dataElementValidators, - IEnumerable formDataValidators + IEnumerable formDataValidators, + IEnumerable validators, +#pragma warning disable CS0618 // Type or member is obsolete + IEnumerable instanceValidators, +#pragma warning restore CS0618 // Type or member is obsolete + IAppMetadata appMetadata ) { _taskValidators = taskValidators; + _generalSettings = generalSettings; _dataElementValidators = dataElementValidators; _formDataValidators = formDataValidators; + _validators = validators; + _instanceValidators = instanceValidators; + _appMetadata = appMetadata; } - /// - public IEnumerable GetTaskValidators(string taskId) + private IEnumerable GetTaskValidators(string taskId) { return _taskValidators.Where(tv => tv.TaskId == "*" || tv.TaskId == taskId); } - /// - public IEnumerable GetDataElementValidators(string dataTypeId) + private IEnumerable GetDataElementValidators(string taskId, List dataTypes) + { + foreach (var dataElementValidator in _dataElementValidators) + { + if (dataElementValidator.DataType == "*") + { + yield return dataElementValidator; + } + else + { + var dataType = dataTypes.Find(d => d.Id == dataElementValidator.DataType); + if (dataType is null) + { + throw new InvalidOperationException( + $"DataType {dataElementValidator.DataType} from {dataElementValidator.ValidationSource} not found in dataTypes from applicationmetadata" + ); + } + if (dataType.TaskId == taskId) + { + yield return dataElementValidator; + } + } + } + } + + private IEnumerable GetFormDataValidators(string taskId, List dataTypes) { - return _dataElementValidators.Where(dev => dev.DataType == "*" || dev.DataType == dataTypeId); + foreach (var formDataValidator in _formDataValidators) + { + if (formDataValidator.DataType == "*") + { + yield return formDataValidator; + } + else + { + var dataType = dataTypes.Find(d => d.Id == formDataValidator.DataType); + if (dataType is null) + { + throw new InvalidOperationException( + $"DataType {formDataValidator.DataType} from {formDataValidator.ValidationSource} not found in dataTypes from applicationmetadata" + ); + } + if (dataType.TaskId == taskId) + { + yield return formDataValidator; + } + } + } } - /// - public IEnumerable GetFormDataValidators(string dataTypeId) + /// + /// Get all validators for a given task. Wrap , and + /// so that they behave as . + /// + public IEnumerable GetValidators(string taskId) { - return _formDataValidators.Where(fdv => fdv.DataType == "*" || fdv.DataType == dataTypeId); + var validators = new List(); + // add new style validators + validators.AddRange(_validators); + // add legacy task validators, data element validators and form data validators + validators.AddRange(GetTaskValidators(taskId).Select(tv => new TaskValidatorWrapper(tv))); + var dataTypes = _appMetadata.GetApplicationMetadata().Result.DataTypes; + + validators.AddRange( + GetDataElementValidators(taskId, dataTypes) + .Select(dev => new DataElementValidatorWrapper(dev, taskId, dataTypes)) + ); + validators.AddRange( + GetFormDataValidators(taskId, dataTypes).Select(fdv => new FormDataValidatorWrapper(fdv, taskId, dataTypes)) + ); + + // add legacy instance validators wrapped in IValidator wrappers + foreach (var instanceValidator in _instanceValidators) + { + validators.Add(new LegacyIInstanceValidatorTaskValidator(_generalSettings, instanceValidator)); + validators.Add( + new LegacyIInstanceValidatorFormDataValidator(_generalSettings, instanceValidator, _appMetadata) + ); + } + + return validators; } } diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index 748c8f489..33601a4b1 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -17,251 +17,144 @@ public class ValidationService : IValidationService private readonly IAppMetadata _appMetadata; private readonly ILogger _logger; private readonly Telemetry? _telemetry; - private readonly ICachedFormDataAccessor _formDataCache; /// /// Constructor with DI services /// public ValidationService( IValidatorFactory validatorFactory, - IDataClient dataClient, - IAppModel appModel, IAppMetadata appMetadata, ILogger logger, - ICachedFormDataAccessor formDataCache, Telemetry? telemetry = null ) { _validatorFactory = validatorFactory; _appMetadata = appMetadata; _logger = logger; - _formDataCache = formDataCache; _telemetry = telemetry; } /// - public async Task> ValidateInstanceAtTask(Instance instance, string taskId, string? language) - { - ArgumentNullException.ThrowIfNull(instance); - ArgumentNullException.ThrowIfNull(taskId); - - using var activity = _telemetry?.StartValidateInstanceAtTaskActivity(instance, taskId); - - // Run task validations (but don't await yet) - Task[]> taskIssuesTask = RunTaskValidators(instance, taskId, language); - - // Get list of data elements for the task - var application = await _appMetadata.GetApplicationMetadata(); - var dataTypesForTask = application.DataTypes.Where(dt => dt.TaskId == taskId).ToList(); - var dataElementsToValidate = instance - .Data.Where(de => dataTypesForTask.Exists(dt => dt.Id == de.DataType)) - .ToArray(); - // Run ValidateDataElement for each data element (but don't await yet) - var dataIssuesTask = Task.WhenAll( - dataElementsToValidate.Select(dataElement => - ValidateDataElement( - instance, - dataElement, - dataTypesForTask.First(dt => dt.Id == dataElement.DataType), - language - ) - ) - ); - - var lists = await Task.WhenAll(taskIssuesTask, dataIssuesTask); - // Flatten the list of lists to a single list of issues - return lists.SelectMany(x => x.SelectMany(y => y)).ToList(); - } - - private Task[]> RunTaskValidators(Instance instance, string taskId, string? language) - { - var taskValidators = _validatorFactory.GetTaskValidators(taskId); - - return Task.WhenAll( - taskValidators.Select(async v => - { - using var activity = _telemetry?.StartRunTaskValidatorActivity(v); - try - { - _logger.LogDebug( - "Start running validator {ValidatorName} on task {TaskId} in instance {InstanceId}", - v.ValidationSource, - taskId, - instance.Id - ); - var issues = await v.ValidateTask(instance, taskId, language); - issues.ForEach(i => i.Source = v.ValidationSource); // Ensure that the source is set to the validator source - return issues; - } - catch (Exception e) - { - _logger.LogError( - e, - "Error while running validator {ValidatorName} on task {TaskId} in instance {InstanceId}", - v.ValidationSource, - taskId, - instance.Id - ); - activity?.Errored(e); - throw; - } - }) - ); - } - - /// - public async Task> ValidateDataElement( + public async Task> ValidateInstanceAtTask( Instance instance, - DataElement dataElement, - DataType dataType, + string taskId, + IInstanceDataAccessor dataAccessor, string? language ) { ArgumentNullException.ThrowIfNull(instance); - ArgumentNullException.ThrowIfNull(dataElement); - ArgumentNullException.ThrowIfNull(dataElement.DataType); - - using var activity = _telemetry?.StartValidateDataElementActivity(instance, dataElement); + ArgumentNullException.ThrowIfNull(taskId); - // Get both keyed and non-keyed validators for the data type - Task[]> dataElementsIssuesTask = RunDataElementValidators( - instance, - dataElement, - dataType, - language - ); + using var activity = _telemetry?.StartValidateInstanceAtTaskActivity(instance, taskId); - // Run extra validation on form data elements with app logic - if (dataType.AppLogic?.ClassRef is not null) + // Run task validations (but don't await yet) + var validators = _validatorFactory.GetValidators(taskId); + var validationTasks = validators.Select(async v => { - var data = await _formDataCache.Get(instance, dataElement); - var formDataIssuesDictionary = await ValidateFormData( - instance, - dataElement, - dataType, - data, - previousData: null, - ignoredValidators: null, - language - ); - - return (await dataElementsIssuesTask) - .SelectMany(x => x) - .Concat(formDataIssuesDictionary.SelectMany(kv => kv.Value)) - .ToList(); - } - - return (await dataElementsIssuesTask).SelectMany(x => x).ToList(); - } - - private Task[]> RunDataElementValidators( - Instance instance, - DataElement dataElement, - DataType dataType, - string? language - ) - { - var validators = _validatorFactory.GetDataElementValidators(dataType.Id); - - var dataElementsIssuesTask = Task.WhenAll( - validators.Select(async v => + using var validatorActivity = _telemetry?.StartRunValidatorActivity(v); + try { - using var activity = _telemetry?.StartRunDataElementValidatorActivity(v); - try - { - _logger.LogDebug( - "Start running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", - v.ValidationSource, - dataElement.DataType, - dataElement.Id, - instance.Id - ); - var issues = await v.ValidateDataElement(instance, dataElement, dataType, language); - issues.ForEach(i => i.Source = v.ValidationSource); // Ensure that the source is set to the validator source - return issues; - } - catch (Exception e) - { - _logger.LogError( - e, - "Error while running validator {validatorName} on {dataType} for data element {dataElementId} in instance {instanceId}", - v.ValidationSource, - dataElement.DataType, - dataElement.Id, - instance.Id - ); - activity?.Errored(e); - throw; - } - }) - ); + var issues = await v.Validate(instance, taskId, language, dataAccessor); + return KeyValuePair.Create( + v.ValidationSource, + issues.Select(issue => ValidationIssueWithSource.FromIssue(issue, v.ValidationSource)) + ); + } + catch (Exception e) + { + _logger.LogError( + e, + "Error while running validator {validatorName} for task {taskId} on instance {instanceId}", + v.ValidationSource, + taskId, + instance.Id + ); + validatorActivity?.Errored(e); + throw; + } + }); + var lists = await Task.WhenAll(validationTasks); - return dataElementsIssuesTask; + // Flatten the list of lists to a single list of issues + return lists.SelectMany(x => x.Value).ToList(); } /// - public async Task>> ValidateFormData( + public async Task>> ValidateIncrementalFormData( Instance instance, - DataElement dataElement, - DataType dataType, - object data, - object? previousData, + string taskId, + List changes, + IInstanceDataAccessor dataAccessor, List? ignoredValidators, string? language ) { ArgumentNullException.ThrowIfNull(instance); - ArgumentNullException.ThrowIfNull(dataElement); - ArgumentNullException.ThrowIfNull(dataElement.DataType); - ArgumentNullException.ThrowIfNull(data); - - using var activity = _telemetry?.StartValidateFormDataActivity(instance, dataElement); + ArgumentNullException.ThrowIfNull(taskId); + ArgumentNullException.ThrowIfNull(changes); - // Set data from request instead of fetching the old data. - _formDataCache.Set(dataElement, data); + using var activity = _telemetry?.StartValidateIncrementalActivity(instance, taskId, changes); - // Locate the relevant data validator services from normal and keyed services - var dataValidators = _validatorFactory - .GetFormDataValidators(dataType.Id) - .Where(dv => ignoredValidators?.Contains(dv.ValidationSource) != true) // Filter out ignored validators - .Where(dv => previousData is null || dv.HasRelevantChanges(data, previousData)) + var validators = _validatorFactory + .GetValidators(taskId) + .Where(v => !(ignoredValidators?.Contains(v.ValidationSource) ?? false)) .ToArray(); - var validationTasks = dataValidators.Select(async v => + ThrowIfDuplicateValidators(validators, taskId); + + // Run task validations (but don't await yet) + var validationTasks = validators.Select(async validator => { - using var activity = _telemetry?.StartRunFormDataValidatorActivity(v); + using var validatorActivity = _telemetry?.StartRunValidatorActivity(validator); try { - _logger.LogDebug( - "Start running validator {ValidatorName} on {DataType} for data element {DataElementId} in instance {InstanceId}", - v.ValidationSource, - dataElement.DataType, - dataElement.Id, - instance.Id - ); - var issues = await v.ValidateFormData(instance, dataElement, data, language); - issues.ForEach(i => i.Source = v.ValidationSource); // Ensure that the Source is set to the ValidatorSource - return issues; + var hasRelevantChanges = await validator.HasRelevantChanges(instance, taskId, changes, dataAccessor); + validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorRelevantChanges, hasRelevantChanges); + if (hasRelevantChanges) + { + var issues = await validator.Validate(instance, taskId, language, dataAccessor); + var issuesWithSource = issues + .Select(i => ValidationIssueWithSource.FromIssue(i, validator.ValidationSource)) + .ToList(); + return new KeyValuePair?>( + validator.ValidationSource, + issuesWithSource + ); + } + + return new KeyValuePair?>(); } catch (Exception e) { _logger.LogError( e, - "Error while running validator {ValidatorName} on {DataType} for data element {DataElementId} in instance {InstanceId}", - v.ValidationSource, - dataElement.DataType, - dataElement.Id, + "Error while running validator {validatorName} on task {taskId} in instance {instanceId}", + validator.GetType().Name, + taskId, instance.Id ); - activity?.Errored(e); + validatorActivity?.Errored(e); throw; } }); - var validationSources = dataValidators.Select(d => d.ValidationSource).ToList(); + var lists = await Task.WhenAll(validationTasks); - var issuesLists = await Task.WhenAll(validationTasks); + // ! Value is null if no relevant changes. Filter out these before return with ! because ofType don't filter nullables. + return lists.Where(k => k.Value is not null).ToDictionary(kv => kv.Key, kv => kv.Value!); + } - return validationSources.Zip(issuesLists).ToDictionary(kv => kv.First, kv => kv.Second); + private void ThrowIfDuplicateValidators(IValidator[] validators, string taskId) + { + var sourceNames = validators + .Select(v => v.ValidationSource) + .Distinct(StringComparer.InvariantCultureIgnoreCase); + if (sourceNames.Count() != validators.Length) + { + var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); + throw new InvalidOperationException( + $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{sources}" + ); + } } } diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs index e89effe44..130d959d7 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs @@ -54,6 +54,7 @@ public class ValidationIssue /// [JsonProperty(PropertyName = "code")] [JsonPropertyName("code")] + // TODO: Make this required for v9 public string? Code { get; set; } /// @@ -61,20 +62,16 @@ public class ValidationIssue /// [JsonProperty(PropertyName = "description")] [JsonPropertyName("description")] + // TODO: Make this required for v9 public string? Description { get; set; } /// /// The short name of the class that crated the message (set automatically after return of list) /// - /// - /// Intentionally not marked as "required", because it is set in - /// [JsonProperty(PropertyName = "source")] [JsonPropertyName("source")] -#nullable disable - public string Source { get; set; } - -#nullable restore + [Obsolete("Source is set automatically by the validation service. Setting it explicitly will be an error in v9")] + public string? Source { get; set; } /// /// The custom text key to use for the localized text in the frontend. diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssueSource.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssueSource.cs index 8f56fb15d..567a0b53e 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssueSource.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssueSource.cs @@ -29,4 +29,9 @@ public static class ValidationIssueSources /// Expression validation /// public static readonly string Expression = nameof(Expression); + + /// + /// Validation based on data annotations (json / xml schema) + /// + public static readonly string DataAnnotations = nameof(DataAnnotations); } diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs new file mode 100644 index 000000000..28d2de80a --- /dev/null +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs @@ -0,0 +1,90 @@ +using System.Text.Json.Serialization; + +namespace Altinn.App.Core.Models.Validation; + +/// +/// Represents a detailed message from validation. +/// +public class ValidationIssueWithSource +{ + /// + /// Converter function to create a from a and adding a source. + /// + public static ValidationIssueWithSource FromIssue(ValidationIssue issue, string source) + { + return new ValidationIssueWithSource + { + Severity = issue.Severity, + DataElementId = issue.DataElementId, + Field = issue.Field, + Code = issue.Code, + Description = issue.Description, + Source = source, + CustomTextKey = issue.CustomTextKey, + CustomTextParams = issue.CustomTextParams, + }; + } + + /// + /// The seriousness of the identified issue. + /// + /// + /// This property is serialized in json as a number + /// 1: Error (something needs to be fixed) + /// 2: Warning (does not prevent submission) + /// 3: Information (hint shown to the user) + /// 4: Fixed (obsolete, only used for v3 of frontend) + /// 5: Success (Inform the user that something was completed with success) + /// + [JsonPropertyName("severity")] + [JsonConverter(typeof(JsonNumberEnumConverter))] + public required ValidationIssueSeverity Severity { get; set; } + + /// + /// The unique id of the data element of a given instance with the identified issue. + /// + [JsonPropertyName("dataElementId")] + public string? DataElementId { get; set; } + + /// + /// A reference to a property the issue is about. + /// + [JsonPropertyName("field")] + public string? Field { get; set; } + + /// + /// A system readable identification of the type of issue. + /// Eg: + /// + [JsonPropertyName("code")] + public required string? Code { get; set; } + + /// + /// A human readable description of the issue. + /// + [JsonPropertyName("description")] + public required string? Description { get; set; } + + /// + /// The short name of the class that crated the message (set automatically after return of list) + /// + [JsonPropertyName("source")] + public required string Source { get; set; } + + /// + /// The custom text key to use for the localized text in the frontend. + /// + [JsonPropertyName("customTextKey")] + public string? CustomTextKey { get; set; } + + /// + /// might include some parameters (typically the field value, or some derived value) + /// that should be included in error message. + /// + /// + /// The localized text for the key might be "Date must be between {0} and {1}" + /// and the param will provide the dynamical range of allowable dates (eg teh reporting period) + /// + [JsonPropertyName("customTextParams")] + public List? CustomTextParams { get; set; } +} diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs index eddcdfc51..7b5d68306 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs @@ -52,7 +52,8 @@ public class DataControllerPatchTests : ApiTestBase, IClassFixture factory, ITestOutputHelper outputHelper) : base(factory, outputHelper) { - _formDataValidatorMock.Setup(v => v.DataType).Returns("Not a valid data type"); + _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); + _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs index a9418fb59..870092dc5 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs @@ -43,7 +43,8 @@ public class ProcessControllerTests : ApiTestBase, IClassFixture factory, ITestOutputHelper outputHelper) : base(factory, outputHelper) { - _formDataValidatorMock.Setup(v => v.DataType).Returns("Not a valid data type"); + _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); + _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs index 69094a4b8..7dceeba8b 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs @@ -1,9 +1,13 @@ using System.Net; using Altinn.App.Api.Controllers; +using Altinn.App.Core.Features; 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.Instances; using Altinn.App.Core.Internal.Validation; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; @@ -14,29 +18,41 @@ namespace Altinn.App.Api.Tests.Controllers; public class ValidateControllerTests { + private const string Org = "ttd"; + private const string App = "app"; + private const int InstanceOwnerPartyId = 1337; + private static readonly Guid _instanceId = Guid.NewGuid(); + + private readonly Mock _instanceMock = new(); + private readonly Mock _appMetadataMock = new(); + private readonly Mock _validationMock = new(); + private readonly Mock _dataClientMock = new(); + private readonly Mock _appModelMock = new(); + + public ValidateControllerTests() + { + _appMetadataMock + .Setup(a => a.GetApplicationMetadata()) + .ReturnsAsync(new ApplicationMetadata($"{Org}/{App}") { DataTypes = [] }); + } + [Fact] public async Task ValidateInstance_returns_NotFound_when_GetInstance_returns_null() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - Guid instanceId = new Guid(); - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(null!)); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); - var result = await validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId); + var result = await validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId); // Assert Assert.IsType(result); @@ -46,31 +62,26 @@ public async Task ValidateInstance_returns_NotFound_when_GetInstance_returns_nul public async Task ValidateInstance_throws_ValidationException_when_Instance_Process_is_null() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - var instanceId = Guid.NewGuid(); Instance instance = new Instance { Id = "instanceId", Process = null }; - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(instance)); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); // Assert var exception = await Assert.ThrowsAsync( - () => validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId) + () => validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId) ); Assert.Equal("Unable to validate instance without a started process.", exception.Message); } @@ -79,35 +90,28 @@ public async Task ValidateInstance_throws_ValidationException_when_Instance_Proc public async Task ValidateInstance_throws_ValidationException_when_Instance_Process_CurrentTask_is_null() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - var instanceId = Guid.NewGuid(); - Instance instance = new Instance { Id = "instanceId", Process = new ProcessState { CurrentTask = null } }; - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(instance)); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); // Assert var exception = await Assert.ThrowsAsync( - () => validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId) + () => validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId) ); Assert.Equal("Unable to validate instance without a started process.", exception.Message); } @@ -116,41 +120,46 @@ public async Task ValidateInstance_throws_ValidationException_when_Instance_Proc public async Task ValidateInstance_returns_OK_with_messages() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - var instanceId = Guid.NewGuid(); Instance instance = new Instance { - Id = "instanceId", + Id = $"{InstanceOwnerPartyId}/{_instanceId}", + InstanceOwner = new() { PartyId = InstanceOwnerPartyId.ToString() }, + Org = Org, + AppId = $"{Org}/{App}", + Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "dummy" } } }; - var validationResult = new List + var validationResult = new List() { - new ValidationIssue { Field = "dummy", Severity = ValidationIssueSeverity.Fixed } + new() + { + Code = "dummy", + Description = "dummy", + Field = "dummy", + Severity = ValidationIssueSeverity.Fixed, + Source = "dummy" + } }; - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(instance)); - validationMock - .Setup(v => v.ValidateInstanceAtTask(instance, "dummy", null)) - .Returns(Task.FromResult(validationResult)); + _validationMock + .Setup(v => v.ValidateInstanceAtTask(instance, "dummy", It.IsAny(), null)) + .ReturnsAsync(validationResult); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); - var result = await validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId); + var result = await validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId); // Assert result.Should().BeOfType().Which.Value.Should().BeEquivalentTo(validationResult); @@ -160,37 +169,35 @@ public async Task ValidateInstance_returns_OK_with_messages() public async Task ValidateInstance_returns_403_when_not_authorized() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - var instanceId = Guid.NewGuid(); - Instance instance = new Instance { - Id = "instanceId", + Id = $"{InstanceOwnerPartyId}/{_instanceId}", + InstanceOwner = new() { PartyId = InstanceOwnerPartyId.ToString() }, + Org = Org, + AppId = $"{Org}/{App}", Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "dummy" } } }; var updateProcessResult = new HttpResponseMessage(HttpStatusCode.Forbidden); PlatformHttpException exception = await PlatformHttpException.CreateAsync(updateProcessResult); - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(instance)); - validationMock.Setup(v => v.ValidateInstanceAtTask(instance, "dummy", null)).Throws(exception); + _validationMock + .Setup(v => v.ValidateInstanceAtTask(instance, "dummy", It.IsAny(), null)) + .Throws(exception); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); - var result = await validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId); + var result = await validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId); // Assert result.Should().BeOfType().Which.StatusCode.Should().Be(403); @@ -200,40 +207,38 @@ public async Task ValidateInstance_returns_403_when_not_authorized() public async Task ValidateInstance_throws_PlatformHttpException_when_not_403() { // Arrange - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - - const string org = "ttd"; - const string app = "app"; - const int instanceOwnerPartyId = 1337; - var instanceId = Guid.NewGuid(); - Instance instance = new Instance { - Id = "instanceId", + Id = $"{InstanceOwnerPartyId}/{_instanceId}", + InstanceOwner = new() { PartyId = InstanceOwnerPartyId.ToString() }, + Org = Org, + AppId = $"{Org}/{App}", Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "dummy" } } }; var updateProcessResult = new HttpResponseMessage(HttpStatusCode.BadRequest); PlatformHttpException exception = await PlatformHttpException.CreateAsync(updateProcessResult); - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerPartyId, instanceId)) + _instanceMock + .Setup(i => i.GetInstance(App, Org, InstanceOwnerPartyId, _instanceId)) .Returns(Task.FromResult(instance)); - validationMock.Setup(v => v.ValidateInstanceAtTask(instance, "dummy", null)).Throws(exception); + _validationMock + .Setup(v => v.ValidateInstanceAtTask(instance, "dummy", It.IsAny(), null)) + .Throws(exception); // Act var validateController = new ValidateController( - instanceMock.Object, - validationMock.Object, - appMetadataMock.Object + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object ); // Assert var thrownException = await Assert.ThrowsAsync( - () => validateController.ValidateInstance(org, app, instanceOwnerPartyId, instanceId) + () => validateController.ValidateInstance(Org, App, InstanceOwnerPartyId, _instanceId) ); Assert.Equal(exception, thrownException); } diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs index 688e1f35e..7d2f71663 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs @@ -1,7 +1,10 @@ using System.Collections; using Altinn.App.Api.Controllers; +using Altinn.App.Core.Features; 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.Instances; using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; @@ -44,7 +47,7 @@ public class TestScenariosData : IEnumerable }, new("thows_ValidationException_when_Application_DataTypes_is_empty") { - DataGuid = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", "D"), + DataGuid = _dataGuid, ReceivedInstance = new Instance { Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "1234" } }, @@ -55,10 +58,14 @@ public class TestScenariosData : IEnumerable }, new("adds_ValidationIssue_when_DataType_TaskId_does_not_match_CurrentTask_ElementId") { - InstanceId = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354ef", "D"), - DataGuid = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", "D"), + InstanceId = _instanceId, + DataGuid = _dataGuid, ReceivedInstance = new Instance { + AppId = $"{ValidationControllerValidateDataTests.Org}/{ValidationControllerValidateDataTests.App}", + Org = ValidationControllerValidateDataTests.Org, + Id = $"{ValidationControllerValidateDataTests.InstanceOwnerId}/{_instanceId}", + InstanceOwner = new() { PartyId = ValidationControllerValidateDataTests.InstanceOwnerId.ToString() }, Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "1234" } }, Data = new List { @@ -73,13 +80,13 @@ public class TestScenariosData : IEnumerable { DataTypes = new List { - new DataType { Id = "0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", TaskId = "1234" } + new DataType { Id = "0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", TaskId = "Task_1" } } }, - ReceivedValidationIssues = new List(), - ExpectedValidationIssues = new List + ReceivedValidationIssues = new List(), + ExpectedValidationIssues = new List { - new ValidationIssue + new() { Code = ValidationIssueCodes.DataElementCodes.DataElementValidatedAtWrongTask, Severity = ValidationIssueSeverity.Warning, @@ -89,17 +96,22 @@ public class TestScenariosData : IEnumerable new Dictionary>(), null, "nb" - ) - } + ), + Source = "source" + }, }, ExpectedResult = typeof(OkObjectResult) }, new("returns_ValidationIssues_from_ValidationService") { - InstanceId = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354ef", "D"), - DataGuid = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", "D"), + InstanceId = _instanceId, + DataGuid = _dataGuid, ReceivedInstance = new Instance { + AppId = $"{ValidationControllerValidateDataTests.Org}/{ValidationControllerValidateDataTests.App}", + Org = ValidationControllerValidateDataTests.Org, + Id = $"{ValidationControllerValidateDataTests.InstanceOwnerId}/{_instanceId}", + InstanceOwner = new() { PartyId = ValidationControllerValidateDataTests.InstanceOwnerId.ToString() }, Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "0fc98a23-fe31-4ef5-8fb9-dd3f479354cd" } @@ -117,29 +129,36 @@ public class TestScenariosData : IEnumerable { DataTypes = new List { - new DataType { Id = "0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", TaskId = "1234" } + new DataType { Id = "0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", TaskId = "Task_1" } } }, - ReceivedValidationIssues = new List + ReceivedValidationIssues = new List { - new ValidationIssue + new() { Code = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, - Severity = ValidationIssueSeverity.Fixed - } + Description = "dummy", + Severity = ValidationIssueSeverity.Fixed, + Source = "source" + }, }, - ExpectedValidationIssues = new List + ExpectedValidationIssues = new List { - new ValidationIssue + new() { Code = ValidationIssueCodes.DataElementCodes.DataElementTooLarge, - Severity = ValidationIssueSeverity.Fixed + Description = "dummy", + Severity = ValidationIssueSeverity.Fixed, + Source = "source" } }, ExpectedResult = typeof(OkObjectResult) } }; + private static readonly Guid _instanceId = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354ef", "D"); + private static readonly Guid _dataGuid = Guid.ParseExact("0fc98a23-fe31-4ef5-8fb9-dd3f479354cd", "D"); + public IEnumerator GetEnumerator() { List testData = new List(); @@ -159,24 +178,37 @@ IEnumerator IEnumerable.GetEnumerator() public class ValidationControllerValidateDataTests { + public const int InstanceOwnerId = 1337; + public const string App = "app-test"; + public const string Org = "ttd"; + private readonly Mock _instanceMock = new(MockBehavior.Strict); + private readonly Mock _appMetadataMock = new(MockBehavior.Strict); + private readonly Mock _validationMock = new(MockBehavior.Strict); + private readonly Mock _dataClientMock = new(MockBehavior.Strict); + private readonly Mock _appModelMock = new(MockBehavior.Strict); + [Theory] [ClassData(typeof(TestScenariosData))] public async Task TestValidateData(ValidateDataTestScenario testScenario) { // Arrange - const string org = "ttd"; - const string app = "app-test"; - const int instanceOwnerId = 1337; - var validateController = SetupController(app, org, instanceOwnerId, testScenario); + SetupMocks(App, Org, InstanceOwnerId, testScenario); + var validateController = new ValidateController( + _instanceMock.Object, + _validationMock.Object, + _appMetadataMock.Object, + _dataClientMock.Object, + _appModelMock.Object + ); // Act and Assert if (testScenario.ExpectedExceptionMessage == null) { var result = await validateController.ValidateData( - org, - app, - instanceOwnerId, + Org, + App, + InstanceOwnerId, testScenario.InstanceId, testScenario.DataGuid ); @@ -187,9 +219,9 @@ public async Task TestValidateData(ValidateDataTestScenario testScenario) var exception = await Assert.ThrowsAsync( () => validateController.ValidateData( - org, - app, - instanceOwnerId, + Org, + App, + InstanceOwnerId, testScenario.InstanceId, testScenario.DataGuid ) @@ -198,41 +230,14 @@ public async Task TestValidateData(ValidateDataTestScenario testScenario) } } - private static ValidateController SetupController( - string app, - string org, - int instanceOwnerId, - ValidateDataTestScenario testScenario - ) + private void SetupMocks(string app, string org, int instanceOwnerId, ValidateDataTestScenario testScenario) { - ( - 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, - int instanceOwnerId, - ValidateDataTestScenario testScenario - ) - { - var instanceMock = new Mock(); - var appMetadataMock = new Mock(); - var validationMock = new Mock(); - if (testScenario.ReceivedInstance != null) - { - instanceMock - .Setup(i => i.GetInstance(app, org, instanceOwnerId, testScenario.InstanceId)) - .Returns(Task.FromResult(testScenario.ReceivedInstance)); - } + _instanceMock + .Setup(i => i.GetInstance(app, org, instanceOwnerId, testScenario.InstanceId)) + .Returns(Task.FromResult(testScenario.ReceivedInstance)!); if (testScenario.ReceivedApplication != null) { - appMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(testScenario.ReceivedApplication); + _appMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(testScenario.ReceivedApplication); } if ( @@ -241,19 +246,17 @@ ValidateDataTestScenario testScenario && testScenario.ReceivedValidationIssues != null ) { - validationMock + _validationMock .Setup(v => - v.ValidateDataElement( + v.ValidateInstanceAtTask( testScenario.ReceivedInstance, - testScenario.ReceivedInstance.Data.First(), - testScenario.ReceivedApplication.DataTypes.First(), + "Task_1", + It.IsAny(), null ) ) - .Returns(Task.FromResult>(testScenario.ReceivedValidationIssues)); + .ReturnsAsync(testScenario.ReceivedValidationIssues); } - - return (instanceMock, appMetadataMock, validationMock); } } @@ -269,10 +272,10 @@ public ValidateDataTestScenario(string testScenarioName) public Guid DataGuid { get; init; } = Guid.NewGuid(); public Instance? ReceivedInstance { get; init; } public ApplicationMetadata? ReceivedApplication { get; init; } - public List? ReceivedValidationIssues { get; init; } + public List? ReceivedValidationIssues { get; init; } public string? ExpectedExceptionMessage { get; init; } public Type? ExpectedResult { get; init; } - public List? ExpectedValidationIssues { get; init; } + public List? ExpectedValidationIssues { get; init; } public override string ToString() { diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs index 9b991b5a9..a6d32bec2 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs @@ -44,7 +44,8 @@ ITestOutputHelper outputHelper ) : base(factory, outputHelper) { - _formDataValidatorMock.Setup(v => v.DataType).Returns("Not a valid data type"); + _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); + _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); diff --git a/test/Altinn.App.Api.Tests/OpenApi/swagger.json b/test/Altinn.App.Api.Tests/OpenApi/swagger.json index dab37d07b..e6a443069 100644 --- a/test/Altinn.App.Api.Tests/OpenApi/swagger.json +++ b/test/Altinn.App.Api.Tests/OpenApi/swagger.json @@ -587,6 +587,155 @@ } } } + }, + "patch": { + "tags": [ + "Data" + ], + "parameters": [ + { + "name": "org", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "app", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "instanceOwnerPartyId", + "in": "path", + "required": true, + "schema": { + "type": "integer", + "format": "int32" + } + }, + { + "name": "instanceGuid", + "in": "path", + "required": true, + "schema": { + "type": "string", + "format": "uuid" + } + }, + { + "name": "language", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "requestBody": { + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/DataPatchRequestMultiple" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/DataPatchRequestMultiple" + } + }, + "application/*+json": { + "schema": { + "$ref": "#/components/schemas/DataPatchRequestMultiple" + } + } + } + }, + "responses": { + "200": { + "description": "OK", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/DataPatchResponseMultiple" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/DataPatchResponseMultiple" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/DataPatchResponseMultiple" + } + } + } + }, + "409": { + "description": "Conflict", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/xml": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/xml": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + }, + "422": { + "description": "Unprocessable Content", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "application/xml": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + }, + "text/xml": { + "schema": { + "$ref": "#/components/schemas/ProblemDetails" + } + } + } + } + } } }, "/{org}/{app}/instances/{instanceOwnerPartyId}/{instanceGuid}/data/{dataGuid}": { @@ -4603,7 +4752,34 @@ ], "responses": { "200": { - "description": "OK" + "description": "OK", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + }, + "application/xml": { + "schema": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + }, + "text/xml": { + "schema": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + } + } } } } @@ -4669,7 +4845,8 @@ "200": { "description": "OK" } - } + }, + "deprecated": true } } }, @@ -5407,6 +5584,30 @@ }, "additionalProperties": false }, + "DataPatchRequestMultiple": { + "required": [ + "ignoredValidators", + "patches" + ], + "type": "object", + "properties": { + "patches": { + "type": "object", + "additionalProperties": { + "$ref": "#/components/schemas/JsonPatch" + }, + "nullable": true + }, + "ignoredValidators": { + "type": "array", + "items": { + "type": "string" + }, + "nullable": true + } + }, + "additionalProperties": false + }, "DataPatchResponse": { "required": [ "newDataModel", @@ -5419,7 +5620,7 @@ "additionalProperties": { "type": "array", "items": { - "$ref": "#/components/schemas/ValidationIssue" + "$ref": "#/components/schemas/ValidationIssueWithSource" } }, "nullable": true @@ -5430,6 +5631,31 @@ }, "additionalProperties": false }, + "DataPatchResponseMultiple": { + "required": [ + "newDataModels", + "validationIssues" + ], + "type": "object", + "properties": { + "validationIssues": { + "type": "object", + "additionalProperties": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + } + }, + "nullable": true + }, + "newDataModels": { + "type": "object", + "additionalProperties": { }, + "nullable": true + } + }, + "additionalProperties": false + }, "DataType": { "type": "object", "properties": { @@ -6636,7 +6862,7 @@ "additionalProperties": { "type": "array", "items": { - "$ref": "#/components/schemas/ValidationIssue" + "$ref": "#/components/schemas/ValidationIssueWithSource" } }, "nullable": true @@ -6674,9 +6900,24 @@ }, "additionalProperties": false }, - "ValidationIssue": { + "ValidationIssueSeverity": { + "enum": [ + 0, + 1, + 2, + 3, + 4, + 5 + ], + "type": "integer", + "format": "int32" + }, + "ValidationIssueWithSource": { "required": [ - "severity" + "code", + "description", + "severity", + "source" ], "type": "object", "properties": { @@ -6717,18 +6958,6 @@ }, "additionalProperties": false }, - "ValidationIssueSeverity": { - "enum": [ - 0, - 1, - 2, - 3, - 4, - 5 - ], - "type": "integer", - "format": "int32" - }, "ValidationStatus": { "type": "object", "properties": { diff --git a/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml b/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml index 4296f7966..fa07b4d00 100644 --- a/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml +++ b/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml @@ -356,6 +356,96 @@ paths: text/xml: schema: $ref: '#/components/schemas/DataElement' + patch: + tags: + - Data + parameters: + - name: org + in: path + required: true + schema: + type: string + - name: app + in: path + required: true + schema: + type: string + - name: instanceOwnerPartyId + in: path + required: true + schema: + type: integer + format: int32 + - name: instanceGuid + in: path + required: true + schema: + type: string + format: uuid + - name: language + in: query + schema: + type: string + requestBody: + content: + application/json: + schema: + $ref: '#/components/schemas/DataPatchRequestMultiple' + text/json: + schema: + $ref: '#/components/schemas/DataPatchRequestMultiple' + application/*+json: + schema: + $ref: '#/components/schemas/DataPatchRequestMultiple' + responses: + '200': + description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/DataPatchResponseMultiple' + application/json: + schema: + $ref: '#/components/schemas/DataPatchResponseMultiple' + text/json: + schema: + $ref: '#/components/schemas/DataPatchResponseMultiple' + '409': + description: Conflict + content: + text/plain: + schema: + $ref: '#/components/schemas/ProblemDetails' + application/json: + schema: + $ref: '#/components/schemas/ProblemDetails' + text/json: + schema: + $ref: '#/components/schemas/ProblemDetails' + application/xml: + schema: + $ref: '#/components/schemas/ProblemDetails' + text/xml: + schema: + $ref: '#/components/schemas/ProblemDetails' + '422': + description: Unprocessable Content + content: + text/plain: + schema: + $ref: '#/components/schemas/ProblemDetails' + application/json: + schema: + $ref: '#/components/schemas/ProblemDetails' + text/json: + schema: + $ref: '#/components/schemas/ProblemDetails' + application/xml: + schema: + $ref: '#/components/schemas/ProblemDetails' + text/xml: + schema: + $ref: '#/components/schemas/ProblemDetails' '/{org}/{app}/instances/{instanceOwnerPartyId}/{instanceGuid}/data/{dataGuid}': get: tags: @@ -2805,6 +2895,22 @@ paths: responses: '200': description: OK + content: + text/plain: + schema: + $ref: '#/components/schemas/ValidationIssueWithSource' + application/json: + schema: + $ref: '#/components/schemas/ValidationIssueWithSource' + text/json: + schema: + $ref: '#/components/schemas/ValidationIssueWithSource' + application/xml: + schema: + $ref: '#/components/schemas/ValidationIssueWithSource' + text/xml: + schema: + $ref: '#/components/schemas/ValidationIssueWithSource' '/{org}/{app}/instances/{instanceOwnerId}/{instanceId}/data/{dataGuid}/validate': get: tags: @@ -2845,6 +2951,7 @@ paths: responses: '200': description: OK + deprecated: true components: schemas: ActionError: @@ -3379,6 +3486,23 @@ components: type: string nullable: true additionalProperties: false + DataPatchRequestMultiple: + required: + - ignoredValidators + - patches + type: object + properties: + patches: + type: object + additionalProperties: + $ref: '#/components/schemas/JsonPatch' + nullable: true + ignoredValidators: + type: array + items: + type: string + nullable: true + additionalProperties: false DataPatchResponse: required: - newDataModel @@ -3390,11 +3514,29 @@ components: additionalProperties: type: array items: - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ValidationIssueWithSource' nullable: true newDataModel: nullable: true additionalProperties: false + DataPatchResponseMultiple: + required: + - newDataModels + - validationIssues + type: object + properties: + validationIssues: + type: object + additionalProperties: + type: array + items: + $ref: '#/components/schemas/ValidationIssueWithSource' + nullable: true + newDataModels: + type: object + additionalProperties: { } + nullable: true + additionalProperties: false DataType: type: object properties: @@ -4271,7 +4413,7 @@ components: additionalProperties: type: array items: - $ref: '#/components/schemas/ValidationIssue' + $ref: '#/components/schemas/ValidationIssueWithSource' nullable: true nullable: true clientActions: @@ -4295,9 +4437,22 @@ components: $ref: '#/components/schemas/KeyValueEntry' nullable: true additionalProperties: false - ValidationIssue: + ValidationIssueSeverity: + enum: + - 0 + - 1 + - 2 + - 3 + - 4 + - 5 + type: integer + format: int32 + ValidationIssueWithSource: required: + - code + - description - severity + - source type: object properties: severity: @@ -4326,16 +4481,6 @@ components: type: string nullable: true additionalProperties: false - ValidationIssueSeverity: - enum: - - 0 - - 1 - - 2 - - 3 - - 4 - - 5 - type: integer - format: int32 ValidationStatus: type: object properties: 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 fa44e4ca3..68091279f 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/DataAnnotationValidatorTests.cs @@ -122,7 +122,7 @@ public async Task ValidateFormData_RequiredProperty() "field": "range", "code": "The field RangeProperty must be between 1 and 10.", "description": "The field RangeProperty must be between 1 and 10.", - "source": "ModelState", + "source": null, "customTextKey": null }, { @@ -132,7 +132,7 @@ public async Task ValidateFormData_RequiredProperty() "field": "requiredProperty", "code": "The RequiredProperty field is required.", "description": "The RequiredProperty field is required.", - "source": "ModelState", + "source": null, "customTextKey": null }, { @@ -142,7 +142,7 @@ public async Task ValidateFormData_RequiredProperty() "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", + "source": null, "customTextKey": null }, { @@ -152,7 +152,7 @@ public async Task ValidateFormData_RequiredProperty() "field": "NestedProperty.requiredProperty", "code": "The RequiredProperty field is required.", "description": "The RequiredProperty field is required.", - "source": "ModelState", + "source": null, "customTextKey": null } ] 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 93da0336a..00f3268a7 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -4,6 +4,8 @@ using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Features.Validation.Default; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; @@ -15,34 +17,41 @@ namespace Altinn.App.Core.Tests.Features.Validators.Default; public class LegacyIValidationFormDataTests { private readonly LegacyIInstanceValidatorFormDataValidator _validator; - private readonly Mock _instanceValidator = new(); + private readonly Mock _instanceValidator = new(MockBehavior.Strict); + private readonly Mock _appMetadata = new(MockBehavior.Strict); + private readonly Mock _instanceDataAccessor = new(MockBehavior.Strict); + + private readonly ApplicationMetadata _applicationMetadata = new ApplicationMetadata("ttd/test") + { + Title = new LanguageString() { { "nb", "test" } }, + DataTypes = new List() + { + new DataType() { Id = "test", TaskId = "Task_1" }, + }, + }; + + private readonly Guid _dataId = Guid.NewGuid(); + private readonly DataElement _dataElement; + private readonly Instance _instance; public LegacyIValidationFormDataTests() { var generalSettings = new GeneralSettings(); _validator = new LegacyIInstanceValidatorFormDataValidator( Microsoft.Extensions.Options.Options.Create(generalSettings), - _instanceValidator.Object + _instanceValidator.Object, + _appMetadata.Object ); - } - - [Fact] - public async Task ValidateFormData_NoErrors() - { - // Arrange - var data = new object(); - - var validator = new LegacyIInstanceValidatorFormDataValidator( - Microsoft.Extensions.Options.Options.Create(new GeneralSettings()), - null - ); - validator.HasRelevantChanges(data, data).Should().BeFalse(); - - // Act - var result = await validator.ValidateFormData(new Instance(), new DataElement(), data, null); - - // Assert - Assert.Empty(result); + _appMetadata.Setup(am => am.GetApplicationMetadata()).ReturnsAsync(_applicationMetadata); + + _dataElement = new DataElement() { DataType = "test", Id = _dataId.ToString(), }; + _instance = new Instance() + { + AppId = "test", + Org = "test", + InstanceOwner = new InstanceOwner() { PartyId = "1", }, + Data = [_dataElement] + }; } [Fact] @@ -53,48 +62,53 @@ public async Task ValidateFormData_WithErrors() _instanceValidator .Setup(iv => iv.ValidateData(It.IsAny(), It.IsAny())) - .Callback( + .Returns( (object _, ModelStateDictionary modelState) => { modelState.AddModelError("test", "test"); modelState.AddModelError("ddd", "*FIXED*test"); + return Task.CompletedTask; } - ); + ) + .Verifiable(Times.Once); + + _instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data); // Act - var result = await _validator.ValidateFormData(new Instance(), new DataElement(), data, null); + var result = await _validator.Validate(_instance, "Task_1", null, _instanceDataAccessor.Object); // Assert result .Should() .BeEquivalentTo( JsonSerializer.Deserialize>( - """ + $$""" [ { "severity": 4, "instanceId": null, - "dataElementId": null, + "dataElementId": "{{_dataId}}", "field": "ddd", "code": "test", "description": "test", - "source": "Custom", "customTextKey": null }, { "severity": 1, "instanceId": null, - "dataElementId": null, + "dataElementId": "{{_dataId}}", "field": "test", "code": "test", "description": "test", - "source": "Custom", "customTextKey": null } ] """ ) ); + + _instanceDataAccessor.Verify(); + _instanceValidator.Verify(); } private class TestModel @@ -128,16 +142,19 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string _instanceValidator .Setup(iv => iv.ValidateData(It.IsAny(), It.IsAny())) - .Callback( + .Returns( (object _, ModelStateDictionary modelState) => { modelState.AddModelError(errorKey, errorMessage); modelState.AddModelError(errorKey, "*FIXED*" + errorMessage + " Fixed"); + return Task.CompletedTask; } - ); + ) + .Verifiable(Times.Once); + _instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once); // Act - var result = await _validator.ValidateFormData(new Instance(), new DataElement(), data, null); + var result = await _validator.Validate(_instance, "Task_1", null, _instanceDataAccessor.Object); // Assert result.Should().HaveCount(2); @@ -150,5 +167,8 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string fixedIssue.Field.Should().Be(field); fixedIssue.Severity.Should().Be(ValidationIssueSeverity.Fixed); fixedIssue.Description.Should().Be(errorMessage + " Fixed"); + + _instanceDataAccessor.Verify(); + _instanceValidator.Verify(); } } diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs index 378b21155..74eb9dc28 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceOldTests.cs @@ -1,4 +1,5 @@ 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.Features.Validation.Helpers; @@ -21,10 +22,10 @@ namespace Altinn.App.Core.Tests.Features.Validators; 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 Mock _httpContextAccessorMock = new(); + private readonly Mock _dataClientMock = new(MockBehavior.Strict); + private readonly Mock _appModelMock = new(MockBehavior.Strict); + private readonly Mock _appMetadataMock = new(MockBehavior.Strict); private readonly ServiceCollection _serviceCollection = new(); private readonly ApplicationMetadata _applicationMetadata = @@ -52,11 +53,7 @@ public ValidationServiceOldTests() _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(); - _serviceCollection.AddScoped(); - - _httpContextAccessorMock.SetupGet(h => h.HttpContext!.TraceIdentifier).Returns(Guid.NewGuid().ToString()); - _serviceCollection.AddSingleton(_httpContextAccessorMock.Object); - + _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new GeneralSettings())); _appMetadataMock.Setup(am => am.GetApplicationMetadata()).ReturnsAsync(_applicationMetadata); } @@ -66,18 +63,25 @@ public async Task FileScanEnabled_VirusFound_ValidationShouldFail() 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 }; + var dataType = new DataType() + { + Id = "testScan", + TaskId = "Task_1", + EnableFileScan = true + }; + _applicationMetadata.DataTypes.Add(dataType); + var dataElement = new DataElement() { DataType = "testScan", FileScanResult = FileScanResult.Infected }; + var instance = new Instance() { Data = [dataElement] }; + var dataAccessor = new Mock(); - List validationIssues = await validationService.ValidateDataElement( + List validationIssues = await validationService.ValidateInstanceAtTask( instance, - dataElement, - dataType, + "Task_1", + dataAccessor.Object, null ); - validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileInfected").Should().NotBeNull(); + validationIssues.Should().ContainSingle(vi => vi.Code == "DataElementFileInfected"); } [Fact] @@ -93,13 +97,15 @@ public async Task FileScanEnabled_PendingScanNotEnabled_ValidationShouldNotFail( AppLogic = null, EnableFileScan = true }; - var instance = new Instance() { }; var dataElement = new DataElement() { DataType = "test", FileScanResult = FileScanResult.Pending, }; + var instance = new Instance() { Data = [dataElement] }; - List validationIssues = await validationService.ValidateDataElement( + var dataAccessorMock = new Mock(); + + List validationIssues = await validationService.ValidateInstanceAtTask( instance, - dataElement, - dataType, + "Task_1", + dataAccessorMock.Object, null ); @@ -112,18 +118,26 @@ public async Task FileScanEnabled_PendingScanEnabled_ValidationShouldNotFail() 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 }; + var dataType = new DataType() + { + Id = "testScan", + TaskId = "Task_1", + EnableFileScan = true, + ValidationErrorOnPendingFileScan = true + }; + _applicationMetadata.DataTypes.Add(dataType); + var dataElement = new DataElement() { DataType = "testScan", FileScanResult = FileScanResult.Pending }; + var instance = new Instance() { Data = [dataElement], }; + var dataAccessorMock = new Mock(); - List validationIssues = await validationService.ValidateDataElement( + List validationIssues = await validationService.ValidateInstanceAtTask( instance, - dataElement, - dataType, + "Task_1", + dataAccessorMock.Object, null ); - validationIssues.FirstOrDefault(vi => vi.Code == "DataElementFileScanPending").Should().NotBeNull(); + validationIssues.Should().ContainSingle(vi => vi.Code == "DataElementFileScanPending"); } [Fact] @@ -132,14 +146,21 @@ public async Task FileScanEnabled_Clean_ValidationShouldNotFail() 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.Clean, }; + var instance = new Instance() + { + AppId = "ttd/test-app", + Org = "ttd", + Data = [dataElement] + }; + + var dataAccessorMock = new Mock(); - List validationIssues = await validationService.ValidateDataElement( + List validationIssues = await validationService.ValidateInstanceAtTask( instance, - dataElement, - dataType, + "Task_1", + dataAccessorMock.Object, null ); @@ -177,10 +198,11 @@ public async Task ValidateAndUpdateProcess_set_canComplete_validationstatus_and_ { new() { DataType = "data", ContentType = "application/json" }, }, - Process = new ProcessState { CurrentTask = new ProcessElementInfo { Name = "Task_1" } } + Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "Task_1" } } }; + var dataAccessorMock = new Mock(); - var issues = await validationService.ValidateInstanceAtTask(instance, taskId, null); + var issues = await validationService.ValidateInstanceAtTask(instance, taskId, dataAccessorMock.Object, null); issues.Should().BeEmpty(); // instance.Process?.CurrentTask?.Validated.CanCompleteTask.Should().BeTrue(); @@ -228,15 +250,13 @@ public async Task ValidateAndUpdateProcess_set_canComplete_false_validationstatu ContentType = "application/json" }, }, - Process = new ProcessState { CurrentTask = new ProcessElementInfo { Name = "Task_1" } } + Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "Task_1" } } }; + var dataAccessorMock = new Mock(); - var issues = await validationService.ValidateInstanceAtTask(instance, taskId, null); + var issues = await validationService.ValidateInstanceAtTask(instance, taskId, dataAccessorMock.Object, null); issues.Should().HaveCount(1); issues.Should().ContainSingle(i => i.Code == ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType); - - // instance.Process?.CurrentTask?.Validated.CanCompleteTask.Should().BeFalse(); - // instance.Process?.CurrentTask?.Validated.Timestamp.Should().NotBeNull(); } [Fact] diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 988a4edff..fdb010c18 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -1,4 +1,5 @@ using System.Text.Json.Serialization; +using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; @@ -28,18 +29,18 @@ private class MyModel } private const int DefaultPartyId = 234; - private static readonly Guid DefaultInstanceId = Guid.NewGuid(); - private static readonly Guid DefaultDataElementId = Guid.NewGuid(); + private static readonly Guid _defaultInstanceId = Guid.NewGuid(); + private static readonly Guid _defaultDataElementId = Guid.NewGuid(); private const string DefaultTaskId = "Task_1"; private const string DefaultOrg = "org"; private const string DefaultApp = "app"; private const string DefaultAppId = $"{DefaultOrg}/{DefaultApp}"; private const string DefaultLanguage = "defaultLanguageCode"; - private static readonly DataElement DefaultDataElement = - new() { Id = DefaultDataElementId.ToString(), DataType = "MyType", }; + private static readonly DataElement _defaultDataElement = + new() { Id = _defaultDataElementId.ToString(), DataType = "MyType", }; - private static readonly DataType DefaultDataType = + private static readonly DataType _defaultDataType = new() { Id = "MyType", @@ -47,23 +48,36 @@ private class MyModel AppLogic = new ApplicationLogic { ClassRef = typeof(MyModel).FullName } }; - private static readonly Instance DefaultInstance = + private static readonly DataType _neverataType = new() { - Id = $"{DefaultPartyId}/{DefaultInstanceId}", + Id = "never", + TaskId = DefaultTaskId, + AppLogic = new ApplicationLogic { ClassRef = typeof(MyModel).FullName } + }; + + private static readonly Instance _defaultInstance = + new() + { + Id = $"{DefaultPartyId}/{_defaultInstanceId}", InstanceOwner = new InstanceOwner() { PartyId = DefaultPartyId.ToString(), }, Org = DefaultOrg, AppId = DefaultAppId, - Data = [DefaultDataElement], - Process = new ProcessState { CurrentTask = new ProcessElementInfo { Name = "Task1" } } + Data = [_defaultDataElement], + Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "Task_1" } } }; - private static readonly ApplicationMetadata DefaultAppMetadata = - new(DefaultAppId) { DataTypes = new List { DefaultDataType, }, }; + private static readonly ApplicationMetadata _defaultAppMetadata = + new(DefaultAppId) + { + DataTypes = new List { _defaultDataType, _neverataType }, + }; private readonly Mock> _loggerMock = new(); private readonly Mock _dataClientMock = new(MockBehavior.Strict); + private readonly IInstanceDataAccessor _dataAccessor; + private readonly Mock _appModelMock = new(MockBehavior.Strict); private readonly Mock _appMetadataMock = new(MockBehavior.Strict); @@ -97,19 +111,26 @@ private class MyModel public ValidationServiceTests() { + _dataAccessor = new CachedInstanceDataAccessor( + _defaultInstance, + _dataClientMock.Object, + _appMetadataMock.Object, + _appModelMock.Object + ); _serviceCollection.AddSingleton(_loggerMock.Object); _serviceCollection.AddSingleton(_dataClientMock.Object); _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(_appModelMock.Object); _appModelMock.Setup(a => a.GetModelType(typeof(MyModel).FullName!)).Returns(typeof(MyModel)); _serviceCollection.AddSingleton(_appMetadataMock.Object); - _appMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(DefaultAppMetadata); + _appMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(_defaultAppMetadata); _serviceCollection.AddSingleton(); - _serviceCollection.AddScoped(); _httpContextAccessorMock.Setup(h => h.HttpContext!.TraceIdentifier).Returns(Guid.NewGuid().ToString()); _serviceCollection.AddSingleton(_httpContextAccessorMock.Object); + _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new GeneralSettings())); + // NeverUsedValidators _serviceCollection.AddSingleton(_taskValidatorNeverMock.Object); SetupTaskValidatorType(_taskValidatorNeverMock, "never", "neverTask"); @@ -122,9 +143,9 @@ public ValidationServiceTests() _serviceCollection.AddSingleton(_taskValidatorMock.Object); SetupTaskValidatorType(_taskValidatorMock, DefaultTaskId, "specificTaskValidator"); _serviceCollection.AddSingleton(_dataElementValidatorMock.Object); - SetupDataElementValidatorType(_dataElementValidatorMock, DefaultDataType.Id, "specificDataElementValidator"); + SetupDataElementValidatorType(_dataElementValidatorMock, _defaultDataType.Id, "specificDataElementValidator"); _serviceCollection.AddSingleton(_formDataValidatorMock.Object); - SetupFormDataValidatorType(_formDataValidatorMock, DefaultDataType.Id, "specificValidator"); + SetupFormDataValidatorType(_formDataValidatorMock, _defaultDataType.Id, "specificValidator"); // AlwaysUsedValidators _serviceCollection.AddSingleton(_taskValidatorAlwaysMock.Object); @@ -148,7 +169,7 @@ private void SetupTaskValidatorReturn( ) { taskValidatorMock - .Setup(v => v.ValidateTask(DefaultInstance, DefaultTaskId, DefaultLanguage)) + .Setup(v => v.ValidateTask(_defaultInstance, DefaultTaskId, DefaultLanguage)) .ReturnsAsync(validationIssues) .Verifiable(times ?? Times.Once()); } @@ -170,7 +191,7 @@ private void SetupDataElementValidatorReturn( ) { dataElementValidatorMock - .Setup(v => v.ValidateDataElement(DefaultInstance, DefaultDataElement, DefaultDataType, DefaultLanguage)) + .Setup(v => v.ValidateDataElement(_defaultInstance, _defaultDataElement, _defaultDataType, DefaultLanguage)) .ReturnsAsync(validationIssues) .Verifiable(times ?? Times.Once()); } @@ -197,7 +218,7 @@ private void SetupFormDataValidatorReturn( { // ValidateFormData formDataValidatorMock - .Setup(v => v.ValidateFormData(DefaultInstance, DefaultDataElement, It.IsAny(), DefaultLanguage)) + .Setup(v => v.ValidateFormData(_defaultInstance, _defaultDataElement, It.IsAny(), DefaultLanguage)) .ReturnsAsync((Instance instance, DataElement dataElement, MyModel data, string? language) => func(data)) .Verifiable(hasRelevantChanges is not false ? (times ?? Times.Once()) : Times.Never()); @@ -208,12 +229,12 @@ private void SetupFormDataValidatorReturn( .Verifiable(hasRelevantChanges is null ? Times.Never : Times.AtLeastOnce); } - private void SourcePropertyIsSet(Dictionary> result) + private void SourcePropertyIsSet(Dictionary> result) { Assert.All(result.Values, SourcePropertyIsSet); } - private void SourcePropertyIsSet(List result) + private void SourcePropertyIsSet(List result) { Assert.All( result, @@ -230,12 +251,12 @@ private void SetupDataClient(MyModel data) _dataClientMock .Setup(d => d.GetFormData( - DefaultInstanceId, + _defaultInstanceId, data.GetType(), DefaultOrg, DefaultApp, DefaultPartyId, - DefaultDataElementId + _defaultDataElementId ) ) .ReturnsAsync(data) @@ -253,30 +274,14 @@ public async Task Validate_WithNoValidators_ReturnsNoErrors() await using var serviceProvider = _serviceCollection.BuildServiceProvider(); var validatorService = serviceProvider.GetRequiredService(); - var data = new MyModel { Name = "Ola" }; - SetupDataClient(data); - - var resultTask = await validatorService.ValidateInstanceAtTask(DefaultInstance, DefaultTaskId, DefaultLanguage); - resultTask.Should().BeEmpty(); - var resultElement = await validatorService.ValidateDataElement( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - DefaultLanguage - ); - resultElement.Should().BeEmpty(); - - var resultData = await validatorService.ValidateFormData( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - data, - null, - null, + var resultTask = await validatorService.ValidateInstanceAtTask( + _defaultInstance, + DefaultTaskId, + _dataAccessor, DefaultLanguage ); - resultData.Should().BeEmpty(); + resultTask.Should().BeEmpty(); } [Fact] @@ -311,12 +316,20 @@ public async Task ValidateFormData_WithSpecificValidator() var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Ola" }; var previousData = new MyModel() { Name = "Kari" }; - var result = await validatorService.ValidateFormData( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - data, - previousData, + SetupDataClient(data); + var result = await validatorService.ValidateIncrementalFormData( + _defaultInstance, + "Task_1", + new List + { + new() + { + DataElement = _defaultDataElement, + PreviousValue = previousData, + CurrentValue = data + } + }, + _dataAccessor, null, DefaultLanguage ); @@ -331,7 +344,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa { SetupFormDataValidatorReturn( _formDataValidatorMock, - hasRelevantChanges: null, + hasRelevantChanges: true, model => new List { new() { Severity = ValidationIssueSeverity.Error, CustomTextKey = "NameNotOla" } @@ -340,7 +353,7 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa SetupFormDataValidatorReturn( _formDataValidatorAlwaysMock, - hasRelevantChanges: null, + hasRelevantChanges: true, model => new List { new() { Severity = ValidationIssueSeverity.Error, CustomTextKey = "AlwaysNameNotOla" } @@ -350,13 +363,26 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa await using var serviceProvider = _serviceCollection.BuildServiceProvider(); var validatorService = serviceProvider.GetRequiredService(); + var dataAccessor = new CachedInstanceDataAccessor( + _defaultInstance, + _dataClientMock.Object, + _appMetadataMock.Object, + _appModelMock.Object + ); var data = new MyModel { Name = "Kari" }; - var resultData = await validatorService.ValidateFormData( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - data, - null, + dataAccessor.Set(_defaultDataElement, data); + var resultData = await validatorService.ValidateIncrementalFormData( + _defaultInstance, + "Task_1", + [ + new DataElementChange() + { + DataElement = _defaultDataElement, + CurrentValue = data, + PreviousValue = data, + } + ], + dataAccessor, null, DefaultLanguage ); @@ -396,37 +422,45 @@ List CreateIssues(string code) SetupDataElementValidatorReturn( _dataElementValidatorMock, CreateIssues("data_element_validator"), - Times.Exactly(2) + Times.Once() ); SetupDataElementValidatorReturn( _dataElementValidatorAlwaysMock, CreateIssues("data_element_validator_always"), - Times.Exactly(2) + Times.Once() ); SetupFormDataValidatorReturn( _formDataValidatorMock, hasRelevantChanges: null, /* should not call HasRelevantChanges */ model => CreateIssues("form_data_validator"), - Times.Exactly(3) + Times.Once() ); SetupFormDataValidatorReturn( _formDataValidatorAlwaysMock, hasRelevantChanges: null, /* should not call HasRelevantChanges */ model => CreateIssues("form_data_validator_always"), - Times.Exactly(3) + Times.Once() ); var data = new MyModel(); SetupDataClient(data); - using var serviceProvider = _serviceCollection.BuildServiceProvider(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); var validationService = serviceProvider.GetRequiredService(); + var dataAccessor = new CachedInstanceDataAccessor( + _defaultInstance, + _dataClientMock.Object, + _appMetadataMock.Object, + _appModelMock.Object + ); + var taskResult = await validationService.ValidateInstanceAtTask( - DefaultInstance, + _defaultInstance, DefaultTaskId, + dataAccessor, DefaultLanguage ); @@ -462,61 +496,6 @@ List CreateIssues(string code) .Be(ValidationIssueSeverity.Error); taskResult.Should().HaveCount(6); SourcePropertyIsSet(taskResult); - - var elementResult = await validationService.ValidateDataElement( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - DefaultLanguage - ); - elementResult - .Should() - .Contain(i => i.Code == "data_element_validator") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - elementResult - .Should() - .Contain(i => i.Code == "data_element_validator_always") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - elementResult - .Should() - .Contain(i => i.Code == "form_data_validator") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - elementResult - .Should() - .Contain(i => i.Code == "form_data_validator_always") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - elementResult.Should().HaveCount(4); - SourcePropertyIsSet(elementResult); - - var dataResult = await validationService.ValidateFormData( - DefaultInstance, - DefaultDataElement, - DefaultDataType, - data, - null, - null, - DefaultLanguage - ); - dataResult - .Should() - .ContainKey("specificValidator") - .WhoseValue.Should() - .ContainSingle(i => i.Code == "form_data_validator") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - dataResult - .Should() - .ContainKey("alwaysUsedValidator") - .WhoseValue.Should() - .ContainSingle(i => i.Code == "form_data_validator_always") - .Which.Severity.Should() - .Be(ValidationIssueSeverity.Error); - dataResult.Should().HaveCount(2); - SourcePropertyIsSet(dataResult); } [Fact] @@ -537,10 +516,15 @@ public async Task ValidateTask_ReturnsNoErrorsFromAllLevels() var data = new MyModel(); SetupDataClient(data); - using var serviceProvider = _serviceCollection.BuildServiceProvider(); + await using var serviceProvider = _serviceCollection.BuildServiceProvider(); var validationService = serviceProvider.GetRequiredService(); - var result = await validationService.ValidateInstanceAtTask(DefaultInstance, DefaultTaskId, DefaultLanguage); + var result = await validationService.ValidateInstanceAtTask( + _defaultInstance, + DefaultTaskId, + _dataAccessor, + DefaultLanguage + ); result.Should().BeEmpty(); } @@ -559,6 +543,11 @@ public void Dispose() _dataElementValidatorAlwaysMock.Verify(); _formDataValidatorAlwaysMock.Verify(); + _dataClientMock.Verify(); + _appMetadataMock.Verify(); + _appModelMock.Verify(); + _loggerMock.Verify(); + _dataClientMock.Verify(); } } diff --git a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt index 5a0488bdb..5e70663ef 100644 --- a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt +++ b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt @@ -1,28 +1,18 @@ { Activities: [ + { + ActivityName: Data.ProcessWrite.IDataProcessorProxy, + IdFormat: W3C + }, { ActivityName: Data.Patch, Tags: [ { instance.guid: Guid_1 - }, - { - result: success } ], IdFormat: W3C } ], - Metrics: [ - { - altinn_app_lib_data_patched: [ - { - Value: 1, - Tags: { - result: success - } - } - ] - } - ] -} \ No newline at end of file + Metrics: [] +} diff --git a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs index 9b6a4c270..087d51184 100644 --- a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs @@ -1,5 +1,6 @@ using System.ComponentModel.DataAnnotations; using Altinn.App.Common.Tests; +using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; @@ -14,6 +15,7 @@ using Json.Pointer; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Options; using Moq; using DataType = Altinn.Platform.Storage.Interface.Models.DataType; @@ -22,13 +24,17 @@ namespace Altinn.App.Core.Tests.Internal.Patch; public class PatchServiceTests : IDisposable { // Test data - private static readonly Guid DataGuid = new("12345678-1234-1234-1234-123456789123"); + private static readonly Guid _dataGuid = new("12345678-1234-1234-1234-123456789123"); private readonly Instance _instance = new() { Id = "1337/12345678-1234-1234-1234-12345678912a", - Process = new ProcessState { CurrentTask = new ProcessElementInfo { Name = "Task_1" } } + AppId = "ttd/test", + Org = "ttd", + InstanceOwner = new() { PartyId = "1337" }, + Data = [_dataElement], + Process = new() { CurrentTask = new() { ElementId = "Task_1" }, } }; // Service mocks @@ -49,10 +55,11 @@ public class PatchServiceTests : IDisposable public PatchServiceTests() { + var applicationMetadata = new ApplicationMetadata("ttd/test") { DataTypes = [_dataType], }; _appMetadataMock .Setup(a => a.GetApplicationMetadata()) - .ReturnsAsync(new ApplicationMetadata("ttd/test")) - .Verifiable(); + .ReturnsAsync(applicationMetadata) + .Verifiable(Times.AtLeastOnce); _appModelMock .Setup(a => a.GetModelType("Altinn.App.Core.Tests.Internal.Patch.PatchServiceTests+MyModel")) .Returns(typeof(MyModel)) @@ -60,6 +67,8 @@ public PatchServiceTests() _formDataValidator.Setup(fdv => fdv.DataType).Returns(_dataType.Id); _formDataValidator.Setup(fdv => fdv.ValidationSource).Returns("formDataValidator"); _formDataValidator.Setup(fdv => fdv.HasRelevantChanges(It.IsAny(), It.IsAny())).Returns(true); + _dataElementValidator.Setup(dev => dev.DataType).Returns(_dataType.Id); + _dataElementValidator.Setup(dev => dev.ValidationSource).Returns("dataElementValidator"); _dataClientMock .Setup(d => d.UpdateData( @@ -74,8 +83,15 @@ public PatchServiceTests() ) .ReturnsAsync(_dataElement) .Verifiable(); - _httpContextAccessorMock.SetupGet(hca => hca.HttpContext!.TraceIdentifier).Returns(Guid.NewGuid().ToString()); - var validatorFactory = new ValidatorFactory([], [_dataElementValidator.Object], [_formDataValidator.Object]); + var validatorFactory = new ValidatorFactory( + [], + Options.Create(new GeneralSettings()), + [_dataElementValidator.Object], + [_formDataValidator.Object], + [], + [], + _appMetadataMock.Object + ); var validationService = new ValidationService( validatorFactory, _dataClientMock.Object, @@ -100,14 +116,15 @@ public PatchServiceTests() ); } - private readonly DataType _dataType = + private static readonly DataType _dataType = new() { Id = "dataTypeId", - AppLogic = new() { ClassRef = "Altinn.App.Core.Tests.Internal.Patch.PatchServiceTests+MyModel" } + AppLogic = new() { ClassRef = "Altinn.App.Core.Tests.Internal.Patch.PatchServiceTests+MyModel" }, + TaskId = "Task_1", }; - private readonly DataElement _dataElement = new() { Id = DataGuid.ToString(), DataType = "dataTypeId" }; + private static readonly DataElement _dataElement = new() { Id = _dataGuid.ToString(), DataType = _dataType.Id }; private class MyModel { @@ -162,21 +179,20 @@ public async Task Test_Ok() .ReturnsAsync(validationIssues); // Act - var response = await _patchService.ApplyPatch( - _instance, - _dataType, - _dataElement, - jsonPatch, - null, - ignoredValidators - ); + var patches = new Dictionary() { { _dataGuid, jsonPatch } }; + var response = await _patchService.ApplyPatches(_instance, patches, null, ignoredValidators); // Assert response.Should().NotBeNull(); response.Success.Should().BeTrue(); response.Ok.Should().NotBeNull(); var res = response.Ok!; - res.NewDataModel.Should().BeOfType().Subject.Name.Should().Be("Test Testesen"); + res.NewDataModels.Should() + .ContainKey(_dataGuid) + .WhoseValue.Should() + .BeOfType() + .Subject.Name.Should() + .Be("Test Testesen"); var validator = res.ValidationIssues.Should().ContainSingle().Which; validator.Key.Should().Be("formDataValidator"); var issue = validator.Value.Should().ContainSingle().Which; @@ -238,14 +254,8 @@ public async Task Test_JsonPatchTest_fail() .ReturnsAsync(validationIssues); // Act - var response = await _patchService.ApplyPatch( - _instance, - _dataType, - _dataElement, - jsonPatch, - null, - ignoredValidators - ); + var patches = new Dictionary() { { _dataGuid, jsonPatch } }; + var response = await _patchService.ApplyPatches(_instance, patches, null, ignoredValidators); // Assert response.Should().NotBeNull(); @@ -306,14 +316,8 @@ public async Task Test_JsonPatch_does_not_deserialize() .ReturnsAsync(validationIssues); // Act - var response = await _patchService.ApplyPatch( - _instance, - _dataType, - _dataElement, - jsonPatch, - null, - ignoredValidators - ); + var patches = new Dictionary() { { _dataGuid, jsonPatch } }; + var response = await _patchService.ApplyPatches(_instance, patches, null, ignoredValidators); // Assert response.Should().NotBeNull(); diff --git a/test/Altinn.App.Core.Tests/Internal/Process/Elements/AppProcessStateTests.cs b/test/Altinn.App.Core.Tests/Internal/Process/Elements/AppProcessStateTests.cs index ccc02868c..266332c3e 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/Elements/AppProcessStateTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/Elements/AppProcessStateTests.cs @@ -21,7 +21,7 @@ public void Constructor_with_ProcessState_copies_values() Started = DateTime.Now, Ended = DateTime.Now, Flow = 2, - Name = "Task_1", + Name = "Utfylling", Validated = new() { Timestamp = DateTime.Now, CanCompleteTask = false }, ElementId = "Task_1", FlowType = "FlowType", @@ -71,7 +71,7 @@ public void Constructor_with_ProcessState_copies_values_validated_null() Started = DateTime.Now, Ended = DateTime.Now, Flow = 2, - Name = "Task_1", + Name = "Utfylling", Validated = null, ElementId = "Task_1", FlowType = "FlowType", diff --git a/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs b/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs index 793354ff4..1ec884a46 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs @@ -300,12 +300,11 @@ private ExpressionsExclusiveGateway SetupExpressionsGateway( var frontendSettings = Options.Create(new FrontEndSettings()); - _httpContextAccessor.SetupGet(hca => hca.HttpContext!.TraceIdentifier).Returns(Guid.NewGuid().ToString()); - var layoutStateInit = new LayoutEvaluatorStateInitializer( _resources.Object, frontendSettings, - new CachedFormDataAccessor( + new CachedInstanceDataAccessor( + _instance, _dataClient.Object, _appMetadata.Object, _appModel.Object, diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestFunctions.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestFunctions.cs index 95b3d448a..c02289b6e 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestFunctions.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/CommonTests/TestFunctions.cs @@ -235,7 +235,9 @@ public void Ensure_tests_For_All_Folders() // This is just a way to ensure that all folders have test methods associcated. var jsonTestFolders = Directory .GetDirectories(Path.Join("LayoutExpressions", "CommonTests", "shared-tests", "functions")) + .Where(d => Directory.GetFiles(d).Length > 0) .Select(d => Path.GetFileName(d)) + .OrderBy(d => d) .ToArray(); var testMethods = this.GetType() .GetMethods() @@ -245,10 +247,9 @@ public void Ensure_tests_For_All_Folders() .Value ) .OfType() + .OrderBy(d => d) .ToArray(); - testMethods - .Should() - .BeEquivalentTo(jsonTestFolders, "Shared test folders should have a corresponding test method"); + testMethods.Should().Equal(jsonTestFolders, "Shared test folders should have a corresponding test method"); } }