diff --git a/src/Altinn.App.Api/Controllers/ActionsController.cs b/src/Altinn.App.Api/Controllers/ActionsController.cs index a1eeaed01..6c6f288af 100644 --- a/src/Altinn.App.Api/Controllers/ActionsController.cs +++ b/src/Altinn.App.Api/Controllers/ActionsController.cs @@ -202,7 +202,7 @@ Dictionary resultUpdatedDataModels continue; } var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase)); - var previousData = await dataAccessor.Get(dataElement); + var previousData = await dataAccessor.GetData(dataElement); ObjectUtils.InitializeAltinnRowId(newModel); ObjectUtils.PrepareModelForXmlStorage(newModel); @@ -254,7 +254,7 @@ await _dataClient.UpdateData( return PartitionValidationIssuesByDataElement(validationIssues); } - private Dictionary< + private static Dictionary< string, Dictionary> > PartitionValidationIssuesByDataElement(Dictionary> validationIssues) @@ -266,12 +266,12 @@ private Dictionary< { if (!updatedValidationIssues.TryGetValue(issue.DataElementId ?? "", out var elementIssues)) { - elementIssues = new Dictionary>(); + elementIssues = []; updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues; } if (!elementIssues.TryGetValue(validationSource, out var sourceIssues)) { - sourceIssues = new List(); + sourceIssues = []; 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 02a009c21..400f796c9 100644 --- a/src/Altinn.App.Api/Controllers/DataController.cs +++ b/src/Altinn.App.Api/Controllers/DataController.cs @@ -511,13 +511,6 @@ public async Task> PatchFormDataMultiple ); } - CachedInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor( - instance, - _dataClient, - _appMetadata, - _appModel - ); - foreach (Guid dataGuid in dataPatchRequest.Patches.Keys) { var dataElement = instance.Data.First(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal)); diff --git a/src/Altinn.App.Api/Controllers/ValidateController.cs b/src/Altinn.App.Api/Controllers/ValidateController.cs index e8f231ff5..0238f6293 100644 --- a/src/Altinn.App.Api/Controllers/ValidateController.cs +++ b/src/Altinn.App.Api/Controllers/ValidateController.cs @@ -130,7 +130,7 @@ public async Task ValidateData( throw new ValidationException("Unable to validate instance without a started process."); } - List messages = new List(); + List messages = []; DataElement? element = instance.Data.FirstOrDefault(d => d.Id == dataGuid.ToString()); @@ -150,10 +150,9 @@ public async Task ValidateData( 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) - ); + // Run validations for all data elements, but only return the issues for the specific data element + var issues = await _validationService.ValidateInstanceAtTask(instance, dataType.TaskId, dataAccessor, language); + messages.AddRange(issues.Where(i => i.DataElementId == element.Id)); string taskId = instance.Process.CurrentTask.ElementId; diff --git a/src/Altinn.App.Core/Features/ITaskValidator.cs b/src/Altinn.App.Core/Features/ITaskValidator.cs index 1cf2c6abf..c84355e84 100644 --- a/src/Altinn.App.Core/Features/ITaskValidator.cs +++ b/src/Altinn.App.Core/Features/ITaskValidator.cs @@ -1,6 +1,5 @@ using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; -using Microsoft.Extensions.DependencyInjection; namespace Altinn.App.Core.Features; diff --git a/src/Altinn.App.Core/Features/IValidator.cs b/src/Altinn.App.Core/Features/IValidator.cs index 68f613b06..829980f69 100644 --- a/src/Altinn.App.Core/Features/IValidator.cs +++ b/src/Altinn.App.Core/Features/IValidator.cs @@ -87,5 +87,5 @@ public interface IInstanceDataAccessor /// /// 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); + Task GetData(DataElement dataElement); } diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs index e0c091bc7..07e75401d 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -76,7 +76,6 @@ public async Task> Validate( var validationIssues = new List(); foreach (var dataElement in formDataElementsForTask) { - var data = instanceDataAccessor.Get(dataElement); var issues = await ValidateFormData(instance, dataElement, instanceDataAccessor, taskId, language); validationIssues.AddRange(issues); } diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index 034612788..bcea696b6 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -62,7 +62,7 @@ public async Task> Validate( 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))) { - var data = await instanceDataAccessor.Get(dataElement); + var data = await instanceDataAccessor.GetData(dataElement); var modelState = new ModelStateDictionary(); await _instanceValidator.ValidateData(data, modelState); issues.AddRange( diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs index 8022bc3db..4ad4b3a08 100644 --- a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs @@ -10,13 +10,11 @@ internal class FormDataValidatorWrapper : IValidator { private readonly IFormDataValidator _formDataValidator; private readonly string _taskId; - private readonly List _dataTypes; - public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string taskId, List dataTypes) + public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string taskId) { _formDataValidator = formDataValidator; _taskId = taskId; - _dataTypes = dataTypes; } /// @@ -44,7 +42,7 @@ public async Task> Validate( continue; } - var data = await instanceDataAccessor.Get(dataElement); + var data = await instanceDataAccessor.GetData(dataElement); var dataElementValidationResult = await _formDataValidator.ValidateFormData( instance, dataElement, diff --git a/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs b/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs index 06a171ae9..a8db9a747 100644 --- a/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs +++ b/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs @@ -84,7 +84,12 @@ public DataModel(IEnumerable> dataModels) return null; } - private object? GetModelDataRecursive(string[] keys, int index, object? currentModel, ReadOnlySpan indicies) + private static object? GetModelDataRecursive( + string[] keys, + int index, + object? currentModel, + ReadOnlySpan indicies + ) { if (index == keys.Length || currentModel is null) { diff --git a/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs index 2fdd71a39..448adc7b0 100644 --- a/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs @@ -46,7 +46,7 @@ IAppModel appModel public Instance Instance => _instance; /// - public async Task Get(DataElement dataElement) + public async Task GetData(DataElement dataElement) { return await _cache.GetOrCreate( dataElement.Id, @@ -97,7 +97,6 @@ public async Task GetOrCreate(TKey key, Func> valueFa { task = _cache.GetOrAdd(key, innerKey => new Lazy>(() => valueFactory(innerKey))).Value; } - ; return await task; } @@ -116,7 +115,6 @@ public void Set(TKey key, TValue data) private async Task GetBinaryData(DataElement dataElement) { - ; var data = await _dataClient.GetBinaryData( _org, _app, diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index 9b8817bee..b95cba265 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -3,7 +3,6 @@ using Altinn.App.Core.Features; using Altinn.App.Core.Helpers.DataModel; using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.Data; using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.Options; @@ -76,7 +75,9 @@ public async Task Init( dataTasks.AddRange( instance .Data.Where(dataElement => dataElement.DataType == dataType) - .Select(async dataElement => KeyValuePair.Create(dataElement, await dataAccessor.Get(dataElement))) + .Select(async dataElement => + KeyValuePair.Create(dataElement, await dataAccessor.GetData(dataElement)) + ) ); } diff --git a/src/Altinn.App.Core/Internal/Patch/PatchService.cs b/src/Altinn.App.Core/Internal/Patch/PatchService.cs index 1e01ed0bd..f48f2d0b2 100644 --- a/src/Altinn.App.Core/Internal/Patch/PatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/PatchService.cs @@ -11,7 +11,6 @@ using Altinn.App.Core.Models.Result; using Altinn.Platform.Storage.Interface.Models; using Json.Patch; -using static Altinn.App.Core.Features.Telemetry; namespace Altinn.App.Core.Internal.Patch; @@ -78,7 +77,7 @@ public async Task> ApplyPatches( }; } - var oldModel = await dataAccessor.Get(dataElement); + var oldModel = await dataAccessor.GetData(dataElement); var oldModelNode = JsonSerializer.SerializeToNode(oldModel); var patchResult = jsonPatch.Apply(oldModelNode); diff --git a/src/Altinn.App.Core/Internal/Process/ExpressionsExclusiveGateway.cs b/src/Altinn.App.Core/Internal/Process/ExpressionsExclusiveGateway.cs index 0909ef11d..a8282326b 100644 --- a/src/Altinn.App.Core/Internal/Process/ExpressionsExclusiveGateway.cs +++ b/src/Altinn.App.Core/Internal/Process/ExpressionsExclusiveGateway.cs @@ -1,10 +1,6 @@ -using System.Globalization; using System.Text; using System.Text.Json; using Altinn.App.Core.Features; -using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.AppModel; -using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Expressions; using Altinn.App.Core.Internal.Process.Elements; using Altinn.App.Core.Models.Expressions; diff --git a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs index c5baf4b2b..0ce5a8cf7 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs @@ -60,7 +60,7 @@ public async Task Finalize(string taskId, Instance instance) await Task.WhenAll( changedDataElements.Select(async dataElement => { - var data = await dataAccessor.Get(dataElement); + var data = await dataAccessor.GetData(dataElement); return _dataClient.UpdateData( data, Guid.Parse(instance.Id.Split('/')[1]), @@ -83,7 +83,7 @@ private async Task> RunRemoveFieldsInModelOnTaskComplet ) { ArgumentNullException.ThrowIfNull(instance.Data); - HashSet modifiedDataElements = new(); + HashSet modifiedDataElements = []; var dataTypesWithLogic = dataTypesToLock.Where(d => !string.IsNullOrEmpty(d.AppLogic?.ClassRef)).ToList(); await Task.WhenAll( @@ -128,11 +128,10 @@ private async Task RemoveFieldsOnTaskComplete( ) { bool isModified = false; - var data = await dataAccessor.Get(dataElement); + var data = await dataAccessor.GetData(dataElement); // remove AltinnRowIds - ObjectUtils.RemoveAltinnRowId(data); - isModified = true; + isModified |= ObjectUtils.RemoveAltinnRowId(data); // Remove hidden data before validation, ignore hidden rows. if (_appSettings.Value?.RemoveHiddenData == true) @@ -145,7 +144,7 @@ private async Task RemoveFieldsOnTaskComplete( language ); LayoutEvaluator.RemoveHiddenData(evaluationState, RowRemovalOption.Ignore); - // TODO: + // TODO: Make RemoveHiddenData return a bool indicating if data was removed isModified = true; } @@ -185,11 +184,13 @@ await _dataClient.InsertFormData( else { // Remove the shadow fields from the data + // TODO: This does not work!!! data = JsonSerializer.Deserialize(serializedData, data.GetType()) ?? throw new JsonException( "Could not deserialize back datamodel after removing shadow fields. Data was \"null\"" ); + isModified = true; // TODO: Detect if modifications were made } } diff --git a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs index c469b7b12..fda064a36 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs @@ -131,7 +131,7 @@ public IEnumerable GetValidators(string taskId) .Select(dev => new DataElementValidatorWrapper(dev, taskId, dataTypes)) ); validators.AddRange( - GetFormDataValidators(taskId, dataTypes).Select(fdv => new FormDataValidatorWrapper(fdv, taskId, dataTypes)) + GetFormDataValidators(taskId, dataTypes).Select(fdv => new FormDataValidatorWrapper(fdv, taskId)) ); // add legacy instance validators wrapped in IValidator wrappers diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index cc3573557..911f81046 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -1,7 +1,4 @@ using Altinn.App.Core.Features; -using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.AppModel; -using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.Logging; @@ -14,7 +11,6 @@ namespace Altinn.App.Core.Internal.Validation; public class ValidationService : IValidationService { private readonly IValidatorFactory _validatorFactory; - private readonly IAppMetadata _appMetadata; private readonly ILogger _logger; private readonly Telemetry? _telemetry; @@ -23,13 +19,11 @@ public class ValidationService : IValidationService /// public ValidationService( IValidatorFactory validatorFactory, - IAppMetadata appMetadata, ILogger logger, Telemetry? telemetry = null ) { _validatorFactory = validatorFactory; - _appMetadata = appMetadata; _logger = logger; _telemetry = telemetry; } @@ -64,7 +58,7 @@ public async Task> ValidateInstanceAtTask( { _logger.LogError( e, - "Error while running validator {validatorName} for task {taskId} on instance {instanceId}", + "Error while running validator {ValidatorName} for task {TaskId} on instance {InstanceId}", v.ValidationSource, taskId, instance.Id @@ -144,7 +138,7 @@ public async Task>> ValidateI return lists.Where(k => k.Value is not null).ToDictionary(kv => kv.Key, kv => kv.Value!); } - private void ThrowIfDuplicateValidators(IValidator[] validators, string taskId) + private static void ThrowIfDuplicateValidators(IValidator[] validators, string taskId) { var sourceNames = validators .Select(v => v.ValidationSource) diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs index 130d959d7..dd2fde834 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssue.cs @@ -1,6 +1,5 @@ using System.Text.Json; using System.Text.Json.Serialization; -using Altinn.App.Core.Internal.Validation; using Newtonsoft.Json; namespace Altinn.App.Core.Models.Validation; 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 3c7005ef7..a0b954875 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -72,7 +72,7 @@ public async Task ValidateFormData_WithErrors() ) .Verifiable(Times.Once); - _instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data); + _instanceDataAccessor.Setup(ida => ida.GetData(_dataElement)).ReturnsAsync(data); // Act var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null); @@ -151,7 +151,7 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string } ) .Verifiable(Times.Once); - _instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once); + _instanceDataAccessor.Setup(ida => ida.GetData(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once); // Act var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null); diff --git a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs index 464eef5c1..86fd8b348 100644 --- a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs @@ -92,7 +92,7 @@ public PatchServiceTests() [], _appMetadataMock.Object ); - var validationService = new ValidationService(validatorFactory, _appMetadataMock.Object, _vLoggerMock.Object); + var validationService = new ValidationService(validatorFactory, _vLoggerMock.Object); _patchService = new PatchService( _appMetadataMock.Object,