diff --git a/src/Altinn.App.Api/Controllers/ActionsController.cs b/src/Altinn.App.Api/Controllers/ActionsController.cs index f2bb4abf9..6c9de7e11 100644 --- a/src/Altinn.App.Api/Controllers/ActionsController.cs +++ b/src/Altinn.App.Api/Controllers/ActionsController.cs @@ -126,11 +126,11 @@ public async Task> Perform( return Forbid(); } - var dataMutator = new CachedInstanceDataAccessor( + var dataMutator = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerialization ); UserActionContext userActionContext = @@ -179,7 +179,6 @@ public async Task> Perform( var saveTask = dataMutator.SaveChanges(changes); var validationIssues = await GetIncrementalValidations( - instance, dataMutator, changes, actionRequest.IgnoredValidators, @@ -187,7 +186,10 @@ public async Task> Perform( ); await saveTask; - var updatedDataModels = changes.ToDictionary(c => c.DataElement.Id, c => c.CurrentFormData); + var updatedDataModels = changes.FormDataChanges.ToDictionary( + c => c.DataElementIdentifier.Id, + c => c.CurrentFormData + ); #pragma warning disable CS0618 // Type or member is obsolete if (result.UpdatedDataModels is { Count: > 0 }) @@ -216,16 +218,16 @@ private async Task> >?> GetIncrementalValidations( - Instance instance, IInstanceDataAccessor dataAccessor, - List changes, + DataElementChanges changes, List? ignoredValidators, string? language ) { - var taskId = instance.Process.CurrentTask.ElementId; + var taskId = + dataAccessor.Instance.Process?.CurrentTask?.ElementId + ?? throw new Exception("Unable to validate instance without a started process."); var validationIssues = await _validationService.ValidateIncrementalFormData( - instance, dataAccessor, taskId, changes, diff --git a/src/Altinn.App.Api/Controllers/DataController.cs b/src/Altinn.App.Api/Controllers/DataController.cs index f250f40a1..6cc94fc03 100644 --- a/src/Altinn.App.Api/Controllers/DataController.cs +++ b/src/Altinn.App.Api/Controllers/DataController.cs @@ -47,7 +47,6 @@ public class DataController : ControllerBase private readonly IInstanceClient _instanceClient; private readonly IInstantiationProcessor _instantiationProcessor; private readonly IAppModel _appModel; - private readonly IAppResources _appResourcesService; private readonly IAppMetadata _appMetadata; private readonly IPrefill _prefillService; private readonly IFileAnalysisService _fileAnalyserService; @@ -67,7 +66,6 @@ public class DataController : ControllerBase /// A service with access to data storage. /// Services implementing logic during data read/write /// Service for generating app model - /// The apps resource service /// The app metadata service /// The feature manager controlling enabled features. /// A service with prefill related logic. @@ -82,7 +80,6 @@ public DataController( IDataClient dataClient, IEnumerable dataProcessors, IAppModel appModel, - IAppResources appResourcesService, IPrefill prefillService, IFileAnalysisService fileAnalyserService, IFileValidationService fileValidationService, @@ -99,7 +96,6 @@ ModelSerializationService modelDeserializer _dataClient = dataClient; _dataProcessors = dataProcessors; _appModel = appModel; - _appResourcesService = appResourcesService; _appMetadata = appMetadata; _prefillService = prefillService; _fileAnalyserService = fileAnalyserService; @@ -117,12 +113,15 @@ ModelSerializationService modelDeserializer /// unique id of the party that this the owner of the instance /// unique id to identify the instance /// identifies the data element type to create - /// A list is returned if multiple elements are created. + /// A Created response with the DataElement or a BadRequest with a list of issues [Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)] [HttpPost] [DisableFormValueModelBinding] [RequestSizeLimit(REQUEST_SIZE_LIMIT)] [ProducesResponseType(typeof(DataElement), 201)] + [Obsolete( + "Use the POST method with the dataType parameter in url instead, to get more sensible BadRequests when validation fails." + )] public async Task Create( [FromRoute] string org, [FromRoute] string app, @@ -131,99 +130,335 @@ public async Task Create( [FromQuery] string dataType ) { - /* The Body of the request is read much later when it has been made sure it is worth it. */ + var response = await PostImpl( + org, + app, + instanceOwnerPartyId, + instanceGuid, + dataType, + ignoredValidatorsString: null, + language: null + ); + if (response.Success) + { + var dataElement = + response.Ok.Instance.Data.Find(d => Guid.Parse(d.Id) == response.Ok.NewDataElementId) + ?? throw new InvalidOperationException("Data element not found in instance after creation"); + return Created(dataElement.SelfLinks.Apps, dataElement); + } + // Special case for compatibility with old clients + if (response.Error is DataPostErrorResponse fileValidationError) + { + return BadRequest(await GetErrorDetails(fileValidationError.UploadValidationIssues)); + } + if (response.Error.Status == (int)HttpStatusCode.BadRequest) + { + // Old clients will expect BadRequest to have a list of issues or a string + // not problem details. + return BadRequest( + await GetErrorDetails( + [ + new ValidationIssueWithSource + { + Description = response.Error.Detail, + Code = response.Error.Title, + Severity = ValidationIssueSeverity.Error, + Source = response.Error.Type ?? "DataController" + } + ] + ) + ); + } + + return Problem(response.Error); + } + + /// + /// Creates and instantiates a data element of a given element-type. Clients can upload the data element in the request content. + /// + /// unique identifier of the organisation responsible for the app + /// application identifier which is unique within an organisation + /// unique id of the party that this the owner of the instance + /// unique id to identify the instance + /// identifies the data element type to create + /// comma separated string of validators to ignore + /// The currently active user language + /// DataPostResponse on success and an extended problemDetails with validation issues if upload validation fails + [Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)] + [HttpPost("{dataType}")] + [DisableFormValueModelBinding] + [RequestSizeLimit(REQUEST_SIZE_LIMIT)] + [ProducesResponseType(typeof(DataPostResponse), (int)HttpStatusCode.Created)] + [ProducesResponseType(typeof(ProblemDetails), (int)HttpStatusCode.Conflict)] + [ProducesResponseType(typeof(DataPostErrorResponse), (int)HttpStatusCode.BadRequest)] + [ProducesResponseType(typeof(ProblemDetails), (int)HttpStatusCode.NotFound)] + public async Task> Post( + [FromRoute] string org, + [FromRoute] string app, + [FromRoute] int instanceOwnerPartyId, + [FromRoute] Guid instanceGuid, + [FromRoute] string dataType, + [FromQuery] string? ignoredValidators = null, + [FromQuery] string? language = null + ) + { + var response = await PostImpl( + org, + app, + instanceOwnerPartyId, + instanceGuid, + dataType, + ignoredValidators, + language + ); + if (response.Success) + { + return response.Ok; + } + + return Problem(response.Error); + } + + private async Task> PostImpl( + string org, + string app, + int instanceOwnerPartyId, + Guid instanceGuid, + string dataTypeString, + string? ignoredValidatorsString, + string? language + ) + { try { - var instanceResult = await GetInstanceDataOrError(org, app, instanceOwnerPartyId, instanceGuid, dataType); + var instanceResult = await GetInstanceDataOrError( + org, + app, + instanceOwnerPartyId, + instanceGuid, + dataTypeString + ); if (!instanceResult.Success) { - return Problem(instanceResult.Error); + return instanceResult.Error; } - var (instance, dataTypeFromMetadata) = instanceResult.Ok; + var (instance, dataType, applicationMetadata) = instanceResult.Ok; - if (DataElementAccessChecker.GetCreateProblem(instance, dataTypeFromMetadata, User) is { } accessProblem) + if (DataElementAccessChecker.GetCreateProblem(instance, dataType, User) is { } accessProblem) { - return Problem(accessProblem); - } - - if (dataTypeFromMetadata.AppLogic?.ClassRef is not null) - { - return await CreateAppModelData(org, app, instance, dataType); + return accessProblem; } - // else, handle the binary upload - (bool validationRestrictionSuccess, List errors) = - DataRestrictionValidation.CompliesWithDataRestrictions(Request, dataTypeFromMetadata); + var dataMutator = new InstanceDataUnitOfWork( + instance, + _dataClient, + _instanceClient, + applicationMetadata, + _modelDeserializer + ); - if (!validationRestrictionSuccess) + // Save data elements with form data + if (dataType.AppLogic?.ClassRef is { } classRef) { - return BadRequest(await GetErrorDetails(errors)); - } + object? appModel; + if (Request.ContentType is null) + { + appModel = _appModel.Create(classRef); + } + else + { + var deserializationResult = await _modelDeserializer.DeserializeSingleFromStream( + Request.Body, + Request.ContentType, + dataType + ); + if (deserializationResult.Success) + { + appModel = deserializationResult.Ok; + } + else + { + return deserializationResult.Error; + } + } - StreamContent streamContent = Request.CreateContentStream(); + // runs prefill from repo configuration if config exists + await _prefillService.PrefillDataModel( + dataMutator.Instance.InstanceOwner.PartyId, + dataType.Id, + appModel + ); + await _instantiationProcessor.DataCreation(dataMutator.Instance, appModel, null); - using Stream fileStream = new MemoryStream(); - await streamContent.CopyToAsync(fileStream); - if (fileStream.Length is 0) + // Just stage the element to be created. We don't get the element id before we call UpdateInstanceData + dataMutator.AddFormDataElement(dataType.Id, appModel); + } + else { - const string errorMessage = "Invalid data provided. Error: The file is zero bytes."; - var error = new ValidationIssue + var createBinaryResult = await CreateBinaryData(dataMutator, dataType); + if (!createBinaryResult.Success) { - Code = ValidationIssueCodes.DataElementCodes.ContentTypeNotAllowed, - Severity = ValidationIssueSeverity.Error, - Description = errorMessage - }; - _logger.LogError(errorMessage); - return BadRequest(await GetErrorDetails([error])); + return createBinaryResult.Error; + } } - bool parseSuccess = Request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues); - string? filename = parseSuccess ? DataRestrictionValidation.GetFileNameFromHeader(headerValues) : null; - - IEnumerable fileAnalysisResults = new List(); - if (FileAnalysisEnabledForDataType(dataTypeFromMetadata)) + var initialChanges = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); + if (initialChanges is not { AllChanges: [var change] }) { - fileAnalysisResults = await _fileAnalyserService.Analyse(dataTypeFromMetadata, fileStream, filename); + throw new InvalidOperationException("Expected exactly one change in initialChanges"); } - var fileValidationSuccess = true; - List validationIssues = []; - if (FileValidationEnabledForDataType(dataTypeFromMetadata)) + // Mutates initialChanges and to add DataElement Type = Created + await dataMutator.UpdateInstanceData(initialChanges); + + var newDataElement = + change.DataElement + ?? throw new InvalidOperationException("DataElement not set in dataMutator.UpdateInstanceData"); + + await _patchService.RunDataProcessors( + dataMutator, + initialChanges, + instance.Process.CurrentTask.ElementId, + language + ); + + var finalChanges = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); + await dataMutator.UpdateInstanceData(finalChanges); + var saveTask = dataMutator.SaveChanges(finalChanges); + List validationIssues = []; + if (ignoredValidatorsString is not null) { - (fileValidationSuccess, validationIssues) = await _fileValidationService.Validate( - dataTypeFromMetadata, - fileAnalysisResults + var ignoredValidators = ignoredValidatorsString + .Split(',') + .Where(v => !string.IsNullOrEmpty(v)) + .ToList(); + validationIssues = await _patchService.RunIncrementalValidation( + dataMutator, + instance.Process.CurrentTask.ElementId, + finalChanges, + ignoredValidators, + language ); } - if (!fileValidationSuccess) + await saveTask; + SelfLinkHelper.SetInstanceAppSelfLinks(instance, Request); + + return new DataPostResponse { - return BadRequest(await GetErrorDetails(validationIssues)); - } + Instance = instance, + NewDataElementId = Guid.Parse(newDataElement.Id), + NewDataModels = finalChanges + .FormDataChanges.Select(formDataChange => new DataModelPairResponse( + formDataChange.DataElementIdentifier.Guid, + formDataChange.CurrentFormData + )) + .ToList(), + ValidationIssues = validationIssues, + }; + } + catch (PlatformHttpException e) + { + return new ProblemDetails() + { + Title = "Platform error", + Detail = e.Message, + Status = (int)e.Response.StatusCode, + }; + } + } + + private async Task> CreateBinaryData( + InstanceDataUnitOfWork dataMutator, + DataType dataTypeFromMetadata + ) + { + (bool validationRestrictionSuccess, List errors) = + DataRestrictionValidation.CompliesWithDataRestrictions(Request, dataTypeFromMetadata); + + if (!validationRestrictionSuccess) + { + var issuesWithSource = errors + .Select(e => ValidationIssueWithSource.FromIssue(e, "DataRestrictionValidation", false)) + .ToList(); + return new DataPostErrorResponse("Common checks failed", issuesWithSource); + } - if (streamContent.Headers.ContentType is null) + if (Request.ContentType is null) + { + return new ProblemDetails() { - return StatusCode((int)HttpStatusCode.InternalServerError, "Content-Type not defined"); - } + Title = "Missing content type", + Detail = "The request is missing a content type header.", + Status = (int)HttpStatusCode.BadRequest, + }; + } - fileStream.Seek(0, SeekOrigin.Begin); - return await CreateBinaryData( - instance, - dataType, - streamContent.Headers.ContentType.ToString(), - filename, - fileStream - ); + var (bytes, actualLength) = await Request.ReadBodyAsByteArrayAsync(dataTypeFromMetadata.MaxSize * 1024 * 1024); + if (bytes is null) + { + return new ProblemDetails() + { + Title = "Request too large", + Detail = + $"The request body is too large. The content length is {actualLength} bytes, which exceeds the limit of {dataTypeFromMetadata.MaxSize} MB", + Status = (int)HttpStatusCode.RequestEntityTooLarge, + }; } - catch (PlatformHttpException e) + if (bytes.Length is 0) { - return HandlePlatformHttpException( - e, - $"Cannot create data element of {dataType} for {instanceOwnerPartyId}/{instanceGuid}" + return new ProblemDetails() + { + Title = "Invalid data", + Detail = "Invalid data provided. Error: The file is zero bytes.", + Status = (int)HttpStatusCode.BadRequest, + }; + } + + bool parseSuccess = Request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues); + string? filename = parseSuccess ? DataRestrictionValidation.GetFileNameFromHeader(headerValues) : null; + + var analysisAndValidationProblem = await RunFileAnalysisAndValidation(dataTypeFromMetadata, bytes, filename); + if (analysisAndValidationProblem != null) + { + return analysisAndValidationProblem; + } + + return dataMutator.AddBinaryDataElement(dataTypeFromMetadata.Id, Request.ContentType, filename, bytes); + } + + private async Task RunFileAnalysisAndValidation( + DataType dataTypeFromMetadata, + byte[] bytes, + string? filename + ) + { + List fileAnalysisResults = []; + if (FileAnalysisEnabledForDataType(dataTypeFromMetadata)) + { + fileAnalysisResults = ( + await _fileAnalyserService.Analyse(dataTypeFromMetadata, new MemoryAsStream(bytes), filename) + ).ToList(); + } + + var fileValidationSuccess = true; + List validationIssues = []; + if (FileValidationEnabledForDataType(dataTypeFromMetadata)) + { + (fileValidationSuccess, validationIssues) = await _fileValidationService.Validate( + dataTypeFromMetadata, + fileAnalysisResults ); } + + if (!fileValidationSuccess) + { + return new DataPostErrorResponse("File validation failed", validationIssues); + } + + return null; } /// @@ -234,7 +469,7 @@ [FromQuery] string dataType /// and the developer need to opt in to the new behaviour. Json object are by default /// returned as part of file validation which is a new feature. /// - private async Task GetErrorDetails(List errors) + private async Task GetErrorDetails(List errors) { return await _featureManager.IsEnabledAsync(FeatureFlags.JsonObjectInDataResponse) ? errors @@ -365,15 +600,7 @@ public async Task Put( return await PutFormData(instance, dataElement, dataType, language); } - (bool validationRestrictionSuccess, List errors) = - DataRestrictionValidation.CompliesWithDataRestrictions(Request, dataType); - - if (!validationRestrictionSuccess) - { - return BadRequest(await GetErrorDetails(errors)); - } - - return await PutBinaryData(instanceOwnerPartyId, instanceGuid, dataGuid); + return await PutBinaryData(instanceOwnerPartyId, instanceGuid, dataGuid, dataType); } catch (PlatformHttpException e) { @@ -500,10 +727,7 @@ public async Task> PatchFormDataMultiple { Instance = res.Ok.Instance, NewDataModels = res - .Ok.UpdatedData.Select(d => new DataPatchResponseMultiple.DataModelPairResponse( - d.Identifier.Guid, - d.Data - )) + .Ok.UpdatedData.Select(d => new DataModelPairResponse(d.Identifier.Guid, d.Data)) .ToList(), ValidationIssues = res.Ok.ValidationIssues } @@ -529,6 +753,7 @@ public async Task> PatchFormDataMultiple /// unique id of the party that is the owner of the instance /// unique id to identify the instance /// unique id to identify the data element to update + /// The currently active language /// The updated data element. [Authorize(Policy = AuthzConstants.POLICY_INSTANCE_WRITE)] [HttpDelete("{dataGuid:guid}")] @@ -537,7 +762,8 @@ public async Task Delete( [FromRoute] string app, [FromRoute] int instanceOwnerPartyId, [FromRoute] Guid instanceGuid, - [FromRoute] Guid dataGuid + [FromRoute] Guid dataGuid, + [FromQuery] string? language = null ) { try @@ -547,14 +773,37 @@ [FromRoute] Guid dataGuid { return Problem(instanceResult.Error); } - var (instance, dataType, _) = instanceResult.Ok; + var (instance, dataType, dataElement) = instanceResult.Ok; if (DataElementAccessChecker.GetDeleteProblem(instance, dataType, dataGuid, User) is { } accessProblem) { return Problem(accessProblem); } - return await DeleteBinaryData(org, app, instanceOwnerPartyId, instanceGuid, dataGuid); + var taskId = + instance.Process?.CurrentTask?.ElementId + ?? throw new InvalidOperationException("Instance have no process"); + + var dataMutator = new InstanceDataUnitOfWork( + instance, + _dataClient, + _instanceClient, + await _appMetadata.GetApplicationMetadata(), + _modelDeserializer + ); + + dataMutator.RemoveDataElement(dataElement); + + // Get the single change for running data processors + var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: false); + await _patchService.RunDataProcessors(dataMutator, changes, taskId, language); + + // Get the updated changes for saving + changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: false); + await dataMutator.UpdateInstanceData(changes); + await dataMutator.SaveChanges(changes); + + return Ok(); } catch (PlatformHttpException e) { @@ -582,87 +831,6 @@ private ObjectResult ExceptionResponse(Exception exception, string message) return StatusCode((int)HttpStatusCode.InternalServerError, $"{message}"); } - private async Task CreateBinaryData( - Instance instanceBefore, - string dataType, - string contentType, - string? filename, - Stream fileStream - ) - { - int instanceOwnerPartyId = int.Parse(instanceBefore.Id.Split("/")[0], CultureInfo.InvariantCulture); - Guid instanceGuid = Guid.Parse(instanceBefore.Id.Split("/")[1]); - - DataElement dataElement = await _dataClient.InsertBinaryData( - instanceBefore.Id, - dataType, - contentType, - filename, - fileStream - ); - - if (Guid.Parse(dataElement.Id) == Guid.Empty) - { - return StatusCode( - (int)HttpStatusCode.InternalServerError, - $"Cannot store form attachment on instance {instanceOwnerPartyId}/{instanceGuid}" - ); - } - - SelfLinkHelper.SetDataAppSelfLinks(instanceOwnerPartyId, instanceGuid, dataElement, Request); - return Created(dataElement.SelfLinks.Apps, dataElement); - } - - private async Task CreateAppModelData(string org, string app, Instance instance, string dataType) - { - Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); - - object? appModel; - - string classRef = _appResourcesService.GetClassRefForLogicDataType(dataType); - - if (Request.ContentType is null) - { - appModel = _appModel.Create(classRef); - } - else - { - ModelDeserializer deserializer = new ModelDeserializer(_logger, _appModel.GetModelType(classRef)); - appModel = await deserializer.DeserializeAsync(Request.Body, Request.ContentType); - - if (!string.IsNullOrEmpty(deserializer.Error) || appModel is null) - { - return BadRequest(deserializer.Error); - } - } - - // runs prefill from repo configuration if config exists - await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, dataType, appModel); - - await _instantiationProcessor.DataCreation(instance, appModel, null); - - await UpdatePresentationTextsOnInstance(instance, dataType, appModel); - await UpdateDataValuesOnInstance(instance, dataType, appModel); - - var instanceOwnerPartyId = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture); - - ObjectUtils.InitializeAltinnRowId(appModel); - ObjectUtils.PrepareModelForXmlStorage(appModel); - - DataElement dataElement = await _dataClient.InsertFormData( - appModel, - instanceGuid, - _appModel.GetModelType(classRef), - org, - app, - instanceOwnerPartyId, - dataType - ); - SelfLinkHelper.SetDataAppSelfLinks(instanceOwnerPartyId, instanceGuid, dataElement, Request); - - return Created(dataElement.SelfLinks.Apps, dataElement); - } - /// /// Gets a data element from storage. /// @@ -692,34 +860,6 @@ DataElement dataElement return NotFound(); } - private async Task DeleteBinaryData( - string org, - string app, - int instanceOwnerId, - Guid instanceGuid, - Guid dataGuid - ) - { - bool successfullyDeleted = await _dataClient.DeleteData( - org, - app, - instanceOwnerId, - instanceGuid, - dataGuid, - false - ); - - if (successfullyDeleted) - { - return Ok(); - } - - return StatusCode( - (int)HttpStatusCode.InternalServerError, - $"Something went wrong when deleting data element {dataGuid} for instance {instanceGuid}" - ); - } - private async Task GetDataType(DataElement element) { Application application = await _appMetadata.GetApplicationMetadata(); @@ -815,25 +955,68 @@ await _dataClient.UpdateData( return Ok(appModel); } - private async Task PutBinaryData(int instanceOwnerPartyId, Guid instanceGuid, Guid dataGuid) + private async Task PutBinaryData( + int instanceOwnerPartyId, + Guid instanceGuid, + Guid dataGuid, + DataType dataType + ) { - if (Request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues)) + //TODO: Consider having a rule that disables PUT for binary data elements. + + (bool validationRestrictionSuccess, List errors) = + DataRestrictionValidation.CompliesWithDataRestrictions(Request, dataType); + + if (!validationRestrictionSuccess) { - var contentDispositionHeader = ContentDispositionHeaderValue.Parse(headerValues.ToString()); - _logger.LogInformation("Content-Disposition: {ContentDisposition}", headerValues.ToString()); - DataElement dataElement = await _dataClient.UpdateBinaryData( - new InstanceIdentifier(instanceOwnerPartyId, instanceGuid), - Request.ContentType, - contentDispositionHeader.FileName.ToString(), - dataGuid, - Request.Body + return BadRequest( + await GetErrorDetails( + errors + .Select(e => ValidationIssueWithSource.FromIssue(e, "DataRestrictionValidation", false)) + .ToList() + ) ); - SelfLinkHelper.SetDataAppSelfLinks(instanceOwnerPartyId, instanceGuid, dataElement, Request); + } - return Created(dataElement.SelfLinks.Apps, dataElement); + if (!Request.Headers.TryGetValue("Content-Disposition", out StringValues headerValues)) + { + return BadRequest("Invalid data provided. Error: The request must include a Content-Disposition header"); } - return BadRequest("Invalid data provided. Error: The request must include a Content-Disposition header"); + var contentDispositionHeader = ContentDispositionHeaderValue.Parse(headerValues.ToString()); + _logger.LogInformation("Content-Disposition: {ContentDisposition}", headerValues.ToString()); + + var (bytes, actualLength) = await Request.ReadBodyAsByteArrayAsync( + dataType.MaxSize * 1024 * 1024 ?? REQUEST_SIZE_LIMIT + ); + if (bytes is null) + { + return BadRequest( + $"The request body is too large. The content length is {actualLength} bytes, which exceeds the limit of {dataType.MaxSize} MB" + ); + } + + var analysisAndValidationProblem = await RunFileAnalysisAndValidation( + dataType, + bytes, + contentDispositionHeader.FileName.ToString() + ); + if (analysisAndValidationProblem != null) + { + return Problem(analysisAndValidationProblem); + } + + DataElement dataElement = await _dataClient.UpdateBinaryData( + new InstanceIdentifier(instanceOwnerPartyId, instanceGuid), + Request.ContentType, + contentDispositionHeader.FileName.ToString(), + dataGuid, + new MemoryAsStream(bytes) + ); + + SelfLinkHelper.SetDataAppSelfLinks(instanceOwnerPartyId, instanceGuid, dataElement, Request); + + return Created(dataElement.SelfLinks.Apps, dataElement); } private async Task PutFormData( @@ -855,11 +1038,11 @@ private async Task PutFormData( var serviceModel = deserializationResult.Ok; - var dataMutator = new CachedInstanceDataAccessor( + var dataMutator = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelDeserializer ); // Get the previous service model for dataProcessing to work @@ -867,21 +1050,23 @@ private async Task PutFormData( // Set the new service model so that dataAccessors see the new state dataMutator.SetFormData(dataElement, serviceModel); - List changesFromRequest = - [ - new() - { - DataElement = dataElement, - PreviousFormData = oldServiceModel, - CurrentFormData = serviceModel, - } - ]; + var requestedChange = new FormDataChange() + { + Type = ChangeType.Updated, + DataElement = dataElement, + ContentType = dataElement.ContentType, + DataType = dataType, + PreviousFormData = oldServiceModel, + CurrentFormData = serviceModel, + PreviousBinaryData = await dataMutator.GetBinaryData(dataElement), + CurrentBinaryData = null, // We don't serialize to xml before running data processors + }; // Run data processors keeping track of changes for diff return var jsonBeforeDataProcessors = JsonSerializer.Serialize(serviceModel); await _patchService.RunDataProcessors( dataMutator, - changesFromRequest, + new DataElementChanges([requestedChange]), instance.Process.CurrentTask.ElementId, language ); @@ -912,44 +1097,6 @@ await _patchService.RunDataProcessors( return Created(dataUrl, dataElement); } - private async Task UpdatePresentationTextsOnInstance(Instance instance, string dataType, object serviceModel) - { - var updatedValues = DataHelper.GetUpdatedDataValues( - (await _appMetadata.GetApplicationMetadata()).PresentationFields, - instance.PresentationTexts, - dataType, - serviceModel - ); - - if (updatedValues.Count > 0) - { - await _instanceClient.UpdatePresentationTexts( - int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), - Guid.Parse(instance.Id.Split("/")[1]), - new PresentationTexts { Texts = updatedValues } - ); - } - } - - private async Task UpdateDataValuesOnInstance(Instance instance, string dataType, object serviceModel) - { - var updatedValues = DataHelper.GetUpdatedDataValues( - (await _appMetadata.GetApplicationMetadata()).DataFields, - instance.DataValues, - dataType, - serviceModel - ); - - if (updatedValues.Count > 0) - { - await _instanceClient.UpdateDataValues( - int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), - Guid.Parse(instance.Id.Split("/")[1]), - new DataValues { Values = updatedValues } - ); - } - } - private ActionResult HandlePlatformHttpException(PlatformHttpException e, string defaultMessage) { return e.Response.StatusCode switch @@ -1114,13 +1261,9 @@ IEnumerable dataElementGuids } } - private async Task> GetInstanceDataOrError( - string org, - string app, - int instanceOwnerPartyId, - Guid instanceGuid, - string dataTypeId - ) + private async Task< + ServiceResult<(Instance instance, DataType dataType, ApplicationMetadata applicationMetadata), ProblemDetails> + > GetInstanceDataOrError(string org, string app, int instanceOwnerPartyId, Guid instanceGuid, string dataTypeId) { try { @@ -1148,7 +1291,7 @@ string dataTypeId }; } - return (instance, dataType); + return (instance, dataType, application); } catch (PlatformHttpException e) { diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index 1cfca75b3..851ea2ccc 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -1,6 +1,6 @@ using System.Globalization; using System.Net; -using System.Text; +using System.Text.Json; using Altinn.App.Api.Helpers.RequestHandling; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Mappers; @@ -16,6 +16,7 @@ using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Events; using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Prefill; using Altinn.App.Core.Internal.Process; using Altinn.App.Core.Internal.Profile; @@ -67,6 +68,9 @@ public class InstancesController : ControllerBase private readonly IProcessEngine _processEngine; private readonly IOrganizationClient _orgClient; private readonly IHostEnvironment _env; + private readonly ModelSerializationService _serializationService; + private readonly IPatchService _patchService; + private static readonly JsonSerializerOptions _jsonSerializerOptionsWeb = new(JsonSerializerDefaults.Web); private const long RequestSizeLimit = 2000 * 1024 * 1024; /// @@ -88,7 +92,9 @@ public InstancesController( IProfileClient profileClient, IProcessEngine processEngine, IOrganizationClient orgClient, - IHostEnvironment env + IHostEnvironment env, + ModelSerializationService serializationService, + IPatchService patchService ) { _logger = logger; @@ -107,6 +113,8 @@ IHostEnvironment env _processEngine = processEngine; _orgClient = orgClient; _env = env; + _serializationService = serializationService; + _patchService = patchService; } /// @@ -171,6 +179,7 @@ [FromRoute] Guid instanceGuid /// unique identifier 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 + /// The currently active user language /// the created instance [HttpPost] [DisableFormValueModelBinding] @@ -181,7 +190,8 @@ [FromRoute] Guid instanceGuid public async Task> Post( [FromRoute] string org, [FromRoute] string app, - [FromQuery] int? instanceOwnerPartyId + [FromQuery] int? instanceOwnerPartyId, + [FromQuery] string? language = null ) { if (string.IsNullOrEmpty(org)) @@ -198,15 +208,16 @@ [FromQuery] int? instanceOwnerPartyId if (VerifyInstantiationPermissions(application, org, app) is { } verificationResult) return verificationResult; - MultipartRequestReader parsedRequest = new MultipartRequestReader(Request); - await parsedRequest.Read(); + var readResult = await MultipartRequestReader.Read(Request); - if (parsedRequest.Errors.Count != 0) + if (!readResult.Success) { - return BadRequest($"Error when reading content: {JsonConvert.SerializeObject(parsedRequest.Errors)}"); + return StatusCode(readResult.Error.Status ?? 500, readResult.Error); } - Instance? instanceTemplate = await ExtractInstanceTemplate(parsedRequest); + var requestParts = readResult.Ok; + + Instance? instanceTemplate = ExtractInstanceTemplate(requestParts); if (instanceOwnerPartyId is null && instanceTemplate is null) { @@ -254,7 +265,7 @@ [FromQuery] int? instanceOwnerPartyId } RequestPartValidator requestValidator = new(application); - string? multipartError = requestValidator.ValidateParts(parsedRequest.Parts); + string? multipartError = requestValidator.ValidateParts(requestParts); if (!string.IsNullOrEmpty(multipartError)) { @@ -334,7 +345,11 @@ [FromQuery] int? instanceOwnerPartyId try { - await StorePrefillParts(instance, application, parsedRequest.Parts); + var prefillProblem = await StorePrefillParts(instance, application, requestParts, language); + if (prefillProblem is not null) + { + return StatusCode(prefillProblem.Status ?? 500, prefillProblem); + } // get the updated instance instance = await _instanceClient.GetInstance( @@ -1105,15 +1120,24 @@ string action } } - private async Task StorePrefillParts(Instance instance, ApplicationMetadata appInfo, List parts) + private async Task StorePrefillParts( + Instance instance, + ApplicationMetadata appInfo, + List parts, + string? language + ) { - Guid instanceGuid = Guid.Parse(instance.Id.Split("/")[1]); - int instanceOwnerIdAsInt = int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture); - string org = instance.Org; - string app = instance.AppId.Split("/")[1]; + var dataMutator = new InstanceDataUnitOfWork( + instance, + _dataClient, + _instanceClient, + appInfo, + _serializationService + ); - foreach (RequestPart part in parts) + for (int partIndex = 0; partIndex < parts.Count; partIndex++) { + RequestPart part = parts[partIndex]; // NOTE: part.Name is nullable on the type here, but `RequestPartValidator.ValidatePart` which is called // further up the stack will error out if it actually null, so we just sanity-check here // and throw if it is null. @@ -1125,68 +1149,57 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI DataType? dataType = appInfo.DataTypes.Find(d => d.Id == part.Name); - DataElement dataElement; - if (dataType?.AppLogic?.ClassRef != null) + if (dataType == null) { - _logger.LogInformation("Storing part {partName}", part.Name); - - Type type; - try - { - type = _appModel.GetModelType(dataType.AppLogic.ClassRef); - } - catch (Exception altinnAppException) + return new ProblemDetails { - throw new ServiceException( - HttpStatusCode.InternalServerError, - $"App.GetAppModelType failed: {altinnAppException.Message}", - altinnAppException - ); - } - - ModelDeserializer deserializer = new ModelDeserializer(_logger, type); - object? data = await deserializer.DeserializeAsync(part.Stream, part.ContentType); + Title = "Data type not found", + Detail = + $"Data type with id {part.Name} from request part {partIndex} not found in application metadata" + }; + } - if (!string.IsNullOrEmpty(deserializer.Error) || data is null) + if (dataType.AppLogic?.ClassRef != null) + { + _logger.LogInformation("Storing part {partName}", part.Name); + var deserializationResult = await _serializationService.DeserializeSingleFromStream( + new MemoryAsStream(part.Bytes), + part.ContentType, + dataType + ); + if (!deserializationResult.Success) { - throw new InvalidOperationException(deserializer.Error); + return deserializationResult.Error; } - await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, part.Name, data); + var data = deserializationResult.Ok; + await _prefillService.PrefillDataModel(instance.InstanceOwner.PartyId, part.Name, data); await _instantiationProcessor.DataCreation(instance, data, null); - ObjectUtils.InitializeAltinnRowId(data); - - dataElement = await _dataClient.InsertFormData( - data, - instanceGuid, - type, - org, - app, - instanceOwnerIdAsInt, - part.Name - ); + dataMutator.AddFormDataElement(dataType.Id, data); } else { - dataElement = await _dataClient.InsertBinaryData( - instance.Id, - part.Name, - part.ContentType, - part.FileName, - part.Stream - ); + _logger.LogInformation("Storing part {partName}", part.Name); + dataMutator.AddBinaryDataElement(dataType.Id, part.ContentType, part.FileName, part.Bytes); } - if (dataElement == null) - { - throw new ServiceException( - HttpStatusCode.InternalServerError, - $"Data service did not return a valid instance metadata when attempt to store data element {part.Name}" - ); - } + var taskId = instance.Process?.CurrentTask?.ElementId; + + if (taskId is null) + throw new InvalidOperationException("There should be a task while initializing data"); + + var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); + await _patchService.RunDataProcessors(dataMutator, changes, taskId, language); + + // Update the changes list if it changed in data processors + changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); + await dataMutator.UpdateInstanceData(changes); + await dataMutator.SaveChanges(changes); } + + return null; } /// @@ -1195,36 +1208,32 @@ private async Task StorePrefillParts(Instance instance, ApplicationMetadata appI /// /// If found the method removes the part corresponding to the instance template form the parts list. /// - /// multipart reader object + /// the parts of the multipart request /// the instance template or null if none is found - private static async Task ExtractInstanceTemplate(MultipartRequestReader reader) + private static Instance? ExtractInstanceTemplate(List parts) { - Instance? instanceTemplate = null; - - RequestPart? instancePart = reader.Parts.Find(part => part.Name == "instance"); + RequestPart? instancePart = parts.Find(part => part.Name == "instance"); // assume that first part with no name is an instanceTemplate if ( instancePart == null - && reader.Parts.Count == 1 - && reader.Parts[0].ContentType.Contains("application/json") - && reader.Parts[0].Name == null + && parts.Count == 1 + && parts[0].ContentType.Contains("application/json") + && parts[0].Name == null + && parts[0].Bytes.Length > 0 ) { - instancePart = reader.Parts[0]; + instancePart = parts[0]; } if (instancePart != null) { - reader.Parts.Remove(instancePart); - - using StreamReader streamReader = new StreamReader(instancePart.Stream, Encoding.UTF8); - string content = await streamReader.ReadToEndAsync(); + parts.Remove(instancePart); - instanceTemplate = JsonConvert.DeserializeObject(content); + return System.Text.Json.JsonSerializer.Deserialize(instancePart.Bytes, _jsonSerializerOptionsWeb); } - return instanceTemplate; + return null; } private async Task RegisterEvent(string eventType, Instance instance) diff --git a/src/Altinn.App.Api/Controllers/ProcessController.cs b/src/Altinn.App.Api/Controllers/ProcessController.cs index c33f9ac23..14f9b000b 100644 --- a/src/Altinn.App.Api/Controllers/ProcessController.cs +++ b/src/Altinn.App.Api/Controllers/ProcessController.cs @@ -249,15 +249,14 @@ [FromRoute] Guid instanceGuid string? language ) { - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerialization ); var validationIssues = await _validationService.ValidateInstanceAtTask( - instance, dataAccessor, currentTaskId, // run full validation ignoredValidators: null, diff --git a/src/Altinn.App.Api/Controllers/ValidateController.cs b/src/Altinn.App.Api/Controllers/ValidateController.cs index a46b6d284..331f4d7b2 100644 --- a/src/Altinn.App.Api/Controllers/ValidateController.cs +++ b/src/Altinn.App.Api/Controllers/ValidateController.cs @@ -4,6 +4,7 @@ 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 Microsoft.AspNetCore.Authorization; @@ -80,16 +81,15 @@ public async Task ValidateInstance( try { - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerialization ); var ignoredSources = ignoredValidators?.Split(',').ToList(); List messages = await _validationService.ValidateInstanceAtTask( - instance, dataAccessor, taskId, ignoredSources, @@ -152,7 +152,7 @@ public async Task ValidateData( throw new ValidationException("Unable to validate data element."); } - Application application = await _appMetadata.GetApplicationMetadata(); + ApplicationMetadata application = await _appMetadata.GetApplicationMetadata(); DataType? dataType = application.DataTypes.FirstOrDefault(et => et.Id == element.DataType); @@ -161,17 +161,16 @@ public async Task ValidateData( throw new ValidationException("Unknown element type."); } - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + application, _modelSerialization ); // Run validations for all data elements, but only return the issues for the specific data element var issues = await _validationService.ValidateInstanceAtTask( - instance, dataAccessor, dataType.TaskId, ignoredValidators: null, diff --git a/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs b/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs index 9aa77216c..6f908e1dc 100644 --- a/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs +++ b/src/Altinn.App.Api/Helpers/RequestHandling/MultipartRequestReader.cs @@ -1,5 +1,7 @@ using System.Globalization; using Altinn.App.Core.Helpers.Extensions; +using Altinn.App.Core.Models.Result; +using Microsoft.AspNetCore.Mvc; using Microsoft.AspNetCore.WebUtilities; using Microsoft.Net.Http.Headers; @@ -8,55 +10,31 @@ namespace Altinn.App.Api.Helpers.RequestHandling; /// /// Represents a reader that can read a multipart http request and split it in data elements. /// -public class MultipartRequestReader +public static class MultipartRequestReader { - private readonly HttpRequest _request; - - /// - /// Initializes a new instance of the class with a . - /// - /// The to be read. - public MultipartRequestReader(HttpRequest request) - { - _request = request; - Parts = new List(); - Errors = new List(); - } - /// /// Gets a value indicating whether the request has multiple parts using the request content type. /// - public bool IsMultipart + private static bool IsMultipart(HttpRequest request) { - get - { - return !string.IsNullOrEmpty(_request.ContentType) - && _request.ContentType.Contains("multipart/", StringComparison.OrdinalIgnoreCase); - } + return !string.IsNullOrEmpty(request.ContentType) + && request.ContentType.Contains("multipart/", StringComparison.OrdinalIgnoreCase); } - /// - /// Gets a list of all parts. - /// - public List Parts { get; } - - /// - /// Gets a list of errors. - /// - public List Errors { get; } - /// /// Process the request and generate parts. /// /// A representing the result of the asynchronous operation. - public async Task Read() + public static async Task, ProblemDetails>> Read(HttpRequest request) { - if (IsMultipart) + List parts = []; + List errors = []; + if (IsMultipart(request)) { int partCounter = 0; try { - MultipartReader reader = new MultipartReader(GetBoundary(), _request.Body); + MultipartReader reader = new MultipartReader(GetBoundary(request), request.Body); MultipartSection? section; while ((section = await reader.ReadNextSectionAsync()) != null) @@ -70,7 +48,7 @@ out ContentDispositionHeaderValue? contentDisposition ) ) { - Errors.Add( + errors.Add( string.Format( CultureInfo.InvariantCulture, "Part number {0} doesn't have a content disposition", @@ -82,7 +60,7 @@ out ContentDispositionHeaderValue? contentDisposition if (section.ContentType == null) { - Errors.Add( + errors.Add( string.Format( CultureInfo.InvariantCulture, "Part number {0} doesn't have a content type", @@ -107,42 +85,64 @@ out ContentDispositionHeaderValue? contentDisposition // Quotes around filename in Content-Disposition is valid, but not as part of the filename. contentFileName = contentFileName?.Trim('\"').AsFileName(false); - long fileSize = contentDisposition.Size ?? 0; + int fileSize = (int?)contentDisposition.Size ?? 0; - MemoryStream memoryStream = new MemoryStream(); + using MemoryStream memoryStream = new MemoryStream(fileSize); await section.Body.CopyToAsync(memoryStream); memoryStream.Position = 0; - Parts.Add( + parts.Add( new RequestPart() { ContentType = section.ContentType, Name = sectionName, - Stream = memoryStream, + Bytes = memoryStream.ToArray(), FileName = contentFileName, - FileSize = fileSize, } ); } } catch (IOException ioex) { - Errors.Add("IOException while reading a section of the request: " + ioex.Message); + errors.Add("IOException while reading a section of the request: " + ioex.Message); } } else { // create part of content - if (_request.ContentType != null) + if (request.ContentType != null) { - Parts.Add(new RequestPart() { ContentType = _request.ContentType, Stream = _request.Body }); + using var memoryStream = new MemoryStream(); + await request.Body.CopyToAsync(memoryStream); + parts.Add( + new RequestPart + { + ContentType = request.ContentType, + Bytes = memoryStream.ToArray(), + FileName = null, + Name = null, + } + ); } + // Else assume the request body is empty } + + if (errors.Count > 0) + { + return new ProblemDetails() + { + Title = "Error reading request", + Detail = string.Join(", ", errors), + Status = StatusCodes.Status400BadRequest, + }; + } + + return parts; } - private string GetBoundary() + private static string GetBoundary(HttpRequest request) { - MediaTypeHeaderValue mediaType = MediaTypeHeaderValue.Parse(_request.ContentType); + MediaTypeHeaderValue mediaType = MediaTypeHeaderValue.Parse(request.ContentType); return mediaType.Boundary.Value?.Trim('"') ?? throw new Exception("Could not retrieve boundary value from Content-Type header"); } diff --git a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs index 876d580da..ee763d2a3 100644 --- a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs +++ b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPart.cs @@ -8,31 +8,25 @@ public class RequestPart /// /// The stream to access this part. /// -#nullable disable - public Stream Stream { get; set; } - -#nullable restore + public required byte[] Bytes { get; set; } /// /// The file name as given in content description. /// - public string? FileName { get; set; } + public required string? FileName { get; set; } /// /// The parts name. /// - public string? Name { get; set; } + public required string? Name { get; set; } /// /// The content type of the part. /// -#nullable disable - public string ContentType { get; set; } - -#nullable restore + public required string ContentType { get; set; } /// - /// The file size of the part, 0 if not given. + /// The file size of the part /// - public long FileSize { get; set; } + public long FileSize => Bytes.Length; } diff --git a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPartValidator.cs b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPartValidator.cs index ebe525264..f2a35b3ef 100644 --- a/src/Altinn.App.Api/Helpers/RequestHandling/RequestPartValidator.cs +++ b/src/Altinn.App.Api/Helpers/RequestHandling/RequestPartValidator.cs @@ -1,114 +1,115 @@ using Altinn.Platform.Storage.Interface.Models; -namespace Altinn.App.Api.Helpers.RequestHandling; - -/// -/// Represents a validator of a single with the help of app metadata -/// -public class RequestPartValidator +namespace Altinn.App.Api.Helpers.RequestHandling { - private readonly Application _appInfo; - /// - /// Initialises a new instance of the class with the given application info. + /// Represents a validator of a single with the help of app metadata /// - /// The application metadata to use when validating a . - public RequestPartValidator(Application appInfo) + public class RequestPartValidator { - _appInfo = appInfo; - } + private readonly Application _appInfo; - /// - /// Operation that can validate a using the internal . - /// - /// The request part to be validated. - /// null if no errors where found. Otherwise an error message. - public string? ValidatePart(RequestPart part) - { - if (part.Name == "instance") + /// + /// Initialises a new instance of the class with the given application info. + /// + /// The application metadata to use when validating a . + public RequestPartValidator(Application appInfo) { - if (!part.ContentType.StartsWith("application/json", StringComparison.Ordinal)) - { - return $"Unexpected Content-Type '{part.ContentType}' of embedded instance template. Expecting 'application/json'"; - } - - //// TODO: Validate that the element can be read as an instance? + _appInfo = appInfo; } - else + + /// + /// Operation that can validate a using the internal . + /// + /// The request part to be validated. + /// null if no errors where found. Otherwise an error message. + public string? ValidatePart(RequestPart part) { - DataType? dataType = _appInfo.DataTypes.Find(e => e.Id == part.Name); - if (dataType == null) + if (part.Name == "instance") { - return $"Multipart section named, '{part.Name}' does not correspond to an element type in application metadata"; - } + if (!part.ContentType.StartsWith("application/json", StringComparison.Ordinal)) + { + return $"Unexpected Content-Type '{part.ContentType}' of embedded instance template. Expecting 'application/json'"; + } - if (part.ContentType == null) - { - return $"The multipart section named {part.Name} is missing Content-Type."; + //// TODO: Validate that the element can be read as an instance? } else { - string contentTypeWithoutEncoding = part.ContentType.Split(";")[0]; + DataType? dataType = _appInfo.DataTypes.Find(e => e.Id == part.Name); + if (dataType == null) + { + return $"Multipart section named, '{part.Name}' does not correspond to an element type in application metadata"; + } - // restrict content type if allowedContentTypes is specified - if ( - dataType.AllowedContentTypes != null - && dataType.AllowedContentTypes.Count > 0 - && !dataType.AllowedContentTypes.Contains(contentTypeWithoutEncoding) - ) + if (part.ContentType == null) { - return $"The multipart section named {part.Name} has a Content-Type '{part.ContentType}' which is invalid for element type '{dataType.Id}'"; + return $"The multipart section named {part.Name} is missing Content-Type."; } - } + else + { + string contentTypeWithoutEncoding = part.ContentType.Split(";")[0]; - long contentSize = part.FileSize != 0 ? part.FileSize : part.Stream.Length; + // restrict content type if allowedContentTypes is specified + if ( + dataType.AllowedContentTypes != null + && dataType.AllowedContentTypes.Count > 0 + && !dataType.AllowedContentTypes.Contains(contentTypeWithoutEncoding) + ) + { + return $"The multipart section named {part.Name} has a Content-Type '{part.ContentType}' which is invalid for element type '{dataType.Id}'"; + } + } - if (contentSize == 0) - { - return $"The multipart section named {part.Name} has no data. Cannot process empty part."; - } + long contentSize = part.FileSize != 0 ? part.FileSize : part.Bytes.Length; - if ( - dataType.MaxSize.HasValue - && dataType.MaxSize > 0 - && contentSize > (long)dataType.MaxSize.Value * 1024 * 1024 - ) - { - return $"The multipart section named {part.Name} exceeds the size limit of element type '{dataType.Id}'"; + if (contentSize == 0) + { + return $"The multipart section named {part.Name} has no data. Cannot process empty part."; + } + + if ( + dataType.MaxSize.HasValue + && dataType.MaxSize > 0 + && contentSize > (long)dataType.MaxSize.Value * 1024 * 1024 + ) + { + return $"The multipart section named {part.Name} exceeds the size limit of element type '{dataType.Id}'"; + } } - } - return null; - } + return null; + } - /// - /// Operation that can validate a list of elements using the internal . - /// - /// The list of request parts to be validated. - /// null if no errors where found. Otherwise an error message. - public string? ValidateParts(List parts) - { - foreach (RequestPart part in parts) + /// + /// Operation that can validate a list of elements using the internal . + /// + /// The list of request parts to be validated. + /// null if no errors where found. Otherwise an error message. + public string? ValidateParts(List parts) { - string? partError = ValidatePart(part); - if (partError != null) + foreach (RequestPart part in parts) { - return partError; + string? partError = ValidatePart(part); + if (partError != null) + { + return partError; + } } - } - foreach (DataType dataType in _appInfo.DataTypes) - { - if (dataType.MaxCount > 0) + foreach (DataType dataType in _appInfo.DataTypes) { - int partCount = parts.Count(p => p.Name == dataType.Id); - if (dataType.MaxCount < partCount) + if (dataType.MaxCount > 0) { - return $"The list of parts contains more elements of type '{dataType.Id}' than the element type allows."; + int partCount = parts.Count(p => p.Name == dataType.Id); + if (dataType.MaxCount < partCount) + { + return $"The list of parts contains more elements of type '{dataType.Id}' than the element type allows."; + } } } - } - return null; + return null; + } } } diff --git a/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs b/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs index e8d54a079..dcd2c41b9 100644 --- a/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs +++ b/src/Altinn.App.Api/Models/DataPatchResponseMultiple.cs @@ -27,14 +27,14 @@ public class DataPatchResponseMultiple /// [JsonPropertyName("instance")] public required Instance Instance { get; init; } - - /// - /// Pair of Guid and data object. - /// - /// The guid of the DataElement - /// The form data of the data element - public record DataModelPairResponse( - [property: JsonPropertyName("dataElementId")] Guid DataElementId, - [property: JsonPropertyName("data")] object Data - ); } + +/// +/// Pair of Guid and data object. +/// +/// The guid of the DataElement +/// The form data of the data element +public record DataModelPairResponse( + [property: JsonPropertyName("dataElementId")] Guid DataElementId, + [property: JsonPropertyName("data")] object Data +); diff --git a/src/Altinn.App.Api/Models/DataPostResponse.cs b/src/Altinn.App.Api/Models/DataPostResponse.cs new file mode 100644 index 000000000..11a26145e --- /dev/null +++ b/src/Altinn.App.Api/Models/DataPostResponse.cs @@ -0,0 +1,60 @@ +using System.Net; +using System.Text.Json.Serialization; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.AspNetCore.Mvc; + +namespace Altinn.App.Api.Models; + +/// +/// Response object for POST to /org/app/instances/{instanceOwnerPartyId:int}/{instanceGuid:guid}/data/{dataType} +/// +public class DataPostResponse +{ + /// + /// The Id of the created data element + /// + [JsonPropertyName("newDataElementId")] + public required Guid NewDataElementId { get; init; } + + /// + /// The instance with updated data + /// + [JsonPropertyName("instance")] + public required Instance Instance { get; init; } + + /// + /// List of validation issues that reported to have relevant changes after a new data element was added + /// + [JsonPropertyName("validationIssues")] + public required List ValidationIssues { get; init; } + + /// + /// List of updated DataModels caused by dataProcessing + /// + [JsonPropertyName("newDataModels")] + public required List NewDataModels { get; init; } +} + +/// +/// Extension of ProblemDetails to include Validation issues from the file upload. +/// +public class DataPostErrorResponse : ProblemDetails +{ + /// + /// Constructor for simple initialization from upload validation issues. + /// + public DataPostErrorResponse(string detail, List validationIssues) + { + Title = "File validation failed"; + Detail = detail; + Status = (int)HttpStatusCode.BadRequest; + UploadValidationIssues = validationIssues; + } + + /// + /// List of the validators that reported to have relevant changes after a new data element was added + /// + [JsonPropertyName("uploadValidationIssues")] + public List UploadValidationIssues { get; } +} diff --git a/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs b/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs index 59f303f8a..3935e2f4a 100644 --- a/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs +++ b/src/Altinn.App.Core/Extensions/HttpContextExtensions.cs @@ -1,3 +1,5 @@ +using System.Buffers; +using System.IO.Pipelines; using System.Net.Http.Headers; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Primitives; @@ -25,4 +27,70 @@ public static StreamContent CreateContentStream(this HttpRequest request) return content; } + + /// + /// Reads the request body and returns it as a byte array using PipeReader and the content-length header + /// to ensure minimal memory copies + /// + /// Missing utility function in AspNetCore + /// + /// The http request + /// The longest body to read + /// byte array or null if maxLength is exceeded and the actual length + /// + internal static async Task<(byte[]?, long actualLength)> ReadBodyAsByteArrayAsync( + this HttpRequest request, + long? maxLength + ) + { + maxLength ??= int.MaxValue; // about 2 GB (if we need more we should save directly to storage) + // If the request has a too large content length, return null and error + var contentLength = + request.ContentLength ?? throw new InvalidOperationException("Content-Length header is missing"); + if (contentLength > maxLength) + { + return (null, contentLength); + } + + // Preallocate the buffer based on Content-Length + var buffer = new byte[contentLength]; + int bytesWritten = 0; + PipeReader reader = request.BodyReader; + while (bytesWritten < contentLength) + { + var readResult = await reader.ReadAsync(); + var bufferLength = (int)readResult.Buffer.Length; + if (bytesWritten + bufferLength > contentLength) + { + // If the request has a too large content length, return null and error + // Don't bother figuring out how much of the request is left to read + return (null, bytesWritten + bufferLength); + } + readResult.Buffer.CopyTo(buffer.AsSpan(bytesWritten, bufferLength)); + bytesWritten += bufferLength; + reader.AdvanceTo(readResult.Buffer.End); + + // Check if the stream is completed + if (readResult.IsCompleted) + { + break; + } + + // Handle possible cancellation of the read operation + if (readResult.IsCanceled) + { + throw new OperationCanceledException("The request body read was canceled."); + } + } + + // Check if the number of bytes read matches the content length + if (bytesWritten != contentLength) + { + throw new InvalidOperationException( + $"Content length mismatch. Expected {contentLength}, but read {bytesWritten}." + ); + } + + return (buffer, contentLength); + } } diff --git a/src/Altinn.App.Core/Features/IDataWriteProcessor.cs b/src/Altinn.App.Core/Features/IDataWriteProcessor.cs index cca9a8f57..84f296523 100644 --- a/src/Altinn.App.Core/Features/IDataWriteProcessor.cs +++ b/src/Altinn.App.Core/Features/IDataWriteProcessor.cs @@ -1,3 +1,5 @@ +using Altinn.App.Core.Models; + namespace Altinn.App.Core.Features; /// @@ -7,9 +9,10 @@ public interface IDataWriteProcessor { /// /// Is called to run custom calculation events defined by app developer when data is written to app. + /// This method is called for POST requests in addition to PATCH and PUT requests as the old . /// /// - /// Make changes directly to the changes[].CurrentFormData object, or fetch other data elements from the instanceDataMutator. + /// Make changes directly to the . object, or fetch other data elements from the instanceDataMutator. /// /// Object to fetch data elements not included in changes /// The current task ID @@ -18,7 +21,7 @@ public interface IDataWriteProcessor Task ProcessDataWrite( IInstanceDataMutator instanceDataMutator, string taskId, - List changes, + DataElementChanges changes, string? language ); } diff --git a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs index 48b045d85..de9add775 100644 --- a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs @@ -13,6 +13,11 @@ public interface IInstanceDataAccessor /// Instance Instance { get; } + /// + /// The data elements on the instance. + /// + IEnumerable DataElements => Instance.Data; + /// /// Get the actual data represented in the data element. /// diff --git a/src/Altinn.App.Core/Features/IInstanceDataMutator.cs b/src/Altinn.App.Core/Features/IInstanceDataMutator.cs index fed449af8..776fec2a4 100644 --- a/src/Altinn.App.Core/Features/IInstanceDataMutator.cs +++ b/src/Altinn.App.Core/Features/IInstanceDataMutator.cs @@ -1,4 +1,5 @@ using Altinn.App.Core.Models; +using Altinn.Platform.Storage.Interface.Models; namespace Altinn.App.Core.Features; @@ -15,7 +16,29 @@ public interface IInstanceDataMutator : IInstanceDataAccessor /// Serialization of data is done immediately, so the data object should be in a valid state. /// /// Throws an InvalidOperationException if the dataType is not found in applicationmetadata - void AddFormDataElement(string dataType, object model); + FormDataChange AddFormDataElement(string dataTypeId, object model); + + /// + /// Add a new data element with app logic to the instance of this accessor + /// + /// + /// Serialization of data is done immediately, so the data object should be in a valid state. + /// + /// Throws an InvalidOperationException if the dataType is not found in applicationmetadata + FormDataChange AddFormDataElement(DataType dataType, object model) => AddFormDataElement(dataType.Id, model); + + /// + /// Add a new data element without app logic to the instance. + /// + /// + /// Saving to storage is not done until the instance is saved, so mutations to data might or might not be sendt to storage. + /// + BinaryDataChange AddBinaryDataElement( + string dataTypeId, + string contentType, + string? filename, + ReadOnlyMemory bytes + ); /// /// Add a new data element without app logic to the instance. @@ -23,7 +46,12 @@ public interface IInstanceDataMutator : IInstanceDataAccessor /// /// Saving to storage is not done until the instance is saved, so mutations to data might or might not be sendt to storage. /// - void AddAttachmentDataElement(string dataType, string contentType, string? filename, ReadOnlyMemory bytes); + BinaryDataChange AddBinaryDataElement( + DataType dataType, + string contentType, + string? filename, + ReadOnlyMemory bytes + ) => AddBinaryDataElement(dataType.Id, contentType, filename, bytes); /// /// Remove a data element from the instance. diff --git a/src/Altinn.App.Core/Features/IValidator.cs b/src/Altinn.App.Core/Features/IValidator.cs index 12a13eb46..2664822f4 100644 --- a/src/Altinn.App.Core/Features/IValidator.cs +++ b/src/Altinn.App.Core/Features/IValidator.cs @@ -1,3 +1,4 @@ +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; @@ -33,69 +34,26 @@ public interface IValidator /// /// Always returning false from has a similar effect, but setting this to false informs /// frontend that the issues from the validator can't be cached, because FE won't be informed when the issue is fixed. - /// Issues from validators with NoIncrementalValidation will be shown once but prevent process/next from succeding. + /// Issues from validators with NoIncrementalValidation will be shown once but prevent process/next from succeeding. /// bool NoIncrementalValidation => false; /// /// Run this validator and return all the issues this validator is aware of. /// - /// The instance to validate - /// Use this to access the form data from s + /// Use this to access the form data from s /// The current task. /// Language for messages, if the messages are too dynamic for the translation system /// - public Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - string? language - ); + public Task> Validate(IInstanceDataAccessor dataAccessor, string taskId, string? language); /// /// 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 - /// Use this to access data from other data elements + /// Use this to access instance and data from data elements /// The current task ID /// List of changed data elements with current and previous value /// - public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ); -} - -/// -/// Represents a change in a data element with current and previous deserialized data -/// -public sealed 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 PreviousFormData { get; init; } - - /// - /// The state of the data element after the change - /// - public required object CurrentFormData { get; init; } - - /// - /// The binary representation (for storage) of the data element before changes - /// - public ReadOnlyMemory? PreviousBinaryData { get; init; } - - /// - /// The binary representation (for storage) of the data element after changes - /// - public ReadOnlyMemory? CurrentBinaryData { get; init; } + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes); } diff --git a/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs b/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs index 4bf708b7f..b309f800f 100644 --- a/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs +++ b/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs @@ -1,4 +1,5 @@ using System.Diagnostics; +using Altinn.App.Core.Models; using static Altinn.App.Core.Features.Telemetry.Validation; namespace Altinn.App.Core.Features; @@ -19,7 +20,7 @@ private static void InitValidation(InitContext context) return activity; } - internal Activity? StartValidateIncrementalActivity(string taskId, List changes) + internal Activity? StartValidateIncrementalActivity(string taskId, DataElementChanges changes) { ArgumentException.ThrowIfNullOrWhiteSpace(taskId); ArgumentNullException.ThrowIfNull(changes); @@ -30,11 +31,13 @@ private static void InitValidation(InitContext context) var changesPrefix = "ChangedDataElements"; var now = DateTimeOffset.UtcNow; - ActivityTagsCollection tags = new([new($"{changesPrefix}.count", changes.Count)]); - for (var i = 0; i < changes.Count; i++) + var allChanges = changes.AllChanges; + + ActivityTagsCollection tags = new([new($"{changesPrefix}.count", allChanges.Count)]); + for (var i = 0; i < allChanges.Count; i++) { - var change = changes[i]; - tags.Add(new($"{changesPrefix}.{i}.Id", change.DataElement.Id)); + var change = allChanges[i]; + tags.Add(new($"{changesPrefix}.{i}.Id", change.DataElementIdentifier.Id)); } activity?.AddEvent(new ActivityEvent(changesPrefix, now, tags)); return activity; diff --git a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs index 5ee817ef0..51fd07861 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs @@ -59,16 +59,14 @@ IAppMetadata appMetadata /// We don't have an efficient way to figure out if changes to the model results in different validations, and frontend ignores this anyway /// public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, - List changes + DataElementChanges changes ) => Task.FromResult(true); /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) @@ -76,17 +74,10 @@ public async Task> Validate( var validationIssues = new List(); foreach (var (dataType, validationConfig) in GetDataTypesWithExpressionsForTask(taskId)) { - var formDataElementsForTask = instance.Data.Where(d => d.DataType == dataType.Id); + var formDataElementsForTask = dataAccessor.DataElements.Where(d => d.DataType == dataType.Id); foreach (var dataElement in formDataElementsForTask) { - var issues = await ValidateFormData( - instance, - dataElement, - instanceDataAccessor, - validationConfig, - taskId, - language - ); + var issues = await ValidateFormData(dataElement, dataAccessor, validationConfig, taskId, language); validationIssues.AddRange(issues); } } @@ -95,7 +86,6 @@ public async Task> Validate( } internal async Task> ValidateFormData( - Instance instance, DataElement dataElement, IInstanceDataAccessor dataAccessor, string rawValidationConfig, diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index 310612186..1fb78dbf5 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -3,8 +3,8 @@ using Altinn.App.Core.Configuration; using Altinn.App.Core.Features.Validation.Helpers; using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Mvc.ModelBinding; using Microsoft.Extensions.Options; @@ -51,8 +51,7 @@ public string ValidationSource /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) @@ -63,15 +62,15 @@ public async Task> Validate( .DataTypes.Where(d => d.TaskId == taskId && d.AppLogic?.ClassRef != null) .Select(d => d.Id) .ToList(); - foreach (var dataElement in instance.Data.Where(d => dataTypes.Contains(d.DataType))) + foreach (var dataElement in dataAccessor.DataElements.Where(d => dataTypes.Contains(d.DataType))) { - var data = await instanceDataAccessor.GetFormData(dataElement); + var data = await dataAccessor.GetFormData(dataElement); var modelState = new ModelStateDictionary(); await _instanceValidator.ValidateData(data, modelState); issues.AddRange( ModelStateHelpers.ModelStateToIssueList( modelState, - instance, + dataAccessor.Instance, dataElement, _generalSettings, data.GetType() @@ -85,12 +84,7 @@ public async Task> Validate( /// /// 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, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ) + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) { 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 8dc3a8eea..0fe2da4a0 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -2,6 +2,7 @@ using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features.Validation.Helpers; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -50,26 +51,20 @@ public string ValidationSource /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) { var modelState = new ModelStateDictionary(); - await _instanceValidator.ValidateTask(instance, taskId, modelState); - return ModelStateHelpers.MapModelStateToIssueList(modelState, instance, _generalSettings); + await _instanceValidator.ValidateTask(dataAccessor.Instance, taskId, modelState); + return ModelStateHelpers.MapModelStateToIssueList(modelState, dataAccessor.Instance, _generalSettings); } /// /// Don't run the legacy Instance validator for incremental validation (it was not running before) /// - public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ) + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) { throw new NotImplementedException( "Validators with NoIncrementalValidation should not be used for incremental validation" diff --git a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs index 39a4a288c..0aa1140f4 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/RequiredValidator.cs @@ -1,7 +1,7 @@ using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.Expressions; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; namespace Altinn.App.Core.Features.Validation.Default; @@ -43,14 +43,13 @@ public bool ShouldRunForTask(string taskId) => /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) { var evaluationState = await _layoutEvaluatorStateInitializer.Init( - instanceDataAccessor, + dataAccessor, taskId, gatewayAction: null, language @@ -63,9 +62,8 @@ public async Task> Validate( /// We don't have an efficient way to figure out if changes to the model results in different validations, and frontend ignores this anyway /// public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, - List changes + DataElementChanges changes ) => Task.FromResult(true); } diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs index 298689316..4bfcf7cc8 100644 --- a/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs @@ -1,3 +1,4 @@ +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; @@ -39,15 +40,14 @@ List dataTypes /// Run all legacy instances for the given . /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) { var issues = new List(); var validateAllElements = _dataElementValidator.DataType == "*"; - foreach (var dataElement in instance.Data) + foreach (var dataElement in dataAccessor.DataElements) { if (validateAllElements || _dataElementValidator.DataType == dataElement.DataType) { @@ -59,7 +59,7 @@ public async Task> Validate( ); } var dataElementValidationResult = await _dataElementValidator.ValidateDataElement( - instance, + dataAccessor.Instance, dataElement, dataType, language @@ -75,12 +75,7 @@ public async Task> Validate( } /// - public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ) + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) { // DataElementValidator did not previously implement incremental validation, so we always return false throw new NotImplementedException( diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs index e95ba0073..c92b68a46 100644 --- a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs @@ -1,3 +1,5 @@ +using Altinn.App.Core.Models; + namespace Altinn.App.Core.Features.Validation.Wrappers; using Altinn.App.Core.Models.Validation; @@ -27,17 +29,16 @@ public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string tas /// Run all legacy instances for the given . /// public async Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, + IInstanceDataAccessor dataAccessor, string taskId, string? language ) { var issues = new List(); var validateAllElements = _formDataValidator.DataType == "*"; - foreach (var dataElement in instance.Data) + foreach (var dataElement in dataAccessor.DataElements) { - var dataType = instanceDataAccessor.GetDataType(dataElement); + var dataType = dataAccessor.GetDataType(dataElement); if (dataType.AppLogic?.ClassRef == null) { continue; @@ -47,9 +48,9 @@ public async Task> Validate( continue; } - var data = await instanceDataAccessor.GetFormData(dataElement); + var data = await dataAccessor.GetFormData(dataElement); var dataElementValidationResult = await _formDataValidator.ValidateFormData( - instance, + dataAccessor.Instance, dataElement, data, language @@ -63,23 +64,24 @@ public async Task> Validate( } /// - public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ) + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) { try { - foreach (var change in changes) + foreach (var change in changes.FormDataChanges) { - if ( - (_formDataValidator.DataType == "*" || _formDataValidator.DataType == change.DataElement.DataType) - && _formDataValidator.HasRelevantChanges(change.CurrentFormData, change.PreviousFormData) - ) + // Check if the DataType is a wildcard or matches the change's DataType, and if the change is not an update or has relevant changes + if (_formDataValidator.DataType == "*" || _formDataValidator.DataType == change.DataType.Id) { - return Task.FromResult(true); + if (change.Type != ChangeType.Updated) + { + return Task.FromResult(true); + } + + if (_formDataValidator.HasRelevantChanges(change.CurrentFormData, change.PreviousFormData)) + { + return Task.FromResult(true); + } } } diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs index 0a7f6690d..7d2a6539a 100644 --- a/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/TaskValidatorWrapper.cs @@ -1,5 +1,5 @@ +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; namespace Altinn.App.Core.Features.Validation.Wrappers; @@ -31,23 +31,13 @@ public TaskValidatorWrapper(ITaskValidator taskValidator) public bool NoIncrementalValidation => true; /// - public Task> Validate( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - string? language - ) + public Task> Validate(IInstanceDataAccessor dataAccessor, string taskId, string? language) { - return _taskValidator.ValidateTask(instance, taskId, language); + return _taskValidator.ValidateTask(dataAccessor.Instance, taskId, language); } /// - public Task HasRelevantChanges( - Instance instance, - IInstanceDataAccessor instanceDataAccessor, - string taskId, - List changes - ) + public Task HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes) { throw new NotImplementedException( "TaskValidatorWrapper should not be used for incremental validation, because it sets NoIncrementalValidation to true" diff --git a/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs b/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs index 96dbc45e5..8eec159bd 100644 --- a/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs +++ b/src/Altinn.App.Core/Helpers/DataModel/DataModel.cs @@ -20,7 +20,7 @@ public class DataModel public DataModel(IInstanceDataAccessor dataAccessor, ApplicationMetadata appMetadata) { _dataAccessor = dataAccessor; - foreach (var dataElement in dataAccessor.Instance.Data) + foreach (var dataElement in dataAccessor.DataElements) { var dataTypeId = dataElement.DataType; var dataType = appMetadata.DataTypes.Find(d => d.Id == dataTypeId); diff --git a/src/Altinn.App.Core/Helpers/Serialization/ModelSerializationService.cs b/src/Altinn.App.Core/Helpers/Serialization/ModelSerializationService.cs index 6e39a6417..1bc10a767 100644 --- a/src/Altinn.App.Core/Helpers/Serialization/ModelSerializationService.cs +++ b/src/Altinn.App.Core/Helpers/Serialization/ModelSerializationService.cs @@ -127,11 +127,35 @@ DataType dataType object model; if (contentType?.Contains("application/xml") ?? true) // default to xml if no content type is provided { - model = DeserializeXml(segment, modelType); + try + { + model = DeserializeXml(segment, modelType); + } + catch (XmlException e) + { + return new ProblemDetails() + { + Title = "Failed to deserialize XML", + Detail = e.Message, + Status = StatusCodes.Status400BadRequest, + }; + } } else if (contentType.Contains("application/json")) { - model = DeserializeJson(segment, modelType); + try + { + model = DeserializeJson(segment, modelType); + } + catch (JsonException e) + { + return new ProblemDetails() + { + Title = "Failed to deserialize JSON", + Detail = e.Message, + Status = StatusCodes.Status400BadRequest, + }; + } } else { @@ -262,4 +286,13 @@ private static string GetRootElementName(Type modelType) return modelType.Name; } } + + /// + /// Initialize an empty object of the specified type + /// + public object GetEmpty(DataType dataType) + { + ArgumentException.ThrowIfNullOrWhiteSpace(dataType?.AppLogic?.ClassRef); + return _appModel.Create(dataType.AppLogic.ClassRef); + } } diff --git a/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs b/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs index 8087202d9..216eb7e38 100644 --- a/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs +++ b/src/Altinn.App.Core/Infrastructure/Clients/Storage/DataClient.cs @@ -419,7 +419,6 @@ public async Task InsertBinaryData( apiUrl += $"&generatedFromTask={generatedFromTask}"; } string token = _userTokenProvider.GetUserToken(); - DataElement dataElement; StreamContent content = new StreamContent(stream); content.Headers.ContentType = MediaTypeHeaderValue.Parse(contentType); @@ -436,11 +435,11 @@ public async Task InsertBinaryData( if (response.IsSuccessStatusCode) { - string instancedata = await response.Content.ReadAsStringAsync(); - // ! TODO: this null-forgiving operator should be fixed/removed for the next major release - dataElement = JsonConvert.DeserializeObject(instancedata)!; + string dataElementString = await response.Content.ReadAsStringAsync(); - return dataElement; + var dataElement = JsonConvert.DeserializeObject(dataElementString); + if (dataElement is not null) + return dataElement; } _logger.LogError( diff --git a/src/Altinn.App.Core/Internal/Data/CachedInstanceDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CachedInstanceDataAccessor.cs deleted file mode 100644 index 33eb8ca20..000000000 --- a/src/Altinn.App.Core/Internal/Data/CachedInstanceDataAccessor.cs +++ /dev/null @@ -1,463 +0,0 @@ -using System.Collections.Concurrent; -using System.Globalization; -using Altinn.App.Core.Features; -using Altinn.App.Core.Helpers; -using Altinn.App.Core.Helpers.Serialization; -using Altinn.App.Core.Internal.App; -using Altinn.App.Core.Internal.Instances; -using Altinn.App.Core.Models; -using Altinn.Platform.Storage.Interface.Models; - -namespace Altinn.App.Core.Internal.Data; - -/// -/// Class that caches form data to avoid multiple calls to the data service for a single validation -/// -/// Do not add this to the DI container, as it should only be created explicitly because of data leak potential. -/// -internal sealed class CachedInstanceDataAccessor : IInstanceDataMutator -{ - // DataClient needs a few arguments to fetch data - private readonly Guid _instanceGuid; - private readonly int _instanceOwnerPartyId; - - // Services from DI - private readonly IDataClient _dataClient; - private readonly IInstanceClient _instanceClient; - private readonly IAppMetadata _appMetadata; - private readonly ModelSerializationService _modelSerializationService; - - // Caches - // Cache for the most up to date form data (can be mutated or replaced with SetFormData(dataElementId, data)) - private readonly LazyCache _formDataCache = new(); - - // Cache for the binary content of the file as currently in storage (updated on save) - private readonly LazyCache> _binaryCache = new(); - - // Data elements to delete (eg RemoveDataElement(dataElementId)), but not yet deleted from instance or storage - private readonly ConcurrentBag _dataElementsToDelete = new(); - - // Data elements to add (eg AddFormDataElement(dataTypeString, data)), but not yet added to instance or storage - private readonly ConcurrentBag<( - DataType dataType, - string contentType, - string? filename, - object? model, - ReadOnlyMemory bytes - )> _dataElementsToAdd = new(); - - // The update functions returns updated data elements. - // We want to make sure that the data elements are updated in the instance object - private readonly ConcurrentBag _savedDataElements = new(); - - public CachedInstanceDataAccessor( - Instance instance, - IDataClient dataClient, - IInstanceClient instanceClient, - IAppMetadata appMetadata, - ModelSerializationService modelSerializationService - ) - { - if (instance.Id is not null) - { - var splitId = instance.Id.Split("/"); - _instanceOwnerPartyId = int.Parse(splitId[0], CultureInfo.InvariantCulture); - _instanceGuid = Guid.Parse(splitId[1]); - } - - Instance = instance; - _dataClient = dataClient; - _appMetadata = appMetadata; - _modelSerializationService = modelSerializationService; - _instanceClient = instanceClient; - } - - public Instance Instance { get; } - - /// - public async Task GetFormData(DataElementIdentifier dataElementIdentifier) - { - return await _formDataCache.GetOrCreate( - dataElementIdentifier, - async () => - { - var binaryData = await GetBinaryData(dataElementIdentifier); - - return _modelSerializationService.DeserializeFromStorage( - binaryData.Span, - GetDataType(dataElementIdentifier) - ); - } - ); - } - - /// - public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) - { - if (_instanceOwnerPartyId == 0 || _instanceGuid == Guid.Empty) - { - throw new InvalidOperationException("Cannot access instance data before it has been created"); - } - - var appMetadata = await _appMetadata.GetApplicationMetadata(); - - return await _binaryCache.GetOrCreate( - dataElementIdentifier, - async () => - await _dataClient.GetDataBytes( - appMetadata.AppIdentifier.Org, - appMetadata.AppIdentifier.App, - _instanceOwnerPartyId, - _instanceGuid, - dataElementIdentifier.Guid - ) - ); - } - - /// - public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) - { - return Instance.Data.Find(d => d.Id == dataElementIdentifier.Id) - ?? throw new InvalidOperationException( - $"Data element of id {dataElementIdentifier.Id} not found on instance" - ); - } - - /// - public DataType GetDataType(DataElementIdentifier dataElementIdentifier) - { - var dataElement = GetDataElement(dataElementIdentifier); - var appMetadata = _appMetadata.GetApplicationMetadata().Result; - var dataType = appMetadata.DataTypes.Find(d => d.Id == dataElement.DataType); - if (dataType is null) - { - throw new InvalidOperationException( - $"Data type {dataElement.DataType} not found in applicationmetadata.json" - ); - } - - return dataType; - } - - /// - public void AddFormDataElement(string dataTypeString, object model) - { - var dataType = GetDataTypeByString(dataTypeString); - if (dataType.AppLogic?.ClassRef is not { } classRef) - { - throw new InvalidOperationException( - $"Data type {dataTypeString} does not have a class reference in app metadata" - ); - } - - var modelType = model.GetType(); - if (modelType.FullName != classRef) - { - throw new InvalidOperationException( - $"Data object registered for {dataTypeString} is not of type {classRef} as specified in application metadata" - ); - } - - ObjectUtils.InitializeAltinnRowId(model); - var (bytes, contentType) = _modelSerializationService.SerializeToStorage(model, dataType); - - _dataElementsToAdd.Add((dataType, contentType, null, model, bytes)); - } - - /// - public void AddAttachmentDataElement( - string dataTypeString, - string contentType, - string? filename, - ReadOnlyMemory bytes - ) - { - var dataType = GetDataTypeByString(dataTypeString); - if (dataType.AppLogic?.ClassRef is not null) - { - throw new InvalidOperationException( - $"Data type {dataTypeString} has a AppLogic.ClassRef in app metadata, and is not a binary data element" - ); - } - _dataElementsToAdd.Add((dataType, contentType, filename, null, bytes)); - } - - /// - public void RemoveDataElement(DataElementIdentifier dataElementIdentifier) - { - var dataElement = Instance.Data.Find(d => d.Id == dataElementIdentifier.Id); - if (dataElement is null) - { - throw new InvalidOperationException( - $"Data element with id {dataElementIdentifier.Id} not found in instance" - ); - } - - _dataElementsToDelete.Add(dataElementIdentifier); - } - - public List GetDataElementChanges(bool initializeAltinnRowId) - { - var changes = new List(); - foreach (var dataElement in Instance.Data) - { - DataElementIdentifier dataElementIdentifier = dataElement; - object? data = _formDataCache.GetCachedValueOrDefault(dataElementIdentifier); - // Skip data elements that have not been deserialized into a object - if (data is null) - continue; - var dataType = GetDataType(dataElementIdentifier); - var previousBinary = _binaryCache.GetCachedValueOrDefault(dataElementIdentifier); - if (initializeAltinnRowId) - { - ObjectUtils.InitializeAltinnRowId(data); - } - var (currentBinary, _) = _modelSerializationService.SerializeToStorage(data, dataType); - _binaryCache.Set(dataElementIdentifier, currentBinary); - - if (!currentBinary.Span.SequenceEqual(previousBinary.Span)) - { - changes.Add( - new DataElementChange() - { - DataElement = dataElement, - CurrentFormData = data, - PreviousFormData = _modelSerializationService.DeserializeFromStorage( - previousBinary.Span, - dataType - ), - CurrentBinaryData = currentBinary, - PreviousBinaryData = previousBinary, - } - ); - } - } - - return changes; - } - - internal async Task UpdateInstanceData(List changes) - { - if (_instanceOwnerPartyId == 0 || _instanceGuid == Guid.Empty) - { - throw new InvalidOperationException("Cannot access instance data before it has been created"); - } - - var tasks = new List(); - ConcurrentBag createdDataElements = new(); - // We need to create data elements here, so that we can set them correctly on the instance - // Updating and deleting is done in SaveChanges and happen in parallel with validation. - - // Upload added data elements - foreach (var (dataType, contentType, filename, data, bytes) in _dataElementsToAdd) - { - async Task InsertBinaryData() - { - var dataElement = await _dataClient.InsertBinaryData( - Instance.Id, - dataType.Id, - contentType, - filename, - new MemoryAsStream(bytes) - ); - _binaryCache.Set(dataElement, bytes); - if (data is not null) - { - _formDataCache.Set(dataElement, data); - } - createdDataElements.Add(dataElement); - } - - tasks.Add(InsertBinaryData()); - } - - var appMetadata = await _appMetadata.GetApplicationMetadata(); - - // Delete data elements - foreach (var dataElementId in _dataElementsToDelete) - { - async Task DeleteData() - { - await _dataClient.DeleteData( - appMetadata.AppIdentifier.Org, - appMetadata.AppIdentifier.App, - _instanceOwnerPartyId, - _instanceGuid, - dataElementId.Guid, - true - ); - } - - tasks.Add(DeleteData()); - } - - await Task.WhenAll(tasks); - - //Update DataValues and presentation texts - foreach (var change in changes) - { - await UpdateDataValuesOnInstance(Instance, change.DataElement.DataType, change.CurrentFormData); - await UpdatePresentationTextsOnInstance(Instance, change.DataElement.DataType, change.CurrentFormData); - } - - // Remove deleted data elements from instance.Data - Instance.Data.RemoveAll(dataElement => _dataElementsToDelete.Any(d => d.Id == dataElement.Id)); - - // Add Created data elements to instance - Instance.Data.AddRange(createdDataElements); - } - - internal async Task SaveChanges(List changes) - { - var tasks = new List(); - - foreach (var change in changes) - { - if (change.CurrentBinaryData is null) - { - throw new InvalidOperationException("Changes sent to SaveChanges must have a CurrentBinaryData value"); - } - - async Task UpdateDataElement() - { - var newDataElement = await _dataClient.UpdateBinaryData( - new InstanceIdentifier(Instance), - change.DataElement.ContentType, - change.DataElement.Filename, - Guid.Parse(change.DataElement.Id), - new MemoryAsStream(change.CurrentBinaryData.Value) - ); - _savedDataElements.Add(newDataElement); - } - - tasks.Add(UpdateDataElement()); - } - - await Task.WhenAll(tasks); - } - - /// - /// Add or replace existing data element data in the cache - /// - internal void SetFormData(DataElementIdentifier dataElementIdentifier, object data) - { - var dataType = GetDataType(dataElementIdentifier); - if (dataType.AppLogic?.ClassRef is not { } classRef) - { - throw new InvalidOperationException($"Data element {dataElementIdentifier.Id} don't have app logic"); - } - if (data.GetType().FullName != classRef) - { - throw new InvalidOperationException( - $"Data object registered for {dataElementIdentifier.Id} is not of type {classRef} as specified in application metadata for data type {dataType.Id}, but {data.GetType().FullName}" - ); - } - _formDataCache.Set(dataElementIdentifier, data); - } - - /// - /// Simple wrapper around a Dictionary using Lazy to ensure that the valueFactory is only called once - /// - private sealed class LazyCache - { - private readonly Dictionary>> _cache = new(); - - public async Task GetOrCreate(DataElementIdentifier key, Func> valueFactory) - { - Lazy>? lazyTask; - lock (_cache) - { - if (!_cache.TryGetValue(key.Guid, out lazyTask)) - { - lazyTask = new Lazy>(valueFactory); - _cache.Add(key.Guid, lazyTask); - } - } - return await lazyTask.Value; - } - - public void Set(DataElementIdentifier key, T data) - { - lock (_cache) - { - _cache[key.Guid] = new Lazy>(Task.FromResult(data)); - } - } - - public T? GetCachedValueOrDefault(DataElementIdentifier identifier) - { - lock (_cache) - { - if ( - _cache.TryGetValue(identifier.Guid, out var lazyTask) - && lazyTask is { IsValueCreated: true, Value.IsCompletedSuccessfully: true } - ) - { - return lazyTask.Value.Result; - } - } - return default; - } - } - - private DataType GetDataTypeByString(string dataTypeString) - { - var appMetadata = _appMetadata.GetApplicationMetadata().Result; - var dataType = appMetadata.DataTypes.Find(d => d.Id == dataTypeString); - if (dataType is null) - { - throw new InvalidOperationException($"Data type {dataTypeString} not found in app metadata"); - } - - return dataType; - } - - internal void VerifyDataElementsUnchanged() - { - var changes = GetDataElementChanges(initializeAltinnRowId: false); - if (changes.Count > 0) - { - throw new InvalidOperationException( - $"Data elements of type {string.Join(", ", changes.Select(c => c.DataElement.DataType).Distinct())} have been changed by validators" - ); - } - } - - private async Task UpdatePresentationTextsOnInstance(Instance instance, string dataType, object serviceModel) - { - var updatedValues = DataHelper.GetUpdatedDataValues( - (await _appMetadata.GetApplicationMetadata()).PresentationFields, - instance.PresentationTexts, - dataType, - serviceModel - ); - - if (updatedValues.Count > 0) - { - await _instanceClient.UpdatePresentationTexts( - int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), - Guid.Parse(instance.Id.Split("/")[1]), - new PresentationTexts { Texts = updatedValues } - ); - } - } - - private async Task UpdateDataValuesOnInstance(Instance instance, string dataType, object serviceModel) - { - var updatedValues = DataHelper.GetUpdatedDataValues( - (await _appMetadata.GetApplicationMetadata()).DataFields, - instance.DataValues, - dataType, - serviceModel - ); - - if (updatedValues.Count > 0) - { - await _instanceClient.UpdateDataValues( - int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), - Guid.Parse(instance.Id.Split("/")[1]), - new DataValues { Values = updatedValues } - ); - } - } -} diff --git a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs new file mode 100644 index 000000000..0504696b5 --- /dev/null +++ b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs @@ -0,0 +1,630 @@ +using System.Collections.Concurrent; +using System.Diagnostics.CodeAnalysis; +using System.Globalization; +using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Helpers.Serialization; +using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Models; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Internal.Data; + +/// +/// Class that caches form data to avoid multiple calls to the data service for a single validation +/// +/// Do not add this to the DI container, as it should only be created explicitly because of data leak potential. +/// +internal sealed class InstanceDataUnitOfWork : IInstanceDataMutator +{ + // DataClient needs a few arguments to fetch data + private readonly Guid _instanceGuid; + private readonly int _instanceOwnerPartyId; + + // Services from DI + private readonly IDataClient _dataClient; + private readonly IInstanceClient _instanceClient; + private readonly ApplicationMetadata _appMetadata; + private readonly ModelSerializationService _modelSerializationService; + + // Caches + // Cache for the most up to date form data (can be mutated or replaced with SetFormData(dataElementId, data)) + private readonly LazyCache _formDataCache = new(); + + // Cache for the binary content of the file as currently in storage (updated on save) + private readonly LazyCache> _binaryCache = new(); + + // Data elements to delete (eg RemoveDataElement(dataElementId)), but not yet deleted from instance or storage + private readonly ConcurrentBag _changesForDeletion = []; + + // Form data not yet saved to storage (thus no dataElementId) + private readonly ConcurrentBag _changesForCreation = []; + + // The update functions returns updated data elements. + // We want to make sure that the data elements are updated in the instance object + private readonly ConcurrentBag _savedDataElements = []; + + public InstanceDataUnitOfWork( + Instance instance, + IDataClient dataClient, + IInstanceClient instanceClient, + ApplicationMetadata appMetadata, + ModelSerializationService modelSerializationService + ) + { + if (instance.Id is not null) + { + var splitId = instance.Id.Split("/"); + _instanceOwnerPartyId = int.Parse(splitId[0], CultureInfo.InvariantCulture); + _instanceGuid = Guid.Parse(splitId[1]); + } + + Instance = instance; + _dataClient = dataClient; + _appMetadata = appMetadata; + _modelSerializationService = modelSerializationService; + _instanceClient = instanceClient; + } + + public Instance Instance { get; } + + /// + public async Task GetFormData(DataElementIdentifier dataElementIdentifier) + { + return await _formDataCache.GetOrCreate( + dataElementIdentifier, + async () => + { + var binaryData = await GetBinaryData(dataElementIdentifier); + + return _modelSerializationService.DeserializeFromStorage( + binaryData.Span, + GetDataType(dataElementIdentifier) + ); + } + ); + } + + /// + public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) + { + if (_instanceOwnerPartyId == 0 || _instanceGuid == Guid.Empty) + { + throw new InvalidOperationException("Cannot access instance data before it has been created"); + } + + return await _binaryCache.GetOrCreate( + dataElementIdentifier, + async () => + await _dataClient.GetDataBytes( + _appMetadata.AppIdentifier.Org, + _appMetadata.AppIdentifier.App, + _instanceOwnerPartyId, + _instanceGuid, + dataElementIdentifier.Guid + ) + ); + } + + /// + public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) + { + return Instance.Data.Find(d => d.Id == dataElementIdentifier.Id) + ?? throw new InvalidOperationException( + $"Data element of id {dataElementIdentifier.Id} not found on instance" + ); + } + + /// + public DataType GetDataType(DataElementIdentifier dataElementIdentifier) + { + var dataElement = GetDataElement(dataElementIdentifier); + var dataType = _appMetadata.DataTypes.Find(d => d.Id == dataElement.DataType); + if (dataType is null) + { + throw new InvalidOperationException( + $"Data type {dataElement.DataType} not found in applicationmetadata.json" + ); + } + + return dataType; + } + + /// + public FormDataChange AddFormDataElement(string dataTypeId, object model) + { + ArgumentNullException.ThrowIfNull(model); + var dataType = GetDataTypeByString(dataTypeId); + if (dataType.AppLogic?.ClassRef is not { } classRef) + { + throw new InvalidOperationException( + $"Data type {dataTypeId} does not have a class reference in app metadata" + ); + } + + var modelType = model.GetType(); + if (modelType.FullName != classRef) + { + throw new InvalidOperationException( + $"Data object registered for {dataTypeId} is not of type {classRef} as specified in application metadata" + ); + } + + ObjectUtils.InitializeAltinnRowId(model); + var (bytes, contentType) = _modelSerializationService.SerializeToStorage(model, dataType); + + FormDataChange change = new FormDataChange + { + Type = ChangeType.Created, + DataElement = null, + DataType = dataType, + ContentType = contentType, + CurrentFormData = model, + PreviousFormData = _modelSerializationService.GetEmpty(dataType), + CurrentBinaryData = bytes, + PreviousBinaryData = default, // empty memory reference + }; + _changesForCreation.Add(change); + return change; + } + + /// + public BinaryDataChange AddBinaryDataElement( + string dataTypeId, + string contentType, + string? filename, + ReadOnlyMemory bytes + ) + { + var dataType = GetDataTypeByString(dataTypeId); + if (dataType.AppLogic?.ClassRef is not null) + { + throw new InvalidOperationException( + $"Data type {dataTypeId} has a AppLogic.ClassRef in app metadata, and is not a binary data element" + ); + } + + if (dataType.MaxSize.HasValue && bytes.Length > dataType.MaxSize.Value * 1024 * 1024) + { + throw new InvalidOperationException( + $"Data element of type {dataTypeId} exceeds the size limit of {dataType.MaxSize} MB" + ); + } + + if (dataType.AllowedContentTypes is { Count: > 0 } && !dataType.AllowedContentTypes.Contains(contentType)) + { + throw new InvalidOperationException( + $"Data element of type {dataTypeId} has a Content-Type '{contentType}' which is invalid for element type '{dataTypeId}'" + ); + } + + BinaryDataChange change = new BinaryDataChange + { + Type = ChangeType.Created, + DataElement = null, // Not yet saved to storage + DataType = dataType, + FileName = filename, + ContentType = contentType, + CurrentBinaryData = bytes, + }; + _changesForCreation.Add(change); + return change; + } + + /// + public void RemoveDataElement(DataElementIdentifier dataElementIdentifier) + { + var dataElement = Instance.Data.Find(d => d.Id == dataElementIdentifier.Id); + if (dataElement is null) + { + throw new InvalidOperationException( + $"Data element with id {dataElementIdentifier.Id} not found in instance" + ); + } + var dataType = GetDataType(dataElementIdentifier); + if (dataType.AppLogic?.ClassRef is null) + { + _changesForDeletion.Add( + new BinaryDataChange() + { + Type = ChangeType.Deleted, + DataElement = dataElement, + DataType = dataType, + FileName = dataElement.Filename, + ContentType = dataElement.ContentType, + CurrentBinaryData = default, + } + ); + } + else + { + _changesForDeletion.Add( + new FormDataChange() + { + Type = ChangeType.Deleted, + DataElement = dataElement, + DataType = dataType, + ContentType = dataElement.ContentType, + CurrentFormData = _formDataCache.TryGetCachedValue(dataElementIdentifier, out var cfd) + ? cfd + : _modelSerializationService.GetEmpty(dataType), + PreviousFormData = _modelSerializationService.GetEmpty(dataType), + CurrentBinaryData = null, + PreviousBinaryData = _binaryCache.TryGetCachedValue(dataElementIdentifier, out var value) + ? value + : null, + } + ); + } + } + + public DataElementChanges GetDataElementChanges(bool initializeAltinnRowId) + { + var changes = new List(); + + // Add form data where the CurrentFormData serializes to a different binary than the PreviousBinaryData + foreach (var dataElement in Instance.Data) + { + DataElementIdentifier dataElementIdentifier = dataElement; + if (_changesForDeletion.Any(change => change.DataElementIdentifier == dataElementIdentifier)) + { + // Deleted (and created) changes gets added bellow + continue; + } + var dataType = GetDataType(dataElementIdentifier); + + if (!_formDataCache.TryGetCachedValue(dataElementIdentifier, out object? data)) + { + // We don't support making updates to binary data elements (attachments) in IInstanceDataMutator + continue; + } + + // The object has form data + if (dataType.AppLogic?.ClassRef is null) + throw new InvalidOperationException( + $"Data element {dataElementIdentifier.Id} of type {dataType.Id} has cached form data, but no app logic" + ); + var hasCachedBinary = _binaryCache.TryGetCachedValue( + dataElementIdentifier, + out ReadOnlyMemory cachedBinary + ); + if (!hasCachedBinary) + { + throw new InvalidOperationException( + $"Data element {dataElementIdentifier.Id} of type {dataType.Id} has app logic and must be fetched before it is edited" + ); + } + + if (initializeAltinnRowId) + { + ObjectUtils.InitializeAltinnRowId(data); + } + + var (currentBinary, _) = _modelSerializationService.SerializeToStorage(data, dataType); + + if (!currentBinary.Span.SequenceEqual(cachedBinary.Span)) + { + changes.Add( + new FormDataChange() + { + Type = ChangeType.Updated, + DataElement = dataElement, + ContentType = dataElement.ContentType, + DataType = dataType, + CurrentFormData = data, + // For patch requests we could get the previous data from the patch, but it's not available here + // and deserializing twice is not a big deal + PreviousFormData = _modelSerializationService.DeserializeFromStorage( + cachedBinary.Span, + dataType + ), + CurrentBinaryData = currentBinary, + PreviousBinaryData = cachedBinary, + } + ); + } + } + + // Include the change for data elements that have been added + changes.AddRange(_changesForCreation); + changes.AddRange(_changesForDeletion); + + return new DataElementChanges(changes); + } + + private async Task CreateDataElement( + ConcurrentDictionary createdDataElements, + DataElementChange change + ) + { + var bytes = change switch + { + BinaryDataChange bc => bc.CurrentBinaryData, + FormDataChange fdc + => fdc.CurrentBinaryData + ?? throw new InvalidOperationException("FormDataChange must set CurrentBinaryData before saving"), + _ => throw new InvalidOperationException("Change must be of type BinaryChange or FormDataChange") + }; + // Use the BinaryData because we serialize before saving. + var dataElement = await _dataClient.InsertBinaryData( + Instance.Id, + change.DataType.Id, + change.ContentType, + (change as BinaryDataChange)?.FileName, + new MemoryAsStream(bytes) + ); + _binaryCache.Set(dataElement, bytes); + if (change is FormDataChange formDataChange) + { + _formDataCache.Set(dataElement, formDataChange.CurrentFormData); + } + createdDataElements.TryAdd(change, dataElement); + } + + private async Task UpdateDataElement( + DataElementIdentifier dataElementIdentifier, + string contentType, + string? filename, + ReadOnlyMemory bytes + ) + { + var newDataElement = await _dataClient.UpdateBinaryData( + new InstanceIdentifier(Instance), + contentType, + filename, + dataElementIdentifier.Guid, + new MemoryAsStream(bytes) + ); + _savedDataElements.Add(newDataElement); + } + + internal async Task UpdateInstanceData(DataElementChanges changes) + { + if (_instanceOwnerPartyId == 0 || _instanceGuid == Guid.Empty) + { + throw new InvalidOperationException("Cannot access instance data before it has been created"); + } + + var tasks = new List(); + ConcurrentDictionary createdDataElements = []; + // We need to create data elements here, so that we can set them correctly on the instance + // Updating and deleting is done in SaveChanges and happen in parallel with validation. + + // Upload added data elements + foreach (var change in _changesForCreation) + { + tasks.Add(CreateDataElement(createdDataElements, change)); + } + + // Delete data elements + foreach (var change in _changesForDeletion) + { + async Task DeleteData() + { + await _dataClient.DeleteData( + _appMetadata.AppIdentifier.Org, + _appMetadata.AppIdentifier.App, + _instanceOwnerPartyId, + _instanceGuid, + change.DataElementIdentifier.Guid, + false + ); + } + + tasks.Add(DeleteData()); + } + + await Task.WhenAll(tasks); + + // Remove deleted data elements from instance.Data + Instance.Data.RemoveAll(dataElement => _changesForDeletion.Any(d => d.DataElement?.Id == dataElement.Id)); + + // Add Created data elements to instance + Instance.Data.AddRange(createdDataElements.Values); + + // update data elements on new elements + foreach (var change in changes.AllChanges) + { + if (change.DataElement is null) + { + change.DataElement = createdDataElements.TryGetValue(change, out var value) + ? value + : throw new InvalidOperationException( + "DataElementChange without DataElement must be a new data element" + ); + } + if (change is FormDataChange formDataChange) + { + //Update DataValues and presentation texts + // These can not run in parallel with creating the data elements, because they need the data element id + await UpdateDataValuesOnInstance(Instance, formDataChange.DataType.Id, formDataChange.CurrentFormData); + await UpdatePresentationTextsOnInstance( + Instance, + formDataChange.DataType.Id, + formDataChange.CurrentFormData + ); + } + } + } + + internal async Task SaveChanges(DataElementChanges changes) + { + var tasks = new List(); + + foreach (var change in changes.FormDataChanges) + { + if (change.Type != ChangeType.Updated) + continue; + if (change.CurrentBinaryData is null) + { + throw new InvalidOperationException( + "ChangeType.Updated sent to SaveChanges must have a CurrentBinaryData value" + ); + } + if (change.DataElement is null) + throw new InvalidOperationException( + "ChangeType.Updated sent to SaveChanges must have a DataElement value" + ); + + tasks.Add( + UpdateDataElement( + change.DataElement, + change.DataElement.ContentType, + change.DataElement.Filename, + change.CurrentBinaryData.Value + ) + ); + } + + await Task.WhenAll(tasks); + } + + /// + /// Add or replace existing data element data in the cache + /// + internal void SetFormData(DataElementIdentifier dataElementIdentifier, object data) + { + var dataType = GetDataType(dataElementIdentifier); + if (dataType.AppLogic?.ClassRef is not { } classRef) + { + throw new InvalidOperationException($"Data element {dataElementIdentifier.Id} don't have app logic"); + } + if (data.GetType().FullName != classRef) + { + throw new InvalidOperationException( + $"Data object registered for {dataElementIdentifier.Id} is not of type {classRef} as specified in application metadata for data type {dataType.Id}, but {data.GetType().FullName}" + ); + } + _formDataCache.Set(dataElementIdentifier, data); + } + + /// + /// Simple wrapper around a Dictionary using Lazy to ensure that the valueFactory is only called once + /// + private sealed class LazyCache + { + private readonly Dictionary>> _cache = []; + + public async Task GetOrCreate(DataElementIdentifier key, Func> valueFactory) + { + Lazy>? lazyTask; + lock (_cache) + { + if (!_cache.TryGetValue(key.Guid, out lazyTask)) + { + lazyTask = new Lazy>(valueFactory); + _cache.Add(key.Guid, lazyTask); + } + } + return await lazyTask.Value; + } + + public void Set(DataElementIdentifier key, T data) + { + lock (_cache) + { + _cache[key.Guid] = new Lazy>(Task.FromResult(data)); + } + } + + public bool TryGetCachedValue(DataElementIdentifier identifier, [NotNullWhen(true)] out T? value) + { + lock (_cache) + { + if ( + _cache.TryGetValue(identifier.Guid, out var lazyTask) + && lazyTask is { IsValueCreated: true, Value.IsCompletedSuccessfully: true } + ) + { + value = lazyTask.Value.Result ?? throw new InvalidOperationException("Value in cache is null"); + return true; + } + } + value = default; + return false; + } + } + + private DataType GetDataTypeByString(string dataTypeString) + { + var dataType = _appMetadata.DataTypes.Find(d => d.Id == dataTypeString); + if (dataType is null) + { + throw new InvalidOperationException($"Data type {dataTypeString} not found in app metadata"); + } + + return dataType; + } + + internal void VerifyDataElementsUnchangedSincePreviousChanges(DataElementChanges previousChanges) + { + var changes = GetDataElementChanges(initializeAltinnRowId: false); + if (changes.AllChanges.Count != previousChanges.AllChanges.Count) + { + throw new Exception("Number of data elements have changed by validators"); + } + + foreach (var previousChange in previousChanges.AllChanges) + { + var currentChange = + changes.AllChanges.FirstOrDefault(c => c.DataElement?.Id == previousChange.DataElement?.Id) + ?? throw new Exception("Number of data elements have changed by validators"); + + var equal = (currentChange, previousChange) switch + { + ( + FormDataChange { CurrentBinaryData.Span: var currentSpan }, + FormDataChange { CurrentBinaryData.Span: var previousSpan } + ) + => currentSpan.SequenceEqual(previousSpan), + (BinaryDataChange current, BinaryDataChange previous) + => current.CurrentBinaryData.Span.SequenceEqual(previous.CurrentBinaryData.Span), + _ => throw new Exception("Data element type has changed by validators") + }; + if (!equal) + { + throw new Exception( + $"Data element {previousChange.DataType.Id} with id {previousChange.DataElement?.Id} has been changed by validators" + ); + } + } + } + + private async Task UpdatePresentationTextsOnInstance(Instance instance, string dataType, object serviceModel) + { + var updatedValues = DataHelper.GetUpdatedDataValues( + _appMetadata.PresentationFields, + instance.PresentationTexts, + dataType, + serviceModel + ); + + if (updatedValues.Count > 0) + { + await _instanceClient.UpdatePresentationTexts( + int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), + Guid.Parse(instance.Id.Split("/")[1]), + new PresentationTexts { Texts = updatedValues } + ); + } + } + + private async Task UpdateDataValuesOnInstance(Instance instance, string dataType, object serviceModel) + { + var updatedValues = DataHelper.GetUpdatedDataValues( + _appMetadata.DataFields, + instance.DataValues, + dataType, + serviceModel + ); + + if (updatedValues.Count > 0) + { + await _instanceClient.UpdateDataValues( + int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), + Guid.Parse(instance.Id.Split("/")[1]), + new DataValues { Values = updatedValues } + ); + } + } +} diff --git a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs b/src/Altinn.App.Core/Internal/Patch/IPatchService.cs index ee06c5a46..1ca44a60f 100644 --- a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/IPatchService.cs @@ -1,5 +1,7 @@ using Altinn.App.Core.Features; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Result; +using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Json.Patch; @@ -21,12 +23,23 @@ Task> ApplyPatches( ); /// - /// Runs data processors on all the changes. + /// Runs and on the changes. /// Task RunDataProcessors( IInstanceDataMutator dataMutator, - List changes, + DataElementChanges changes, string taskId, string? language ); + + /// + /// Runs incremental validation on the changes. + /// + Task> RunIncrementalValidation( + IInstanceDataAccessor dataAccessor, + string taskId, + DataElementChanges changes, + List? ignoredValidators, + string? language + ); } diff --git a/src/Altinn.App.Core/Internal/Patch/PatchService.cs b/src/Altinn.App.Core/Internal/Patch/PatchService.cs index fb1463306..e0f4eb3bb 100644 --- a/src/Altinn.App.Core/Internal/Patch/PatchService.cs +++ b/src/Altinn.App.Core/Internal/Patch/PatchService.cs @@ -9,6 +9,7 @@ using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; using Altinn.App.Core.Models.Result; +using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Json.Patch; using Microsoft.AspNetCore.Hosting; @@ -25,7 +26,7 @@ internal class PatchService : IPatchService private readonly IDataClient _dataClient; private readonly IInstanceClient _instanceClient; private readonly ModelSerializationService _modelSerializationService; - private readonly IWebHostEnvironment _hostingEnvironment; + private readonly IHostEnvironment _hostingEnvironment; private readonly Telemetry? _telemetry; private readonly IValidationService _validationService; private readonly IEnumerable _dataProcessors; @@ -45,7 +46,7 @@ public PatchService( IEnumerable dataProcessors, IEnumerable dataWriteProcessors, ModelSerializationService modelSerializationService, - IWebHostEnvironment hostingEnvironment, + IHostEnvironment hostingEnvironment, Telemetry? telemetry = null ) { @@ -70,15 +71,15 @@ public async Task> ApplyPatches( { using var activity = _telemetry?.StartDataPatchActivity(instance); - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerializationService ); - List changesAfterPatch = new(); + List changesAfterPatch = []; foreach (var (dataElementGuid, jsonPatch) in patches) { @@ -133,20 +134,23 @@ public async Task> ApplyPatches( dataAccessor.SetFormData(dataElement, newModel); changesAfterPatch.Add( - new DataElementChange + new FormDataChange { + Type = ChangeType.Updated, DataElement = dataElement, + ContentType = dataElement.ContentType, + DataType = dataAccessor.GetDataType(dataElementIdentifier), PreviousFormData = oldModel, CurrentFormData = newModel, PreviousBinaryData = await dataAccessor.GetBinaryData(dataElementIdentifier), - CurrentBinaryData = null, + CurrentBinaryData = null, // Set this after DataProcessors have run } ); } await RunDataProcessors( dataAccessor, - changesAfterPatch, + new DataElementChanges(changesAfterPatch), taskId: instance.Process.CurrentTask.ElementId, language ); @@ -159,7 +163,6 @@ await RunDataProcessors( await dataAccessor.UpdateInstanceData(changes); var validationIssues = await _validationService.ValidateIncrementalFormData( - instance, dataAccessor, instance.Process.CurrentTask.ElementId, changes, @@ -173,24 +176,39 @@ await RunDataProcessors( if (_hostingEnvironment.IsDevelopment()) { // Ensure that validation did not change the data elements - dataAccessor.VerifyDataElementsUnchanged(); + dataAccessor.VerifyDataElementsUnchangedSincePreviousChanges(changes); } + // report back updated and created data elements with appLogic var updatedData = changes - .Select(change => new DataPatchResult.DataModelPair(change.DataElement, change.CurrentFormData)) + .FormDataChanges.Where(change => change.Type is ChangeType.Updated or ChangeType.Created) + .Select(change => new DataPatchResult.DataModelPair( + change.DataElement + ?? throw new InvalidOperationException( + "DataElement must be set in data changes of type update or created" + ), + change.CurrentFormData + )) .ToList(); + // Ensure that all data elements that were patched are included in the updated data // (even if they were not changed or the change was reverted by dataProcessor) foreach (var patchedElementGuid in patches.Keys) { - if (changes.TrueForAll(c => c.DataElement.Id != patchedElementGuid.ToString())) + var patchedElementId = patchedElementGuid.ToString(); + if (changes.FormDataChanges.All(c => c.DataElement?.Id != patchedElementId)) { - var dataElement = - instance.Data.Find(d => d.Id == patchedElementGuid.ToString()) - ?? throw new InvalidOperationException("Data element not found in instance"); - updatedData.Add( - new DataPatchResult.DataModelPair(dataElement, await dataAccessor.GetFormData(dataElement)) - ); + DataElementIdentifier? dataElement = instance.Data.Find(d => d.Id == patchedElementId); + if (dataElement is not null) + { + // Don't return deleted data elements + updatedData.Add( + new DataPatchResult.DataModelPair( + dataElement.Value, + await dataAccessor.GetFormData(dataElement.Value) + ) + ); + } } } @@ -202,25 +220,45 @@ await RunDataProcessors( }; } + public async Task> RunIncrementalValidation( + IInstanceDataAccessor dataAccessor, + string taskId, + DataElementChanges changes, + List? ignoredValidators, + string? language + ) + { + return await _validationService.ValidateIncrementalFormData( + dataAccessor, + taskId, + changes, + ignoredValidators, + language + ); + } + public async Task RunDataProcessors( IInstanceDataMutator dataMutator, - List changes, + DataElementChanges changes, string taskId, string? language ) { foreach (var dataProcessor in _dataProcessors) { - foreach (var change in changes) + foreach (var change in changes.FormDataChanges) { - var dataElementGuid = Guid.Parse(change.DataElement.Id); + if (change.Type != ChangeType.Updated) + { + // Don't run IDataProcessor on created or deleted data elements for backwards compatibility + continue; + } using var processWriteActivity = _telemetry?.StartDataProcessWriteActivity(dataProcessor); try { - // TODO: Create new dataProcessor interface that takes multiple models at the same time. await dataProcessor.ProcessDataWrite( dataMutator.Instance, - dataElementGuid, + change.DataElementIdentifier.Guid, change.CurrentFormData, change.PreviousFormData, language diff --git a/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs index 35515a701..1aba26284 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs @@ -171,11 +171,11 @@ public async Task Next(ProcessNextRequest request) int? userId = request.User.GetUserIdAsInt(); IUserAction? actionHandler = _userActionService.GetActionHandler(request.Action); - var cachedDataMutator = new CachedInstanceDataAccessor( + var cachedDataMutator = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerialization ); @@ -199,6 +199,8 @@ public async Task Next(ProcessNextRequest request) await cachedDataMutator.UpdateInstanceData(changes); await cachedDataMutator.SaveChanges(changes); + // TODO: consider using the same cachedDataMutator for the rest of the process to avoid refetching data from storage + ProcessStateChange? nextResult = await HandleMoveToNext(instance, request.User, request.Action); if (nextResult?.NewProcessState?.Ended is not null) diff --git a/src/Altinn.App.Core/Internal/Process/ProcessNavigator.cs b/src/Altinn.App.Core/Internal/Process/ProcessNavigator.cs index d1548f90f..e9b72501e 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessNavigator.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessNavigator.cs @@ -114,11 +114,11 @@ private async Task> NextFollowAndFilterGateways( Action = action, DataTypeId = gateway.ExtensionElements?.GatewayExtension?.ConnectedDataTypeId }; - IInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor( + IInstanceDataAccessor dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _instanceClient, - _appMetadata, + await _appMetadata.GetApplicationMetadata(), _modelSerialization ); filteredList = await gatewayFilter.FilterAsync( 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 edc82c64c..59d2b4706 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizer.cs @@ -50,16 +50,16 @@ IOptions appSettings /// public async Task Finalize(string taskId, Instance instance) { - var dataAccessor = new CachedInstanceDataAccessor( + ApplicationMetadata applicationMetadata = await _appMetadata.GetApplicationMetadata(); + + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient, _intanceClient, - _appMetadata, + applicationMetadata, _modelSerializer ); - ApplicationMetadata applicationMetadata = await _appMetadata.GetApplicationMetadata(); - List tasks = []; foreach ( var dataType in applicationMetadata.DataTypes.Where(dt => @@ -80,7 +80,7 @@ var dataType in applicationMetadata.DataTypes.Where(dt => } private async Task RemoveFieldsOnTaskComplete( - CachedInstanceDataAccessor dataAccessor, + InstanceDataUnitOfWork dataAccessor, string taskId, ApplicationMetadata applicationMetadata, DataElement dataElement, diff --git a/src/Altinn.App.Core/Internal/Validation/FileValidationService.cs b/src/Altinn.App.Core/Internal/Validation/FileValidationService.cs index 3b8cc9651..f5d7d7ba0 100644 --- a/src/Altinn.App.Core/Internal/Validation/FileValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/FileValidationService.cs @@ -26,13 +26,13 @@ public FileValidationService(IFileValidatorFactory fileValidatorFactory, Telemet /// /// Runs all registered validators on the specified /// - public async Task<(bool Success, List Errors)> Validate( + public async Task<(bool Success, List Errors)> Validate( DataType dataType, - IEnumerable fileAnalysisResults + List fileAnalysisResults ) { using var activity = _telemetry?.StartFileValidateActivity(); - List allErrors = new(); + List allErrors = new(); bool allSuccess = true; List fileValidators = _fileValidatorFactory @@ -47,7 +47,15 @@ IEnumerable fileAnalysisResults if (!success) { allSuccess = false; - allErrors.AddRange(errors); + allErrors.AddRange( + errors.Select(e => + ValidationIssueWithSource.FromIssue( + e, + fileValidator.GetType().Name, + noIncrementalUpdates: false + ) + ) + ); } } diff --git a/src/Altinn.App.Core/Internal/Validation/IFileValidationService.cs b/src/Altinn.App.Core/Internal/Validation/IFileValidationService.cs index 3b7b6fda1..45fa4f668 100644 --- a/src/Altinn.App.Core/Internal/Validation/IFileValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/IFileValidationService.cs @@ -12,8 +12,8 @@ public interface IFileValidationService /// /// Validates the file based on the file analysis results. /// - Task<(bool Success, List Errors)> Validate( + Task<(bool Success, List Errors)> Validate( DataType dataType, - IEnumerable fileAnalysisResults + List fileAnalysisResults ); } diff --git a/src/Altinn.App.Core/Internal/Validation/IValidationService.cs b/src/Altinn.App.Core/Internal/Validation/IValidationService.cs index cf4db5d18..068faf2f2 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidationService.cs @@ -1,6 +1,6 @@ using Altinn.App.Core.Features; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; namespace Altinn.App.Core.Internal.Validation; @@ -12,7 +12,6 @@ public interface IValidationService /// /// Validates the instance with all data elements on the current task and ensures that the instance is ready for process next. /// - /// The instance to validate /// Accessor for instance data to be validated /// The task to run validations for (overriding instance.Process?.CurrentTask?.ElementId) /// List of to ignore @@ -20,7 +19,6 @@ public interface IValidationService /// The language to run validations in /// List of validation issues for this data element Task> ValidateInstanceAtTask( - Instance instance, IInstanceDataAccessor dataAccessor, string taskId, List? ignoredValidators, @@ -32,7 +30,6 @@ Task> ValidateInstanceAtTask( /// Given a list of changes, evaluate and run the relevant validators to get /// the issues from the validators that might return different results based on the changes. /// - /// The instance to validate /// Accessor for instance data to be validated /// The task to run validations for (overriding instance.Process?.CurrentTask?.ElementId) /// List of changed data elements and values to forward to @@ -40,10 +37,9 @@ Task> ValidateInstanceAtTask( /// The language to run validations in /// Dictionary where the key is the and the value is the list of issues this validator produces public Task> ValidateIncrementalFormData( - Instance instance, IInstanceDataAccessor dataAccessor, string taskId, - List changes, + DataElementChanges changes, List? ignoredValidators, string? language ); diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index f2c4aa3b2..6d940ca1d 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -1,6 +1,6 @@ using Altinn.App.Core.Features; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; -using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.Logging; namespace Altinn.App.Core.Internal.Validation; @@ -30,7 +30,6 @@ public ValidationService( /// public async Task> ValidateInstanceAtTask( - Instance instance, IInstanceDataAccessor dataAccessor, string taskId, List? ignoredValidators, @@ -38,7 +37,8 @@ public async Task> ValidateInstanceAtTask( string? language ) { - ArgumentNullException.ThrowIfNull(instance); + ArgumentNullException.ThrowIfNull(dataAccessor); + ArgumentNullException.ThrowIfNull(dataAccessor.Instance); ArgumentNullException.ThrowIfNull(taskId); using var activity = _telemetry?.StartValidateInstanceAtTaskActivity(taskId); @@ -60,7 +60,7 @@ public async Task> ValidateInstanceAtTask( using var validatorActivity = _telemetry?.StartRunValidatorActivity(v); try { - var issues = await v.Validate(instance, dataAccessor, taskId, language); + var issues = await v.Validate(dataAccessor, taskId, language); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorIssueCount, issues.Count); return KeyValuePair.Create( v.ValidationSource, @@ -76,7 +76,7 @@ public async Task> ValidateInstanceAtTask( "Error while running validator {ValidatorName} for task {TaskId} on instance {InstanceId}", v.ValidationSource, taskId, - instance.Id + dataAccessor.Instance.Id ); validatorActivity?.Errored(e); throw; @@ -94,15 +94,14 @@ public async Task> ValidateInstanceAtTask( /// public async Task> ValidateIncrementalFormData( - Instance instance, IInstanceDataAccessor dataAccessor, string taskId, - List changes, + DataElementChanges changes, List? ignoredValidators, string? language ) { - ArgumentNullException.ThrowIfNull(instance); + ArgumentNullException.ThrowIfNull(dataAccessor.Instance); ArgumentNullException.ThrowIfNull(taskId); ArgumentNullException.ThrowIfNull(changes); @@ -121,11 +120,11 @@ public async Task> ValidateIncrementalFormData( using var validatorActivity = _telemetry?.StartRunValidatorActivity(validator); try { - var hasRelevantChanges = await validator.HasRelevantChanges(instance, dataAccessor, taskId, changes); + var hasRelevantChanges = await validator.HasRelevantChanges(dataAccessor, taskId, changes); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorHasRelevantChanges, hasRelevantChanges); if (hasRelevantChanges) { - var issues = await validator.Validate(instance, dataAccessor, taskId, language); + var issues = await validator.Validate(dataAccessor, taskId, language); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorIssueCount, issues.Count); var issuesWithSource = issues .Select(i => @@ -148,7 +147,7 @@ public async Task> ValidateIncrementalFormData( "Error while running validator {validatorName} on task {taskId} in instance {instanceId}", validator.GetType().Name, taskId, - instance.Id + dataAccessor.Instance.Id ); validatorActivity?.Errored(e); throw; diff --git a/src/Altinn.App.Core/Models/DataElementChanges.cs b/src/Altinn.App.Core/Models/DataElementChanges.cs new file mode 100644 index 000000000..6c55ed7b1 --- /dev/null +++ b/src/Altinn.App.Core/Models/DataElementChanges.cs @@ -0,0 +1,129 @@ +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Models; + +/// +/// Wrapper class for a list of changes with methods to get changes of specific types +/// +public sealed class DataElementChanges +{ + internal DataElementChanges(IReadOnlyList allChanges) + { + AllChanges = allChanges; + } + + /// + /// Get all the changes as the abstract base class + /// + public IReadOnlyList AllChanges { get; } + + /// + /// Get changes to FormData elements + /// + public IEnumerable FormDataChanges => AllChanges.OfType(); + + /// + /// Get changes to attachments elements + /// + public IEnumerable BinaryChanges => AllChanges.OfType(); +} + +/// +/// Represents a change in a data element with current and previous deserialized data +/// +public abstract class DataElementChange +{ + /// + /// The type of update: Create, Update or Delete + /// + public required ChangeType Type { get; init; } + + /// + /// The data element the change is related to (null if a new data element) + /// + public required DataElement? DataElement { get; set; } // needs to be set after saving new elements to storage + + /// + /// The data element identifier or an exception if accessed before it was set + /// + public DataElementIdentifier DataElementIdentifier => + DataElement ?? throw new InvalidOperationException("DataElement was accessed before it was set"); + + /// + /// The data type of the data element + /// + public required DataType DataType { get; init; } + + /// + /// The content type of element in storage + /// + public required string ContentType { get; init; } +} + +/// +/// The type of change to a data element +/// +public enum ChangeType +{ + /// + /// The data element was created and will not have set + /// + Created, + + /// + /// The data element was updated and will have a set + /// + Updated, + + /// + /// The data element was deleted and will have set + /// + Deleted +} + +/// +/// Representation of a change to a binary data element +/// +public sealed class BinaryDataChange : DataElementChange +{ + /// + /// The file name of the attachment file + /// + public required string? FileName { get; init; } + + /// + /// The binary data + /// + public required ReadOnlyMemory CurrentBinaryData { get; init; } +} + +/// +/// A change to a data element +/// +public sealed class FormDataChange : DataElementChange +{ + /// + /// The state of the data element before the change + /// + public required object PreviousFormData { get; init; } + + /// + /// The state of the data element after the change + /// + public required object CurrentFormData { get; init; } + + /// + /// The binary representation (for storage) of the data element before changes + /// + /// Empty memory for new data elements + public required ReadOnlyMemory PreviousBinaryData { get; init; } + + /// + /// The binary representation (for storage) of the data element after changes + /// + /// + /// Not available for form data in data processing phase, because it can't reflect + /// the changes made by the data processors. + /// + public required ReadOnlyMemory? CurrentBinaryData { get; init; } +} diff --git a/src/Altinn.App.Core/Models/DataElementIdentifier.cs b/src/Altinn.App.Core/Models/DataElementIdentifier.cs index 4cffb450a..447017b94 100644 --- a/src/Altinn.App.Core/Models/DataElementIdentifier.cs +++ b/src/Altinn.App.Core/Models/DataElementIdentifier.cs @@ -17,27 +17,37 @@ namespace Altinn.App.Core.Models; /// public string Id { get; } - private DataElementIdentifier(Guid guid, string id) + /// + /// Constructor that takes a string representation of a guid + /// + /// The + public DataElementIdentifier(string id) { - Guid = guid; + Guid = Guid.Parse(id); Id = id; } /// - /// Implicit conversion to allow DataElements to be used as DataElementIds + /// Constructor that takes a guid /// - public static implicit operator DataElementIdentifier(DataElement dataElement) => - new(Guid.Parse(dataElement.Id), dataElement.Id); + /// The parsed as a guid + public DataElementIdentifier(Guid guid) + { + Guid = guid; + Id = guid.ToString(); + } /// - /// Make the implicit conversion from string (containing a valid guid) to DataElementIdentifier work + /// Implicit conversion to allow DataElements to be used as DataElementIds /// - public static explicit operator DataElementIdentifier(string id) => new(Guid.Parse(id), id); + public static implicit operator DataElementIdentifier(DataElement dataElement) => new(dataElement.Id); /// - /// Make the implicit conversion from guid to DataElementIdentifier work + /// Implicit conversion to allow DataElements to be used as DataElementIds, + /// but accept and return null values /// - public static explicit operator DataElementIdentifier(Guid guid) => new(guid, guid.ToString()); + public static implicit operator DataElementIdentifier?(DataElement? dataElement) => + dataElement is null ? default : new(dataElement.Id); /// /// Make the ToString method return the ID diff --git a/src/Altinn.App.Core/Models/Layout/LayoutModel.cs b/src/Altinn.App.Core/Models/Layout/LayoutModel.cs index 2bd153c8f..28682720d 100644 --- a/src/Altinn.App.Core/Models/Layout/LayoutModel.cs +++ b/src/Altinn.App.Core/Models/Layout/LayoutModel.cs @@ -72,14 +72,11 @@ public async Task> GenerateComponentContexts(Instance ins var pageContexts = new List(); foreach (var page in _defaultLayoutSet.Pages) { - pageContexts.Add( - await GenerateComponentContextsRecurs( - page, - dataModel, - _defaultLayoutSet.GetDefaultDataElementId(instance), - [] - ) - ); + var defaultElementId = _defaultLayoutSet.GetDefaultDataElementId(instance); + if (defaultElementId is not null) + { + pageContexts.Add(await GenerateComponentContextsRecurs(page, dataModel, defaultElementId.Value, [])); + } } return pageContexts; @@ -208,8 +205,8 @@ DataElementIdentifier defaultDataElementIdentifier return new ComponentContext(subFormComponent, null, null, defaultDataElementIdentifier, children); } - internal DataElementIdentifier GetDefaultDataElementId(Instance instanceContext) + internal DataElementIdentifier? GetDefaultDataElementId(Instance instance) { - return _defaultLayoutSet.GetDefaultDataElementId(instanceContext); + return _defaultLayoutSet.GetDefaultDataElementId(instance); } } diff --git a/src/Altinn.App.Core/Models/Layout/LayoutSetComponent.cs b/src/Altinn.App.Core/Models/Layout/LayoutSetComponent.cs index 1846f938b..18075088e 100644 --- a/src/Altinn.App.Core/Models/Layout/LayoutSetComponent.cs +++ b/src/Altinn.App.Core/Models/Layout/LayoutSetComponent.cs @@ -52,10 +52,14 @@ public PageComponent GetPage(string pageName) /// /// Get the of the that is default for this layout /// - public DataElementIdentifier GetDefaultDataElementId(Instance instance) + public DataElementIdentifier? GetDefaultDataElementId(Instance instance) { var dataType = DefaultDataType.Id; - return instance.Data.Find(d => d.DataType == dataType) - ?? throw new ArgumentException($"Data element with type {dataType} not found"); + var dataElement = instance.Data.Find(d => d.DataType == dataType); + if (dataElement is null) + { + return null; + } + return (DataElementIdentifier?)dataElement; } } diff --git a/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs b/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs index fc050eeab..551c6ac27 100644 --- a/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs +++ b/src/Altinn.App.Core/Models/Validation/ValidationIssueWithSource.cs @@ -105,4 +105,7 @@ public static ValidationIssueWithSource FromIssue(ValidationIssue issue, string /// /// The for the Validator that created theese issues /// List of issues -public record ValidationSourcePair(string Source, List Issues); +public record ValidationSourcePair( + [property: JsonPropertyName("source")] string Source, + [property: JsonPropertyName("issues")] List Issues +); diff --git a/test/Altinn.App.Api.Tests/Controllers/DataControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataControllerTests.cs index 6b52cf9dc..ca16fa3f5 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataControllerTests.cs @@ -40,6 +40,44 @@ public async Task PutDataElement_MissingDataType_ReturnsBadRequest() responseContent.Should().Contain("dataType"); } + [Fact] + public async Task PostBinaryElement_ContentTooLarge_ReturnsBadRequest() + { + // Setup test data + string org = "tdd"; + string app = "contributer-restriction"; + int instanceOwnerPartyId = 1337; + string dataType = "specificFileType"; // Should have restrictions on 1 mb in app metadata + Guid guid = new Guid("0fc98a23-fe31-4ef5-8fb9-dd3f479354cd"); + HttpClient client = GetRootedClient(org, app); + string token = PrincipalUtil.GetOrgToken("nav", "160694123"); + client.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue("Bearer", token); + + TestData.DeleteInstanceAndData(org, app, instanceOwnerPartyId, guid); + TestData.PrepareInstance(org, app, instanceOwnerPartyId, guid); + + using var content = new ByteArrayContent(new byte[1024 * 1024 + 1]); // 1 mb + content.Headers.ContentType = new MediaTypeHeaderValue("application/pdf"); + content.Headers.ContentDisposition = new ContentDispositionHeaderValue("attachment") + { + FileName = "example.pdf" + }; + var response = await client.PostAsync( + $"/{org}/{app}/instances/{instanceOwnerPartyId}/{guid}/data/{dataType}", + content + ); + var responseContent = await response.Content.ReadAsStringAsync(); + OutputHelper.WriteLine(responseContent); + response.StatusCode.Should().Be(HttpStatusCode.BadRequest); + responseContent + .Should() + .Contain( + """ + "code":"DataElementTooLarge","description":"Invalid data provided. Error: Binary attachment exceeds limit of 1048576","source":"DataRestrictionValidation" + """ + ); + } + [Fact] public async Task CreateDataElement_BinaryPdf_AnalyserShouldRunOk() { diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs index d1e576181..ef828f916 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_LayoutEvaluatorTests.cs @@ -46,7 +46,7 @@ public async Task ProcessDataWrite( var hidden = await LayoutEvaluator.GetHiddenFieldsForRemoval(layoutEvaluatorState); if (dataId.HasValue) { - var id = (DataElementIdentifier)dataId; + var id = new DataElementIdentifier(dataId.Value); hidden .Should() .BeEquivalentTo([new DataReference() { DataElementIdentifier = id, Field = "melding.hidden" }]); diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_PutTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_PutTests.cs index 22747853d..e00709e19 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_PutTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_PutTests.cs @@ -64,7 +64,7 @@ public async Task PutDataElement_TestSinglePartUpdate_ReturnsOk() p.ProcessDataWrite( It.IsAny(), It.IsAny(), - It.IsAny>(), + It.IsAny(), It.IsAny() ) ) @@ -166,7 +166,7 @@ public async Task PutDataElement_TestMultiPartUpdateWithCustomDataProcessor_Retu p.ProcessDataWrite( It.IsAny(), It.IsAny(), - It.IsAny>(), + It.IsAny(), It.IsAny() ) ) diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs index 98c8f3133..4fa6e89d9 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs @@ -2,11 +2,13 @@ using Altinn.App.Api.Models; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers.Serialization; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Events; using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Prefill; using Altinn.App.Core.Internal.Profile; using Altinn.App.Core.Internal.Registers; @@ -43,9 +45,14 @@ public class InstancesController_ActiveInstancesTest private readonly Mock _processEngine = new(); private readonly Mock _oarganizationClientMock = new(); private readonly Mock _envMock = new(); + private readonly ModelSerializationService _modelSerializationService; - private InstancesController SUT => - new InstancesController( + private InstancesController SUT; + + public InstancesController_ActiveInstancesTest() + { + _modelSerializationService = new ModelSerializationService(_appModel.Object); + SUT = new InstancesController( _logger.Object, _registrer.Object, _instanceClient.Object, @@ -61,8 +68,20 @@ public class InstancesController_ActiveInstancesTest _profile.Object, _processEngine.Object, _oarganizationClientMock.Object, - _envMock.Object + _envMock.Object, + _modelSerializationService, + new PatchService( + _appMetadata.Object, + _data.Object, + _instanceClient.Object, + null!, + [], + [], + _modelSerializationService, + _envMock.Object + ) ); + } private void VerifyNoOtherCalls() { diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs index af9aab7cc..78d558b1f 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -3,14 +3,17 @@ using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Helpers; +using Altinn.App.Core.Helpers.Serialization; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Events; using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Prefill; using Altinn.App.Core.Internal.Profile; using Altinn.App.Core.Internal.Registers; +using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; using Altinn.App.Core.Models.Process; using Altinn.App.Core.Models.Validation; @@ -46,6 +49,7 @@ public class InstancesController_CopyInstanceTests private readonly Mock _httpContextMock = new(); private readonly Mock _oarganizationClientMock = new(); private readonly Mock _envMock = new(); + private readonly Mock _validationService = new(MockBehavior.Strict); private readonly InstancesController SUT; @@ -53,6 +57,17 @@ public InstancesController_CopyInstanceTests() { ControllerContext controllerContext = new ControllerContext() { HttpContext = _httpContextMock.Object }; + var modelSerializationService = new ModelSerializationService(_appModel.Object); + var patchService = new PatchService( + _appMetadata.Object, + _data.Object, + _instanceClient.Object, + _validationService.Object, + [], + [], + modelSerializationService, + _envMock.Object + ); SUT = new InstancesController( _logger.Object, _registrer.Object, @@ -69,7 +84,9 @@ public InstancesController_CopyInstanceTests() _profile.Object, _processEngine.Object, _oarganizationClientMock.Object, - _envMock.Object + _envMock.Object, + modelSerializationService, + patchService ) { ControllerContext = controllerContext diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs index 6cd9992cf..9689c1b18 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerTests.cs @@ -132,9 +132,7 @@ public async Task ValidateInstance_returns_OK_with_messages() .Returns(Task.FromResult(instance)); _validationMock - .Setup(v => - v.ValidateInstanceAtTask(instance, It.IsAny(), "dummy", null, null, null) - ) + .Setup(v => v.ValidateInstanceAtTask(It.IsAny(), "dummy", null, null, null)) .ReturnsAsync(validationResult); // Act @@ -166,9 +164,7 @@ public async Task ValidateInstance_returns_403_when_not_authorized() .Returns(Task.FromResult(instance)); _validationMock - .Setup(v => - v.ValidateInstanceAtTask(instance, It.IsAny(), "dummy", null, null, null) - ) + .Setup(v => v.ValidateInstanceAtTask(It.IsAny(), "dummy", null, null, null)) .Throws(exception); // Act @@ -200,9 +196,7 @@ public async Task ValidateInstance_throws_PlatformHttpException_when_not_403() .Returns(Task.FromResult(instance)); _validationMock - .Setup(v => - v.ValidateInstanceAtTask(instance, It.IsAny(), "dummy", null, null, null) - ) + .Setup(v => v.ValidateInstanceAtTask(It.IsAny(), "dummy", null, null, null)) .Throws(exception); // Act diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs index 826d15557..71cc09701 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateControllerValidateDataTests.cs @@ -253,14 +253,7 @@ private void SetupMocks(string app, string org, int instanceOwnerId, ValidateDat { _validationMock .Setup(v => - v.ValidateInstanceAtTask( - testScenario.ReceivedInstance, - It.IsAny(), - "Task_1", - null, - It.IsAny(), - null - ) + v.ValidateInstanceAtTask(It.IsAny(), "Task_1", null, It.IsAny(), null) ) .ReturnsAsync(testScenario.ReceivedValidationIssues); } diff --git a/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs b/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs index 2f9fe19c2..8d7b35243 100644 --- a/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs +++ b/test/Altinn.App.Api.Tests/CustomWebApplicationFactory.cs @@ -222,11 +222,11 @@ protected async Task VerifyStatusAndDeserialize( HttpStatusCode expectedStatusCode ) { - // Verify status code - response.Should().HaveStatusCode(expectedStatusCode); - // Deserialize content and log everything if it fails var content = await response.Content.ReadAsStringAsync(); + OutputHelper.WriteLine($"Response content: {content}"); + // Verify status code + response.Should().HaveStatusCode(expectedStatusCode); try { return JsonSerializer.Deserialize(content, JsonSerializerOptions) diff --git a/test/Altinn.App.Api.Tests/OpenApi/swagger.json b/test/Altinn.App.Api.Tests/OpenApi/swagger.json index 7a1616b19..f18f8c2a6 100644 --- a/test/Altinn.App.Api.Tests/OpenApi/swagger.json +++ b/test/Altinn.App.Api.Tests/OpenApi/swagger.json @@ -586,7 +586,8 @@ } } } - } + }, + "deprecated": true }, "patch": { "tags": [ @@ -798,6 +799,173 @@ } } }, + "/{org}/{app}/instances/{instanceOwnerPartyId}/{instanceGuid}/data/{dataType}": { + "post": { + "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": "dataType", + "in": "path", + "required": true, + "schema": { + "type": "string" + } + }, + { + "name": "ignoredValidators", + "in": "query", + "schema": { + "type": "string" + } + }, + { + "name": "language", + "in": "query", + "schema": { + "type": "string" + } + } + ], + "responses": { + "201": { + "description": "Created", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/DataPostResponse" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/DataPostResponse" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/DataPostResponse" + } + } + } + }, + "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" + } + } + } + }, + "400": { + "description": "Bad Request", + "content": { + "text/plain": { + "schema": { + "$ref": "#/components/schemas/DataPostErrorResponse" + } + }, + "application/json": { + "schema": { + "$ref": "#/components/schemas/DataPostErrorResponse" + } + }, + "text/json": { + "schema": { + "$ref": "#/components/schemas/DataPostErrorResponse" + } + } + } + }, + "404": { + "description": "Not Found", + "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": [ @@ -1184,6 +1352,13 @@ "type": "string", "format": "uuid" } + }, + { + "name": "language", + "in": "query", + "schema": { + "type": "string" + } } ], "responses": { @@ -1998,6 +2173,13 @@ "type": "integer", "format": "int32" } + }, + { + "name": "language", + "in": "query", + "schema": { + "type": "string" + } } ], "responses": { @@ -5799,6 +5981,74 @@ }, "additionalProperties": false }, + "DataPostErrorResponse": { + "type": "object", + "properties": { + "type": { + "type": "string", + "nullable": true + }, + "title": { + "type": "string", + "nullable": true + }, + "status": { + "type": "integer", + "format": "int32", + "nullable": true + }, + "detail": { + "type": "string", + "nullable": true + }, + "instance": { + "type": "string", + "nullable": true + }, + "uploadValidationIssues": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ValidationIssueWithSource" + }, + "nullable": true, + "readOnly": true + } + }, + "additionalProperties": { } + }, + "DataPostResponse": { + "required": [ + "instance", + "newDataElementId", + "newDataModels", + "validationIssues" + ], + "type": "object", + "properties": { + "newDataElementId": { + "type": "string", + "format": "uuid" + }, + "instance": { + "$ref": "#/components/schemas/Instance" + }, + "validationIssues": { + "type": "array", + "items": { + "$ref": "#/components/schemas/ValidationSourcePair" + }, + "nullable": true + }, + "newDataModels": { + "type": "array", + "items": { + "$ref": "#/components/schemas/DataModelPairResponse" + }, + "nullable": true + } + }, + "additionalProperties": false + }, "DataType": { "type": "object", "properties": { diff --git a/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml b/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml index 72ec77d7e..cbc90aa23 100644 --- a/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml +++ b/test/Altinn.App.Api.Tests/OpenApi/swagger.yaml @@ -356,6 +356,7 @@ paths: text/xml: schema: $ref: '#/components/schemas/DataElement' + deprecated: true patch: tags: - Data @@ -482,6 +483,107 @@ paths: text/xml: schema: $ref: '#/components/schemas/ProblemDetails' + '/{org}/{app}/instances/{instanceOwnerPartyId}/{instanceGuid}/data/{dataType}': + post: + 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: dataType + in: path + required: true + schema: + type: string + - name: ignoredValidators + in: query + schema: + type: string + - name: language + in: query + schema: + type: string + responses: + '201': + description: Created + content: + text/plain: + schema: + $ref: '#/components/schemas/DataPostResponse' + application/json: + schema: + $ref: '#/components/schemas/DataPostResponse' + text/json: + schema: + $ref: '#/components/schemas/DataPostResponse' + '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' + '400': + description: Bad Request + content: + text/plain: + schema: + $ref: '#/components/schemas/DataPostErrorResponse' + application/json: + schema: + $ref: '#/components/schemas/DataPostErrorResponse' + text/json: + schema: + $ref: '#/components/schemas/DataPostErrorResponse' + '404': + description: Not Found + 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: @@ -723,6 +825,10 @@ paths: schema: type: string format: uuid + - name: language + in: query + schema: + type: string responses: '200': description: OK @@ -1222,6 +1328,10 @@ paths: schema: type: integer format: int32 + - name: language + in: query + schema: + type: string responses: '201': description: Created @@ -3627,6 +3737,56 @@ components: instance: $ref: '#/components/schemas/Instance' additionalProperties: false + DataPostErrorResponse: + type: object + properties: + type: + type: string + nullable: true + title: + type: string + nullable: true + status: + type: integer + format: int32 + nullable: true + detail: + type: string + nullable: true + instance: + type: string + nullable: true + uploadValidationIssues: + type: array + items: + $ref: '#/components/schemas/ValidationIssueWithSource' + nullable: true + readOnly: true + additionalProperties: { } + DataPostResponse: + required: + - instance + - newDataElementId + - newDataModels + - validationIssues + type: object + properties: + newDataElementId: + type: string + format: uuid + instance: + $ref: '#/components/schemas/Instance' + validationIssues: + type: array + items: + $ref: '#/components/schemas/ValidationSourcePair' + nullable: true + newDataModels: + type: array + items: + $ref: '#/components/schemas/DataModelPairResponse' + nullable: true + additionalProperties: false DataType: type: object properties: diff --git a/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs index d9568cffd..101a14482 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/ExpressionValidatorTests.cs @@ -89,7 +89,6 @@ private async Task RunExpressionValidationTest(string fileName, string folder) var dataAccessor = new InstanceDataAccessorFake(instance) { { dataElement, dataModel } }; var validationIssues = await _validator.ValidateFormData( - instance, dataElement, dataAccessor, JsonSerializer.Serialize(testCase.ValidationConfig), 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 70eab4a60..c097dd3c4 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/Default/LegacyIValidationFormDataTests.cs @@ -7,6 +7,7 @@ using Altinn.App.Core.Internal.App; using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; +using Altinn.App.Core.Tests.LayoutExpressions.TestUtilities; using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; using Microsoft.AspNetCore.Mvc.ModelBinding; @@ -19,7 +20,7 @@ public class LegacyIValidationFormDataTests private readonly LegacyIInstanceValidatorFormDataValidator _validator; private readonly Mock _instanceValidator = new(MockBehavior.Strict); private readonly Mock _appMetadata = new(MockBehavior.Strict); - private readonly Mock _instanceDataAccessor = new(MockBehavior.Strict); + private readonly InstanceDataAccessorFake _instanceDataAccessor; private readonly ApplicationMetadata _applicationMetadata = new ApplicationMetadata("ttd/test") { @@ -57,6 +58,7 @@ public LegacyIValidationFormDataTests() InstanceOwner = new InstanceOwner() { PartyId = "1", }, Data = [_dataElement] }; + _instanceDataAccessor = new InstanceDataAccessorFake(_instance, _applicationMetadata, "Task_1", "test"); } [Fact] @@ -77,10 +79,10 @@ public async Task ValidateFormData_WithErrors() ) .Verifiable(Times.Once); - _instanceDataAccessor.Setup(ida => ida.GetFormData(_dataElement)).ReturnsAsync(data); + _instanceDataAccessor.Add(_dataElement, data); // Act - var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null); + var result = await _validator.Validate(_instanceDataAccessor, "Task_1", null); // Assert result @@ -112,7 +114,6 @@ public async Task ValidateFormData_WithErrors() ) ); - _instanceDataAccessor.Verify(); _instanceValidator.Verify(); } @@ -156,10 +157,10 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string } ) .Verifiable(Times.Once); - _instanceDataAccessor.Setup(ida => ida.GetFormData(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once); + _instanceDataAccessor.Add(_dataElement, data); // Act - var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null); + var result = await _validator.Validate(_instanceDataAccessor, "Task_1", null); // Assert result.Should().HaveCount(2); @@ -173,7 +174,6 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string 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/LegacyValidationServiceTests/ValidationServiceOldTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceOldTests.cs index 08e9fa6a4..c0a9197db 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceOldTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceOldTests.cs @@ -9,6 +9,7 @@ using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; +using Altinn.App.Core.Tests.LayoutExpressions.TestUtilities; using Altinn.Platform.Storage.Interface.Enums; using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; @@ -72,10 +73,11 @@ public async Task FileScanEnabled_VirusFound_ValidationShouldFail() _applicationMetadata.DataTypes.Add(dataType); var dataElement = new DataElement() { DataType = "testScan", FileScanResult = FileScanResult.Infected }; var instance = new Instance() { Data = [dataElement] }; - var dataAccessor = new Mock(); + var dataAccessor = new Mock(MockBehavior.Strict); + dataAccessor.SetupGet(da => da.Instance).Returns(instance); + dataAccessor.SetupGet(da => da.DataElements).Returns(instance.Data); List validationIssues = await validationService.ValidateInstanceAtTask( - instance, dataAccessor.Object, "Task_1", null, @@ -102,10 +104,11 @@ public async Task FileScanEnabled_PendingScanNotEnabled_ValidationShouldNotFail( var dataElement = new DataElement() { DataType = "test", FileScanResult = FileScanResult.Pending, }; var instance = new Instance() { Data = [dataElement] }; - var dataAccessorMock = new Mock(); + var dataAccessorMock = new Mock(MockBehavior.Strict); + dataAccessorMock.SetupGet(da => da.Instance).Returns(instance); + dataAccessorMock.SetupGet(da => da.DataElements).Returns(instance.Data); List validationIssues = await validationService.ValidateInstanceAtTask( - instance, dataAccessorMock.Object, "Task_1", null, @@ -132,10 +135,11 @@ public async Task FileScanEnabled_PendingScanEnabled_ValidationShouldNotFail() _applicationMetadata.DataTypes.Add(dataType); var dataElement = new DataElement() { DataType = "testScan", FileScanResult = FileScanResult.Pending }; var instance = new Instance() { Data = [dataElement], }; - var dataAccessorMock = new Mock(); + var dataAccessorMock = new Mock(MockBehavior.Strict); + dataAccessorMock.SetupGet(da => da.Instance).Returns(instance); + dataAccessorMock.SetupGet(da => da.DataElements).Returns(instance.Data); List validationIssues = await validationService.ValidateInstanceAtTask( - instance, dataAccessorMock.Object, "Task_1", null, @@ -161,11 +165,13 @@ public async Task FileScanEnabled_Clean_ValidationShouldNotFail() Data = [dataElement] }; - var dataAccessorMock = new Mock(); + var dataAccessorMock = new InstanceDataAccessorFake(instance, _applicationMetadata, "Task_1", "test") + { + { dataElement, new ReadOnlyMemory() } + }; List validationIssues = await validationService.ValidateInstanceAtTask( - instance, - dataAccessorMock.Object, + dataAccessorMock, "Task_1", null, null, @@ -208,16 +214,11 @@ public async Task ValidateAndUpdateProcess_set_canComplete_validationstatus_and_ }, Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "Task_1" } } }; - var dataAccessorMock = new Mock(); + var dataAccessorMock = new Mock(MockBehavior.Strict); + dataAccessorMock.SetupGet(da => da.Instance).Returns(instance); + dataAccessorMock.SetupGet(da => da.DataElements).Returns(instance.Data); - var issues = await validationService.ValidateInstanceAtTask( - instance, - dataAccessorMock.Object, - taskId, - null, - null, - null - ); + var issues = await validationService.ValidateInstanceAtTask(dataAccessorMock.Object, taskId, null, null, null); issues.Should().BeEmpty(); // instance.Process?.CurrentTask?.Validated.CanCompleteTask.Should().BeTrue(); @@ -267,16 +268,11 @@ public async Task ValidateAndUpdateProcess_set_canComplete_false_validationstatu }, Process = new ProcessState { CurrentTask = new ProcessElementInfo { ElementId = "Task_1" } } }; - var dataAccessorMock = new Mock(); + var dataAccessorMock = new Mock(MockBehavior.Strict); + dataAccessorMock.SetupGet(da => da.Instance).Returns(instance); + dataAccessorMock.SetupGet(da => da.DataElements).Returns(instance.Data); - var issues = await validationService.ValidateInstanceAtTask( - instance, - dataAccessorMock.Object, - taskId, - null, - null, - null - ); + var issues = await validationService.ValidateInstanceAtTask(dataAccessorMock.Object, taskId, null, null, null); issues.Should().HaveCount(1); issues.Should().ContainSingle(i => i.Code == ValidationIssueCodes.InstanceCodes.TooManyDataElementsOfType); } diff --git a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs index 8b993b50a..1b174c411 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs @@ -113,11 +113,11 @@ public class MyModel public ValidationServiceTests() { _modelSerialization = new ModelSerializationService(_appModelMock.Object); - _dataAccessor = new CachedInstanceDataAccessor( + _dataAccessor = new InstanceDataUnitOfWork( _defaultInstance, _dataClientMock.Object, _instanceClientMock.Object, - _appMetadataMock.Object, + _defaultAppMetadata, _modelSerialization ); _serviceCollection.AddSingleton(_loggerMock.Object); @@ -274,7 +274,6 @@ public async Task Validate_WithNoValidators_ReturnsNoErrors() var validatorService = serviceProvider.GetRequiredService(); var resultTask = await validatorService.ValidateInstanceAtTask( - _defaultInstance, _dataAccessor, DefaultTaskId, null, @@ -318,18 +317,23 @@ public async Task ValidateFormData_WithSpecificValidator() var previousData = new MyModel() { Name = "Kari" }; SetupDataClient(data); var result = await validatorService.ValidateIncrementalFormData( - _defaultInstance, _dataAccessor, "Task_1", - new List - { - new() - { - DataElement = _defaultDataElement, - PreviousFormData = previousData, - CurrentFormData = data - } - }, + new DataElementChanges( + [ + new FormDataChange() + { + Type = ChangeType.Updated, + DataElement = _defaultDataElement, + DataType = _defaultDataType, + ContentType = "application/xml", + PreviousFormData = previousData, + CurrentFormData = data, + PreviousBinaryData = null, + CurrentBinaryData = null + } + ] + ), null, DefaultLanguage ); @@ -364,25 +368,31 @@ public async Task ValidateFormData_WithMyNameValidator_ReturnsErrorsWhenNameIsKa var validatorService = serviceProvider.GetRequiredService(); var data = new MyModel { Name = "Kari" }; - List dataElementChanges = - [ - new DataElementChange() - { - DataElement = _defaultDataElement, - CurrentFormData = data, - PreviousFormData = data, - } - ]; + DataElementChanges dataElementChanges = + new( + [ + new FormDataChange() + { + Type = ChangeType.Updated, + DataElement = _defaultDataElement, + DataType = _defaultDataType, + ContentType = "application/xml", + CurrentFormData = data, + PreviousFormData = data, + PreviousBinaryData = null, + CurrentBinaryData = null + } + ] + ); SetupDataClient(data); - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( _defaultInstance, _dataClientMock.Object, _instanceClientMock.Object, - _appMetadataMock.Object, + _defaultAppMetadata, _modelSerialization ); var resultData = await validatorService.ValidateIncrementalFormData( - _defaultInstance, dataAccessor, "Task_1", dataElementChanges, @@ -453,16 +463,15 @@ List CreateIssues(string code) await using var serviceProvider = _serviceCollection.BuildServiceProvider(); var validationService = serviceProvider.GetRequiredService(); - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( _defaultInstance, _dataClientMock.Object, _instanceClientMock.Object, - _appMetadataMock.Object, + _defaultAppMetadata, _modelSerialization ); var taskResult = await validationService.ValidateInstanceAtTask( - _defaultInstance, dataAccessor, DefaultTaskId, null, @@ -526,7 +535,6 @@ public async Task ValidateTask_ReturnsNoErrorsFromAllLevels() var validationService = serviceProvider.GetRequiredService(); var result = await validationService.ValidateInstanceAtTask( - _defaultInstance, _dataAccessor, DefaultTaskId, null, diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 693cfe782..8e2f1a7df 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -57,7 +57,7 @@ public Task InitializeAsync() private Mock RegisterValidatorMock( string source, bool? hasRelevantChanges = null, - List? expectedChanges = null, + DataElementChanges? expectedChanges = null, List? issues = null, string? expectedLanguage = null, bool noIncrementalValidation = false @@ -68,14 +68,13 @@ private Mock RegisterValidatorMock( mock.Setup(v => v.ValidationSource).Returns(source); if (hasRelevantChanges.HasValue && expectedChanges is not null) { - mock.Setup(v => v.HasRelevantChanges(_instance, _instanceDataAccessor, "Task_1", expectedChanges)) + mock.Setup(v => v.HasRelevantChanges(_instanceDataAccessor, "Task_1", expectedChanges)) .ReturnsAsync(hasRelevantChanges.Value); } if (issues is not null) { - mock.Setup(v => v.Validate(_instance, _instanceDataAccessor, "Task_1", expectedLanguage)) - .ReturnsAsync(issues); + mock.Setup(v => v.Validate(_instanceDataAccessor, "Task_1", expectedLanguage)).ReturnsAsync(issues); } mock.SetupGet(v => v.NoIncrementalValidation).Returns(noIncrementalValidation); @@ -91,14 +90,7 @@ public async Task ValidateInstanceAtTask_WithNoData_ShouldReturnNoIssues() // Act var validationService = _serviceProvider.Value.GetRequiredService(); - var result = await validationService.ValidateInstanceAtTask( - _instance, - _instanceDataAccessor, - "Task_1", - null, - null, - null - ); + var result = await validationService.ValidateInstanceAtTask(_instanceDataAccessor, "Task_1", null, null, null); // Assert Assert.Empty(result); @@ -111,9 +103,8 @@ public async Task ValidateIncrementalFormData_WithNoData_ShouldReturnNoIssues() { var validationService = _serviceProvider.Value.GetRequiredService(); - var changes = new List(); + var changes = new DataElementChanges([]); var result = await validationService.ValidateIncrementalFormData( - _instance, _instanceDataAccessor, "Task_1", changes, @@ -129,7 +120,7 @@ public async Task ValidateIncrementalFormData_WithNoData_ShouldReturnNoIssues() [Fact] public async Task ValidateIncrementalFormData_WithIgnoredValidators_ShouldRunOnlyNonIgnoredValidators() { - var changes = new List(); + var changes = new DataElementChanges([]); var issues = new List(); RegisterValidatorMock("IgnoredValidator"); // Throws error if changes or validation is called @@ -137,7 +128,6 @@ public async Task ValidateIncrementalFormData_WithIgnoredValidators_ShouldRunOnl var validationService = _serviceProvider.Value.GetRequiredService(); var result = await validationService.ValidateIncrementalFormData( - _instance, _instanceDataAccessor, "Task_1", changes, @@ -168,7 +158,6 @@ public async Task ValidateInstanceAtTask_WithIgnoredValidators_ShouldRunOnlyNonI var validationService = _serviceProvider.Value.GetRequiredService(); var result = await validationService.ValidateInstanceAtTask( - _instance, _instanceDataAccessor, "Task_1", new List { "IgnoredValidator" }, @@ -205,7 +194,6 @@ public async Task ValidateInstanceAtTask_WithDifferentValidators_ShouldIgnoreNon var validationService = _serviceProvider.Value.GetRequiredService(); var issues = await validationService.ValidateInstanceAtTask( - _instance, _instanceDataAccessor, "Task_1", null, @@ -218,25 +206,16 @@ public async Task ValidateInstanceAtTask_WithDifferentValidators_ShouldIgnoreNon switch (onlyIncrementalValidators) { case true: - incrementalMock.Verify(v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), Times.Once); - nonIncrementalMock.Verify( - v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), - Times.Never - ); + incrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Once); + nonIncrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Never); break; case false: - incrementalMock.Verify(v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), Times.Never); - nonIncrementalMock.Verify( - v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), - Times.Once - ); + incrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Never); + nonIncrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Once); break; case null: - incrementalMock.Verify(v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), Times.Once); - nonIncrementalMock.Verify( - v => v.Validate(_instance, _instanceDataAccessor, "Task_1", null), - Times.Once - ); + incrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Once); + nonIncrementalMock.Verify(v => v.Validate(_instanceDataAccessor, "Task_1", null), Times.Once); break; } } @@ -302,14 +281,7 @@ public async Task GenericFormDataValidator_serviceModelIsString_CallsValidatorFu _services.AddSingleton(genericValidator); var validationService = _serviceProvider.Value.GetRequiredService(); - var issues = await validationService.ValidateInstanceAtTask( - _instance, - _instanceDataAccessor, - taskId, - null, - null, - null - ); + var issues = await validationService.ValidateInstanceAtTask(_instanceDataAccessor, taskId, null, null, null); var issue = issues.Should().ContainSingle().Which; issue.Source.Should().Be($"{genericValidator.GetType().FullName}-{defaultDataType}"); issue.DataElementId.Should().Be(dataElement.Id); @@ -408,14 +380,7 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() ); var validationService = _serviceProvider.Value.GetRequiredService(); - var issues = await validationService.ValidateInstanceAtTask( - _instance, - _instanceDataAccessor, - "Task_1", - null, - null, - null - ); + var issues = await validationService.ValidateInstanceAtTask(_instanceDataAccessor, "Task_1", null, null, null); issues.Should().ContainSingle(i => i.Code == "TestCode543"); formDataValidatorNoAppLogicMock.Verify(); @@ -456,29 +421,38 @@ public async Task GenericFormDataValidator_serviceModelIsString_CallsValidatorFu } ]; - List changes = - new() - { - new DataElementChange() + var changes = new DataElementChanges( + [ + new FormDataChange() { + Type = ChangeType.Updated, DataElement = dataElement, + DataType = _instanceDataAccessor.GetDataType(dataElement), + ContentType = "text/plain", CurrentFormData = "currentValue", - PreviousFormData = "previousValue" + PreviousFormData = "previousValue", + CurrentBinaryData = default, + PreviousBinaryData = default }, - new DataElementChange() + new FormDataChange() { + Type = ChangeType.Updated, DataElement = dataElementNoValidation, + DataType = _instanceDataAccessor.GetDataType(dataElement), + ContentType = "text/plain", CurrentFormData = "currentValue", - PreviousFormData = "previousValue" + PreviousFormData = "previousValue", + CurrentBinaryData = null, + PreviousBinaryData = null } - }; + ] + ); var genericValidator = new GenericValidatorFake(defaultDataType, validatorIssues, hasRelevantChanges: true); _services.AddSingleton(genericValidator); var validationService = _serviceProvider.Value.GetRequiredService(); var issues = await validationService.ValidateIncrementalFormData( - _instance, _instanceDataAccessor, taskId, changes, diff --git a/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs b/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs index 750c64c24..8e08dc0db 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/ExpressionsExclusiveGatewayTests.cs @@ -271,11 +271,11 @@ public async Task FilterAsync_Expression_filters_based_on_datamodel_set_by_gatew var frontendSettings = Options.Create(new FrontEndSettings()); - var dataAccessor = new CachedInstanceDataAccessor( + var dataAccessor = new InstanceDataUnitOfWork( instance, _dataClient.Object, _instanceClient.Object, - _appMetadata.Object, + appMetadata, modelSerializationService ); diff --git a/test/Altinn.App.Core.Tests/Internal/Process/ProcessEngineTest.cs b/test/Altinn.App.Core.Tests/Internal/Process/ProcessEngineTest.cs index 26663f420..f1d41877d 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/ProcessEngineTest.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/ProcessEngineTest.cs @@ -409,6 +409,7 @@ public async Task Next_returns_unsuccessful_unauthorized_when_action_handler_ret EndEvent = "EndEvent_1" } }; + _appMetadataMock.Setup(x => x.GetApplicationMetadata()).ReturnsAsync(new ApplicationMetadata("org/app")); Mock userActionMock = new Mock(MockBehavior.Strict); userActionMock.Setup(u => u.Id).Returns("sign"); userActionMock diff --git a/test/Altinn.App.Core.Tests/Internal/Process/ProcessNavigatorTests.cs b/test/Altinn.App.Core.Tests/Internal/Process/ProcessNavigatorTests.cs index edebcc5e1..a25156976 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/ProcessNavigatorTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/ProcessNavigatorTests.cs @@ -7,6 +7,7 @@ using Altinn.App.Core.Internal.Process; using Altinn.App.Core.Internal.Process.Elements; using Altinn.App.Core.Internal.Process.Elements.Base; +using Altinn.App.Core.Models; using Altinn.App.Core.Models.Process; using Altinn.App.Core.Tests.Internal.Process.TestUtils; using Altinn.App.PlatformServices.Tests.Internal.Process.StubGatewayFilters; @@ -273,6 +274,7 @@ private ProcessNavigator SetupProcessNavigator( IEnumerable gatewayFilters ) { + _appMetadata.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(new ApplicationMetadata("org/app")); ProcessReader pr = ProcessTestUtils.SetupProcessReader(bpmnfile); return new ProcessNavigator( pr, diff --git a/test/Altinn.App.Core.Tests/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizerTests.cs b/test/Altinn.App.Core.Tests/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizerTests.cs index 337e85df8..a160ed4ad 100644 --- a/test/Altinn.App.Core.Tests/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizerTests.cs +++ b/test/Altinn.App.Core.Tests/Internal/Process/ProcessTasks/Common/ProcessTaskFinalizerTests.cs @@ -71,6 +71,6 @@ public async Task Finalize_WithValidInputs_ShouldCallCorrectMethods() // Assert // Called once in Finalize and once in CachedInstanceDataAccessor.UpdateInstanceData - _appMetadataMock.Verify(x => x.GetApplicationMetadata(), Times.Exactly(2)); + _appMetadataMock.Verify(x => x.GetApplicationMetadata(), Times.Once); } } diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs index ecf5290f2..500e8d47d 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs @@ -281,7 +281,7 @@ public async Task Test1() { _instance.Data[3], new SubFormModel(null, null, null, null, null) } }; - var issues = await validationService.ValidateInstanceAtTask(_instance, dataAccessor, TaskId, null, null, null); + var issues = await validationService.ValidateInstanceAtTask(dataAccessor, TaskId, null, null, null); _output.WriteLine(JsonSerializer.Serialize(issues, _options)); // Order of issues is not guaranteed, so we sort them before verification