Skip to content

Commit

Permalink
Restructure validation to support incremental validation for multiple…
Browse files Browse the repository at this point in the history 
… data models
  • Loading branch information
@ivarne
ivarne committed Aug 15, 2024
1 parent 96cc833 commit 29ed4dc
Show file tree
Hide file tree
Showing 54 changed files with 1,978 additions and 1,080 deletions.
134 changes: 74 additions & 60 deletions src/Altinn.App.Api/Controllers/ActionsController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -33,23 +34,19 @@ public class ActionsController : ControllerBase
private readonly IValidationService _validationService;
private readonly IDataClient _dataClient;
private readonly IAppMetadata _appMetadata;
private readonly IAppModel _appModel;

/// <summary>
/// Create new instance of the <see cref="ActionsController"/> class
/// </summary>
/// <param name="authorization">The authorization service</param>
/// <param name="instanceClient">The instance client</param>
/// <param name="userActionService">The user action service</param>
/// <param name="validationService">Service for performing validations of user data</param>
/// <param name="dataClient">Client for accessing data in storage</param>
/// <param name="appMetadata">Service for getting application metadata</param>
public ActionsController(
IAuthorizationService authorization,
IInstanceClient instanceClient,
UserActionService userActionService,
IValidationService validationService,
IDataClient dataClient,
IAppMetadata appMetadata
IAppMetadata appMetadata,
IAppModel appModel
)
{
_authorization = authorization;
Expand All @@ -58,6 +55,7 @@ IAppMetadata appMetadata
_validationService = validationService;
_dataClient = dataClient;
_appMetadata = appMetadata;
_appModel = appModel;
}

/// <summary>
Expand Down Expand Up @@ -162,41 +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
);
}

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<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,
Expand All @@ -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<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 < 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<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)
);

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<string, List<ValidationIssueWithSource>>();
updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues;
}
if (!elementIssues.TryGetValue(validationSource, out var sourceIssues))
{
sourceIssues = new List<ValidationIssueWithSource>();
elementIssues[validationSource] = sourceIssues;
}
sourceIssues.Add(issue);
}
}

Expand Down
108 changes: 85 additions & 23 deletions src/Altinn.App.Api/Controllers/DataController.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,53 @@ public async Task<ActionResult<DataPatchResponse>> 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");
}

/// <summary>
/// Updates an existing form data element with a patch of changes.
/// </summary>
/// <param name="org">unique identfier of the organisation responsible for the app</param>
/// <param name="app">application identifier which is unique within an organisation</param>
/// <param name="instanceOwnerPartyId">unique id of the party that is the owner of the instance</param>
/// <param name="instanceGuid">unique id to identify the instance</param>
/// <param name="dataPatchRequest">Container object for the <see cref="JsonPatch" /> and list of ignored validators</param>
/// <param name="language">The language selected by the user.</param>
/// <returns>A response object with the new full model and validation issues from all the groups that run</returns>
[Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)]
[HttpPatch("")]
[ProducesResponseType(typeof(DataPatchResponseMultiple), 200)]
[ProducesResponseType(typeof(ProblemDetails), 409)]
[ProducesResponseType(typeof(ProblemDetails), 422)]
public async Task<ActionResult<DataPatchResponseMultiple>> PatchFormDataMultiple(
[FromRoute] string org,
[FromRoute] string app,
[FromRoute] int instanceOwnerPartyId,
[FromRoute] Guid instanceGuid,
[FromBody] DataPatchRequestMultiple dataPatchRequest,
[FromQuery] string? language = null
)
{
try
{
Expand All @@ -464,44 +511,59 @@ public async Task<ActionResult<DataPatchResponse>> 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<DataPatchResult, DataPatchError> res = await _patchService.ApplyPatch(
ServiceResult<DataPatchResult, DataPatchError> 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
}
);
Expand All @@ -513,7 +575,7 @@ public async Task<ActionResult<DataPatchResponse>> 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}"
);
}
}
Expand Down
Loading

0 comments on commit 29ed4dc

Please sign in to comment.