Continue work and fix a few more tests

src/Altinn.App.Api/Controllers/ActionsController.cs

@@ -46,7 +46,8 @@ public ActionsController(
         IValidationService validationService,
         IDataClient dataClient,
         IAppMetadata appMetadata,
-        IAppModel appModel)
+        IAppModel appModel
+    )
     {
         _authorization = authorization;
         _instanceClient = instanceClient;
@@ -159,42 +160,53 @@ public async Task<ActionResult<UserActionResponse>> Perform(
             );
         }
 
+        var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);
+        Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>? 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
+            );
         }
-        IInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor(_dataClient, _appMetadata, _appModel);
-        return new OkObjectResult(
+
+        return Ok(
             new UserActionResponse()
             {
                 ClientActions = result.ClientActions,
                 UpdatedDataModels = result.UpdatedDataModels,
-                UpdatedValidationIssues = await GetValidations(
-                    instance,
-                    result.UpdatedDataModels,
-                    dataAccessor,
-                    actionRequest.IgnoredValidators,
-                    language
-                ),
+                UpdatedValidationIssues = validationIssues,
                 RedirectUrl = result.RedirectUrl,
             }
         );
     }
 
-    private async Task SaveChangedModels(Instance instance, Dictionary<string, object> resultUpdatedDataModels)
+    private async Task<List<DataElementChange>> SaveChangedModels(
+        Instance instance,
+        CachedInstanceDataAccessor dataAccessor,
+        Dictionary<string, object> resultUpdatedDataModels
+    )
     {
+        var changes = new List<DataElementChange>();
         var instanceIdentifier = new InstanceIdentifier(instance);
         foreach (var (elementId, newModel) in resultUpdatedDataModels)
         {
             if (newModel is null)
             {
                 continue;
             }
+            var dataElement = instance.Data.First(d => 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,
@@ -204,67 +216,68 @@ 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<Dictionary<string, Dictionary<string, List<ValidationIssue>>>?> GetValidations(
+    private async Task<Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>?> GetValidations(
         Instance instance,
-        Dictionary<string, object>? resultUpdatedDataModels,
         IInstanceDataAccessor dataAccessor,
+        List<DataElementChange> changes,
         List<string>? ignoredValidators,
         string? language
     )
     {
-        if (resultUpdatedDataModels is null || resultUpdatedDataModels.Count == 0)
-        {
-            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<string, Dictionary<string, List<ValidationIssue>>>();
+        // 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<string, List<ValidationIssueWithSource>>
+    > PartitionValidationIssuesByDataElement(Dictionary<string, List<ValidationIssueWithSource>> validationIssues)
+    {
+        var updatedValidationIssues = new Dictionary<string, Dictionary<string, List<ValidationIssueWithSource>>>();
+        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)
-            );
-
-            if (validationIssues.Count > 0)
+            foreach (var issue in issuesFromSource)
             {
-                updatedValidationIssues.Add(elementId, validationIssues);
+                if (!updatedValidationIssues.TryGetValue(issue.DataElementId ?? "", out var elementIssues))
+                {
+                    elementIssues = new Dictionary<string, List<ValidationIssueWithSource>>();
+                    updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues;
+                }
+                if (!elementIssues.TryGetValue(validationSource, out var sourceIssues))
+                {
+                    sourceIssues = new List<ValidationIssueWithSource>();
+                    elementIssues[validationSource] = sourceIssues;
+                }
+                sourceIssues.Add(issue);
             }
         }
 
-        var taskId = instance.Process.CurrentTask.ElementId;
-        var validationIssues = await _validationService.ValidateIncrementalFormData(instance, taskId, changes,
-            dataAccessor, ignoredValidators, language);
-
-
-
-
-
-
-
-
         return updatedValidationIssues;
     }
 }

src/Altinn.App.Api/Controllers/DataController.cs

@@ -511,6 +511,13 @@ public async Task<ActionResult<DataPatchResponseMultiple>> 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));
@@ -534,7 +541,7 @@ public async Task<ActionResult<DataPatchResponseMultiple>> PatchFormDataMultiple
                 }
             }
 
-            ServiceResult<DataPatchResultMultiple, DataPatchError> res = await _patchService.ApplyPatches(
+            ServiceResult<DataPatchResult, DataPatchError> res = await _patchService.ApplyPatches(
                 instance,
                 dataPatchRequest.Patches,
                 language,
@@ -556,7 +563,7 @@ await UpdatePresentationTextsOnInstance(
                 return Ok(
                     new DataPatchResponse
                     {
-                        NewDataModel = res.Ok.NewDataModel,
+                        NewDataModel = res.Ok.NewDataModels,
                         ValidationIssues = res.Ok.ValidationIssues
                     }
                 );
@@ -568,7 +575,7 @@ await UpdatePresentationTextsOnInstance(
         {
             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}"
             );
         }
     }

src/Altinn.App.Api/Controllers/ProcessController.cs

@@ -248,7 +248,7 @@ [FromRoute] Guid instanceGuid
         string? language
     )
     {
-        var dataAcceesor = new CachedInstanceDataAccessor(_dataClient, _appMetadata, _appModel);
+        var dataAcceesor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);
         var validationIssues = await _validationService.ValidateInstanceAtTask(
             instance,
             currentTaskId,

src/Altinn.App.Api/Controllers/ValidateController.cs

@@ -76,7 +76,7 @@ public async Task<IActionResult> ValidateInstance(
 
         try
         {
-            var dataAccessor = new CachedInstanceDataAccessor(_dataClient, _appMetadata, _appModel);
+            var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);
             List<ValidationIssueWithSource> messages = await _validationService.ValidateInstanceAtTask(
                 instance,
                 taskId,
@@ -148,7 +148,7 @@ public async Task<IActionResult> ValidateData(
             throw new ValidationException("Unknown element type.");
         }
 
-        var dataAccessor = new CachedInstanceDataAccessor(_dataClient, _appMetadata, _appModel);
+        var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);
 
         // TODO: Consider filtering so that only relevant issues are reported.
         messages.AddRange(

src/Altinn.App.Api/Models/DataPatchResponse.cs

@@ -11,7 +11,7 @@ public class DataPatchResponse
     /// <summary>
     /// The validation issues that were found during the patch operation.
     /// </summary>
-    public required Dictionary<string, List<ValidationIssue>> ValidationIssues { get; init; }
+    public required Dictionary<string, List<ValidationIssueWithSource>> ValidationIssues { get; init; }
 
     /// <summary>
     /// The current data model after the patch operation.

src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs

@@ -11,7 +11,7 @@ public class DataPatchResponseMultiple
     /// <summary>
     /// The validation issues that were found during the patch operation.
     /// </summary>
-    public required Dictionary<string, List<ValidationIssue>> ValidationIssues { get; init; }
+    public required Dictionary<string, List<ValidationIssueWithSource>> ValidationIssues { get; init; }
 
     /// <summary>
     /// The current data in all data models updated by the patch operation.

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
     /// </summary>
     [JsonPropertyName("updatedValidationIssues")]
-    public Dictionary<string, Dictionary<string, List<ValidationIssue>>>? UpdatedValidationIssues { get; set; }
+    public Dictionary<
+        string,
+        Dictionary<string, List<ValidationIssueWithSource>>
+    >? UpdatedValidationIssues { get; set; }
 
     /// <summary>
     /// Actions the client should perform after action has been performed backend

src/Altinn.App.Core/Features/IValidator.cs

@@ -3,39 +3,84 @@
 
 namespace Altinn.App.Core.Features;
 
+/// <summary>
+/// Main interface for validation of instances
+/// </summary>
 public interface IValidator
 {
+    /// <summary>
+    /// The task id for the task that the validator is associated with or "*" if the validator should run for all tasks.
+    /// </summary>
     public string TaskId { get; }
 
+    /// <summary>
+    /// Unique string that identifies the source of the validation issues from this validator
+    /// Used for incremental validation. Default implementation should typically work.
+    /// </summary>
     public string ValidationSource => $"{GetType().FullName}-{TaskId}";
 
+    /// <summary>
+    ///
+    /// </summary>
+    /// <param name="instance">The instance to validate</param>
+    /// <param name="taskId">The current task. </param>
+    /// <param name="language">Language for messages, if the messages are too dynamic for the translation system</param>
+    /// <param name="instanceDataAccessor">Use this to access data from other data elements</param>
+    /// <returns></returns>
     public Task<List<ValidationIssue>> Validate(
         Instance instance,
         string taskId,
         string? language,
         IInstanceDataAccessor instanceDataAccessor
     );
+
+    /// <summary>
+    /// 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.
+    /// </summary>
+    /// <param name="instance">The instance to validate</param>
+    /// <param name="taskId">The current task ID</param>
+    /// <param name="changes">List of changed data elements with current and previous value</param>
+    /// <param name="instanceDataAccessor">Use this to access data from other data elements</param>
+    /// <returns></returns>
+    public Task<bool> HasRelevantChanges(
+        Instance instance,
+        string taskId,
+        List<DataElementChange> changes,
+        IInstanceDataAccessor instanceDataAccessor
+    );
 }
 
+/// <summary>
+/// Represents a change in a data element with current and previous deserialized data
+/// </summary>
 public class DataElementChange
 {
-    public DataElement DataElement { get; init; }
-    public object PreviousValue { get; init; }
-    public object CurrentValue { get; init; }
-}
+    /// <summary>
+    /// The data element the change is related to
+    /// </summary>
+    public required DataElement DataElement { get; init; }
 
-public interface IInstanceDataAccessor
-{
-    Task<object> Get(Instance instance, DataElement dataElement);
+    /// <summary>
+    /// The state of the data element before the change
+    /// </summary>
+    public required object PreviousValue { get; init; }
+
+    /// <summary>
+    /// The state of the data element after the change
+    /// </summary>
+    public required object CurrentValue { get; init; }
 }
 
-public interface IIncrementalValidator : IValidator
+/// <summary>
+/// Service for accessing data from other data elements in the
+/// </summary>
+public interface IInstanceDataAccessor
 {
-    public Task<bool> HasRelevantChanges(
-        Instance instance,
-        string taskId,
-        string? language,
-        List<DataElementChange> changes,
-        IInstanceDataAccessor instanceDataAccessor
-    );
+    /// <summary>
+    /// Get the actual data represented in the data element.
+    /// </summary>
+    /// <param name="dataElement">The data element to retrieve. Must be from the instance that is currently active</param>
+    /// <returns>The deserialized data model for this data element or a stream for binary elements</returns>
+    Task<object> Get(DataElement dataElement);
 }