diff --git a/src/Altinn.App.Api/Controllers/ActionsController.cs b/src/Altinn.App.Api/Controllers/ActionsController.cs index 583107314..2786ee59b 100644 --- a/src/Altinn.App.Api/Controllers/ActionsController.cs +++ b/src/Altinn.App.Api/Controllers/ActionsController.cs @@ -1,3 +1,4 @@ +using Altinn.App.Api.Extensions; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Models; using Altinn.App.Core.Extensions; @@ -172,7 +173,7 @@ await _appMetadata.GetApplicationMetadata(), ); } - if (dataMutator.AbandonIssues.Count > 0) + if (dataMutator.GetAbandonResponse() is not null) { throw new NotImplementedException( "return an error response from UserActions instead of abandoning the dataMutator" diff --git a/src/Altinn.App.Api/Controllers/DataController.cs b/src/Altinn.App.Api/Controllers/DataController.cs index 3d886b9e2..8108769d9 100644 --- a/src/Altinn.App.Api/Controllers/DataController.cs +++ b/src/Altinn.App.Api/Controllers/DataController.cs @@ -1,7 +1,9 @@ using System.Globalization; using System.Net; using System.Text.Json; +using Altinn.App.Api.Extensions; using Altinn.App.Api.Helpers; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Api.Helpers.RequestHandling; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Models; @@ -16,7 +18,6 @@ using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Instances; -using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Prefill; using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; @@ -52,7 +53,7 @@ public class DataController : ControllerBase private readonly IFileAnalysisService _fileAnalyserService; private readonly IFileValidationService _fileValidationService; private readonly IFeatureManager _featureManager; - private readonly IPatchService _patchService; + private readonly InternalPatchService _patchService; private readonly ModelSerializationService _modelDeserializer; private const long REQUEST_SIZE_LIMIT = 2000 * 1024 * 1024; @@ -85,7 +86,7 @@ public DataController( IFileValidationService fileValidationService, IAppMetadata appMetadata, IFeatureManager featureManager, - IPatchService patchService, + InternalPatchService patchService, ModelSerializationService modelDeserializer ) { @@ -366,14 +367,9 @@ await _patchService.RunDataProcessors( language ); - if (dataMutator.AbandonIssues.Count > 1) + if (dataMutator.GetAbandonResponse() is { } abandonResponse) { - return new DataPostErrorResponse( - "Data Processing abandoned", - dataMutator - .AbandonIssues.Select(i => ValidationIssueWithSource.FromIssue(i, "dataProcessing", true)) - .ToList() - ); + return abandonResponse; } var finalChanges = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); @@ -715,7 +711,7 @@ public async Task> PatchFormDataMultiple } } - ServiceResult res = await _patchService.ApplyPatches( + ServiceResult res = await _patchService.ApplyPatches( instance, dataPatchRequest.Patches.ToDictionary(i => i.DataElementId, i => i.Patch), language, @@ -800,16 +796,9 @@ await _appMetadata.GetApplicationMetadata(), var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: false); await _patchService.RunDataProcessors(dataMutator, changes, taskId, language); - if (dataMutator.AbandonIssues.Count > 1) + if (dataMutator.GetAbandonResponse() is { } abandonResponse) { - return BadRequest( - new DataPostErrorResponse( - "DataProcessing abandoned", - dataMutator - .AbandonIssues.Select(i => ValidationIssueWithSource.FromIssue(i, "dataProcessing", true)) - .ToList() - ) - ); + return Problem(abandonResponse); } // Get the updated changes for saving changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: false); @@ -1105,16 +1094,9 @@ await _patchService.RunDataProcessors( ); var jsonAfterDataProcessors = JsonSerializer.Serialize(serviceModel); - if (dataMutator.AbandonIssues.Count > 1) + if (dataMutator.GetAbandonResponse() is { } abandonResponse) { - return BadRequest( - new DataPostErrorResponse( - "DataProcessing abandoned", - dataMutator - .AbandonIssues.Select(i => ValidationIssueWithSource.FromIssue(i, "dataProcessing", true)) - .ToList() - ) - ); + return Problem(abandonResponse); } // Save changes @@ -1153,29 +1135,6 @@ private ActionResult HandlePlatformHttpException(PlatformHttpException e, string }; } - private ObjectResult Problem(DataPatchError error) - { - var code = error.ErrorType switch - { - DataPatchErrorType.PatchTestFailed => HttpStatusCode.Conflict, - DataPatchErrorType.DeserializationFailed => HttpStatusCode.UnprocessableContent, - DataPatchErrorType.AbandonedRequest => HttpStatusCode.BadRequest, - _ => HttpStatusCode.InternalServerError - }; - - return StatusCode( - (int)code, - new ProblemDetails() - { - Title = error.Title, - Detail = error.Detail, - Type = "https://datatracker.ietf.org/doc/html/rfc6902/", - Status = (int)code, - Extensions = error.Extensions ?? new Dictionary(StringComparer.Ordinal) - } - ); - } - private ObjectResult Problem(ProblemDetails error) { return StatusCode(error.Status ?? (int)HttpStatusCode.InternalServerError, error); diff --git a/src/Altinn.App.Api/Controllers/InstancesController.cs b/src/Altinn.App.Api/Controllers/InstancesController.cs index 2db841898..6adb17fd2 100644 --- a/src/Altinn.App.Api/Controllers/InstancesController.cs +++ b/src/Altinn.App.Api/Controllers/InstancesController.cs @@ -1,6 +1,8 @@ using System.Globalization; using System.Net; using System.Text.Json; +using Altinn.App.Api.Extensions; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Api.Helpers.RequestHandling; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Mappers; @@ -16,7 +18,6 @@ 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; @@ -69,7 +70,7 @@ public class InstancesController : ControllerBase private readonly IOrganizationClient _orgClient; private readonly IHostEnvironment _env; private readonly ModelSerializationService _serializationService; - private readonly IPatchService _patchService; + private readonly InternalPatchService _patchService; private static readonly JsonSerializerOptions _jsonSerializerOptionsWeb = new(JsonSerializerDefaults.Web); private const long RequestSizeLimit = 2000 * 1024 * 1024; @@ -94,7 +95,7 @@ public InstancesController( IOrganizationClient orgClient, IHostEnvironment env, ModelSerializationService serializationService, - IPatchService patchService + InternalPatchService patchService ) { _logger = logger; @@ -348,6 +349,11 @@ public async Task> Post( var prefillProblem = await StorePrefillParts(instance, application, requestParts, language); if (prefillProblem is not null) { + await _instanceClient.DeleteInstance( + int.Parse(instance.InstanceOwner.PartyId, CultureInfo.InvariantCulture), + Guid.Parse(instance.Id.Split("/")[1]), + hard: true + ); return StatusCode(prefillProblem.Status ?? 500, prefillProblem); } @@ -1193,23 +1199,12 @@ string action var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); await _patchService.RunDataProcessors(dataMutator, changes, taskId, language); - if (dataMutator.AbandonIssues.Count > 0) + if (dataMutator.GetAbandonResponse() is { } abandonResponse) { _logger.LogWarning( "Data processing failed for one or more data elements, the instance was created, but we try to delete the instance" ); - await _instanceClient.DeleteInstance( - int.Parse(instance.Id.Split("/")[0], CultureInfo.InvariantCulture), - Guid.Parse(instance.Id.Split("/")[1]), - hard: true - ); - return new ProblemDetails - { - Title = "Data processing failed", - Detail = - "Data processing failed for one or more data elements, the instance was created, but the data was ignored", - Extensions = { { "issues", dataMutator.AbandonIssues }, { "instance", instance } } - }; + return abandonResponse; } // Update the changes list if it changed in data processors diff --git a/src/Altinn.App.Api/Extensions/InstanceDataUnitOfWorkExtensions.cs b/src/Altinn.App.Api/Extensions/InstanceDataUnitOfWorkExtensions.cs new file mode 100644 index 000000000..c82906af2 --- /dev/null +++ b/src/Altinn.App.Api/Extensions/InstanceDataUnitOfWorkExtensions.cs @@ -0,0 +1,34 @@ +using Altinn.App.Core.Internal.Data; +using Altinn.App.Core.Models.Validation; +using Microsoft.AspNetCore.Mvc; + +namespace Altinn.App.Api.Extensions; + +internal static class InstanceDataUnitOfWorkExtensions +{ + internal class FileValidationIssueProblemDetails : ProblemDetails + { + public List? UploadValidationIssues { get; set; } + } + + public static ProblemDetails? GetAbandonResponse(this InstanceDataUnitOfWork instanceDataUnitOfWork) + { + if (instanceDataUnitOfWork.HasAbandonIssues) + { + var issues = instanceDataUnitOfWork + .AbandonIssues.Select(issue => + ValidationIssueWithSource.FromIssue(issue, "DataProcessorAbandon", noIncrementalUpdates: false) + ) + .ToList(); + return new FileValidationIssueProblemDetails + { + Title = "File validation failed", + Detail = "Validation issues were found in the file upload", + Status = 400, + UploadValidationIssues = issues, + }; + } + + return null; + } +} diff --git a/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs index a014ed7eb..77e581da3 100644 --- a/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Api/Extensions/ServiceCollectionExtensions.cs @@ -3,6 +3,7 @@ using Altinn.App.Api.Controllers.Attributes; using Altinn.App.Api.Controllers.Conventions; using Altinn.App.Api.Helpers; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Api.Infrastructure.Filters; using Altinn.App.Api.Infrastructure.Health; using Altinn.App.Api.Infrastructure.Middleware; @@ -101,6 +102,7 @@ IWebHostEnvironment env AddAntiforgery(services); services.AddSingleton(); + services.AddTransient(); services.Configure(options => { diff --git a/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs b/src/Altinn.App.Api/Helpers/Patch/DataPatchResult.cs similarity index 61% rename from src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs rename to src/Altinn.App.Api/Helpers/Patch/DataPatchResult.cs index b55ece908..99a7178ae 100644 --- a/src/Altinn.App.Core/Internal/Patch/DataPatchResult.cs +++ b/src/Altinn.App.Api/Helpers/Patch/DataPatchResult.cs @@ -2,17 +2,17 @@ using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; -namespace Altinn.App.Core.Internal.Patch; +namespace Altinn.App.Api.Helpers.Patch; /// -/// Result of a data patch operation in the . +/// Result of a data patch operation in the . /// public class DataPatchResult { /// /// The updated instance after the patch and dataProcessing operations. /// - public required Instance Instance { get; set; } + public required Instance Instance { get; init; } /// /// The validation issues that were found during the patch operation. @@ -23,11 +23,4 @@ public class DataPatchResult /// Get updated data elements that have app logic in a dictionary with the data element id as key. /// public required DataElementChanges FormDataChanges { get; init; } - - /// - /// Store a pair with Id and Data - /// - /// The data element id - /// The deserialized data - public record DataModelPair(DataElementIdentifier Identifier, object Data); } diff --git a/src/Altinn.App.Core/Internal/Patch/PatchService.cs b/src/Altinn.App.Api/Helpers/Patch/InternalPatchService.cs similarity index 88% rename from src/Altinn.App.Core/Internal/Patch/PatchService.cs rename to src/Altinn.App.Api/Helpers/Patch/InternalPatchService.cs index 4b637690c..32f361710 100644 --- a/src/Altinn.App.Core/Internal/Patch/PatchService.cs +++ b/src/Altinn.App.Api/Helpers/Patch/InternalPatchService.cs @@ -1,6 +1,8 @@ +using System.Net; using System.Text.Json; using System.Text.Json.Nodes; using System.Text.Json.Serialization; +using Altinn.App.Api.Extensions; using Altinn.App.Core.Features; using Altinn.App.Core.Helpers.Serialization; using Altinn.App.Core.Internal.App; @@ -12,15 +14,14 @@ using Altinn.App.Core.Models.Validation; using Altinn.Platform.Storage.Interface.Models; using Json.Patch; -using Microsoft.AspNetCore.Hosting; -using Microsoft.Extensions.Hosting; +using Microsoft.AspNetCore.Mvc; -namespace Altinn.App.Core.Internal.Patch; +namespace Altinn.App.Api.Helpers.Patch; /// /// Service for applying patches to form data elements /// -internal class PatchService : IPatchService +public class InternalPatchService { private readonly IAppMetadata _appMetadata; private readonly IDataClient _dataClient; @@ -36,9 +37,9 @@ internal class PatchService : IPatchService new() { UnmappedMemberHandling = JsonUnmappedMemberHandling.Disallow, PropertyNameCaseInsensitive = true, }; /// - /// Creates a new instance of the class + /// Creates a new instance of the class /// - public PatchService( + public InternalPatchService( IAppMetadata appMetadata, IDataClient dataClient, IInstanceClient instanceClient, @@ -61,8 +62,10 @@ public PatchService( _telemetry = telemetry; } - /// - public async Task> ApplyPatches( + /// + /// Applies a patch to a Form Data element + /// + public async Task> ApplyPatches( Instance instance, Dictionary patches, string? language, @@ -87,10 +90,11 @@ await _appMetadata.GetApplicationMetadata(), if (dataElement is null) { - return new DataPatchError() + return new ProblemDetails() { Title = "Unknown data element to patch", Detail = $"Data element with id {dataElementGuid} not found in instance", + Status = (int)HttpStatusCode.NotFound, }; } @@ -103,13 +107,14 @@ await _appMetadata.GetApplicationMetadata(), if (!patchResult.IsSuccess) { bool testOperationFailed = patchResult.Error.Contains("is not equal to the indicated value."); - return new DataPatchError() + return new ProblemDetails() { Title = testOperationFailed ? "Precondition in patch failed" : "Patch Operation Failed", Detail = patchResult.Error, - ErrorType = testOperationFailed - ? DataPatchErrorType.PatchTestFailed - : DataPatchErrorType.DeserializationFailed, + Type = "https://datatracker.ietf.org/doc/html/rfc6902/", + Status = testOperationFailed + ? (int)HttpStatusCode.Conflict + : (int)HttpStatusCode.UnprocessableContent, Extensions = new Dictionary() { { "previousModel", oldModel }, @@ -121,11 +126,12 @@ await _appMetadata.GetApplicationMetadata(), var newModelResult = DeserializeModel(oldModel.GetType(), patchResult.Result); if (!newModelResult.Success) { - return new DataPatchError() + return new ProblemDetails() { Title = "Patch operation did not deserialize", + Type = "https://datatracker.ietf.org/doc/html/rfc6902/", Detail = newModelResult.Error, - ErrorType = DataPatchErrorType.DeserializationFailed + Status = (int)HttpStatusCode.UnprocessableContent }; } @@ -155,18 +161,9 @@ await RunDataProcessors( language ); - if (dataAccessor.AbandonIssues.Count > 0) + if (dataAccessor.GetAbandonResponse() is { } abandonResponse) { - return new DataPatchError() - { - Title = "Data processors abandoned the patch", - Detail = "Data processors abandoned the patch", - ErrorType = DataPatchErrorType.AbandonedRequest, - Extensions = new Dictionary() - { - { "uploadValidationIssues", dataAccessor.AbandonIssues }, - } // use same key as in DataPostErrorResponse - }; + return abandonResponse; } // Get all changes to data elements by comparing the serialized values @@ -232,6 +229,9 @@ await RunDataProcessors( }; } + /// + /// Runs incremental validation on the changes. + /// public async Task> RunIncrementalValidation( IInstanceDataAccessor dataAccessor, string taskId, @@ -249,6 +249,9 @@ public async Task> RunIncrementalValidation( ); } + /// + /// Runs and on the changes. + /// public async Task RunDataProcessors( IInstanceDataMutator dataMutator, DataElementChanges changes, diff --git a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs index 1f0549751..5edb3fe70 100644 --- a/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs +++ b/src/Altinn.App.Core/Extensions/ServiceCollectionExtensions.cs @@ -34,7 +34,6 @@ using Altinn.App.Core.Internal.Expressions; using Altinn.App.Core.Internal.Instances; using Altinn.App.Core.Internal.Language; -using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Pdf; using Altinn.App.Core.Internal.Prefill; using Altinn.App.Core.Internal.Process; @@ -173,7 +172,6 @@ IWebHostEnvironment env services.TryAddTransient(); services.TryAddTransient(); services.TryAddTransient(); - services.TryAddTransient(); services.AddTransient(); services.Configure(configuration.GetSection("PEPSettings")); services.Configure(configuration.GetSection("PlatformSettings")); diff --git a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs index de9add775..b27164a6e 100644 --- a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs @@ -18,6 +18,17 @@ public interface IInstanceDataAccessor /// IEnumerable DataElements => Instance.Data; + /// + /// Data elements of the instance that has appLogic (form data elements). + /// + IEnumerable FormDataElements => + Instance.Data.Where(d => GetDataType(d).AppLogic?.ClassRef is not null); + + /// + /// Data elements of the instance that represents attachments/binary data (no appLogic). + /// + IEnumerable BinaryDataElements => Instance.Data.Where(d => GetDataType(d).AppLogic?.ClassRef is null); + /// /// Get the actual data represented in the data element. /// diff --git a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs index 09d99edee..36744dacf 100644 --- a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs +++ b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs @@ -261,6 +261,8 @@ public void RemoveDataElement(DataElementIdentifier dataElementIdentifier) internal List AbandonIssues { get; } = []; + public bool HasAbandonIssues => AbandonIssues.Count > 0; + public void AbandonAllChanges(IEnumerable validationIssues) { AbandonIssues.AddRange(validationIssues); @@ -272,7 +274,7 @@ public void AbandonAllChanges(IEnumerable validationIssues) public DataElementChanges GetDataElementChanges(bool initializeAltinnRowId) { - if (AbandonIssues.Count > 0) + if (HasAbandonIssues) { throw new InvalidOperationException("AbandonAllChanges has been called, and no changes should be saved"); } @@ -396,7 +398,7 @@ ReadOnlyMemory bytes internal async Task UpdateInstanceData(DataElementChanges changes) { - if (AbandonIssues.Count > 0) + if (HasAbandonIssues) { throw new InvalidOperationException("AbandonAllChanges has been called, and no changes should be saved"); } @@ -469,7 +471,7 @@ await UpdatePresentationTextsOnInstance( internal async Task SaveChanges(DataElementChanges changes) { - if (AbandonIssues.Count > 0) + if (HasAbandonIssues) { throw new InvalidOperationException("AbandonAllChanges has been called, and no changes should be saved"); } diff --git a/src/Altinn.App.Core/Internal/Patch/DataPatchError.cs b/src/Altinn.App.Core/Internal/Patch/DataPatchError.cs deleted file mode 100644 index 1f173b39f..000000000 --- a/src/Altinn.App.Core/Internal/Patch/DataPatchError.cs +++ /dev/null @@ -1,50 +0,0 @@ -using Altinn.App.Core.Features; - -namespace Altinn.App.Core.Internal.Patch; - -/// -/// Error that can be returned from the when a patch operation fails. -/// -public class DataPatchError -{ - /// - /// The title of the error. - /// - public string? Title { get; set; } - - /// - /// A detailed description of the error. - /// - public string? Detail { get; set; } - - /// - /// The type of error that occurred. - /// - public DataPatchErrorType? ErrorType { get; set; } - - /// - /// Additional information about the error. - /// - public IDictionary? Extensions { get; set; } -} - -/// -/// The type of error that occurred during a data patch operation. -/// -public enum DataPatchErrorType -{ - /// - /// One or more of the JsonPatch tests failed. - /// - PatchTestFailed, - - /// - /// The patch operation lead to an invalid data model. - /// - DeserializationFailed, - - /// - /// The request was abandoned using - /// - AbandonedRequest, -} diff --git a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs b/src/Altinn.App.Core/Internal/Patch/IPatchService.cs deleted file mode 100644 index 1ca44a60f..000000000 --- a/src/Altinn.App.Core/Internal/Patch/IPatchService.cs +++ /dev/null @@ -1,45 +0,0 @@ -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; - -namespace Altinn.App.Core.Internal.Patch; - -/// -/// Service for handling JsonPatches to data elements. -/// -public interface IPatchService -{ - /// - /// Applies a patch to a Form Data element - /// - Task> ApplyPatches( - Instance instance, - Dictionary patches, - string? language, - List? ignoredValidators - ); - - /// - /// Runs and on the changes. - /// - Task RunDataProcessors( - IInstanceDataMutator dataMutator, - 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/Process/ProcessEngine.cs b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs index 78dfc0586..79cd519c2 100644 --- a/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs +++ b/src/Altinn.App.Core/Internal/Process/ProcessEngine.cs @@ -197,7 +197,7 @@ await _appMetadata.GetApplicationMetadata(), return result; } - if (cachedDataMutator.AbandonIssues.Count > 0) + if (cachedDataMutator.HasAbandonIssues) { throw new Exception( "Abandon issues found in data elements. Abandon issues should be handled by the action handler." diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_PostTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_PostTests.cs new file mode 100644 index 000000000..22de25a8d --- /dev/null +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_PostTests.cs @@ -0,0 +1,327 @@ +using System.Net; +using System.Net.Http.Headers; +using System.Net.Http.Json; +using System.Text.Json; +using Altinn.App.Api.Models; +using Altinn.App.Api.Tests.Data; +using Altinn.App.Api.Tests.Data.apps.tdd.contributer_restriction.models; +using Altinn.App.Core.Features; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using App.IntegrationTests.Mocks.Services; +using FluentAssertions; +using Microsoft.AspNetCore.Mvc; +using Microsoft.AspNetCore.Mvc.Testing; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Xunit.Abstractions; + +namespace Altinn.App.Api.Tests.Controllers; + +public class DataController_PostTests : ApiTestBase, IClassFixture> +{ + private static readonly string _org = "tdd"; + private static readonly string _app = "contributer-restriction"; + private static readonly int _instanceOwnerPartyId = 500600; + private static readonly Guid _instanceGuid = Guid.Parse("09e16a2d-e009-4f3a-940b-da1ea54a18b4"); + private readonly string _formElementId = "c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6"; + + private readonly Mock _dataProcessor = new(MockBehavior.Strict); + private readonly Mock _dataWriteProcessor = new(MockBehavior.Strict); + + public DataController_PostTests(WebApplicationFactory factory, ITestOutputHelper outputHelper) + : base(factory, outputHelper) + { + TestData.DeleteInstanceAndData(_org, _app, _instanceOwnerPartyId, _instanceGuid); + TestData.PrepareInstance(_org, _app, _instanceOwnerPartyId, _instanceGuid); + OverrideServicesForAllTests = (services) => + { + services.AddSingleton(_dataProcessor.Object); + services.AddSingleton(_dataWriteProcessor.Object); + }; + } + + [Fact] + public async Task PostFormElement_DataProcessorsModifiesOtherElement_ReturnsChanges() + { + string dataTypeString = "default"; + + // Increase max count to allow for adding a new element + OverrideServicesForThisTest = (services) => + { + services.AddSingleton( + new AppMetadataMutationHook(appMetadata => + { + var dataType = appMetadata.DataTypes.Should().ContainSingle(d => d.Id == dataTypeString).Which; + dataType.MaxCount = 2; + }) + ); + }; + + // DataWriteProcessor is not called on POST (for historical reasons) + ProcessDataWrite((instance, data, previousData, language) => Task.CompletedTask, Times.Never()); + + // Update the added form data in IDataProcessor + RegisterProcessDataRead((instance, dataId, data, language) => Task.CompletedTask, Times.Never()); + + // Update the original form data in IDataWriteProcessor + RegisterProcessWriterDataWrite( + "Task_1", + async (instanceDataMutator, changes, language) => + { + var originalDataElement = instanceDataMutator + .DataElements.Should() + .ContainSingle(d => d.Id == _formElementId) + .Which; + var postedSkjema = changes + .FormDataChanges.Should() + .ContainSingle() + .Which.CurrentFormData.Should() + .BeOfType() + .Which; + postedSkjema.Melding!.Random.Should().Be("fromClient"); + postedSkjema.Melding.Name = "FromDataProcessor"; + var originalData = await instanceDataMutator.GetFormData(originalDataElement); + var skjema = originalData.Should().BeOfType().Which; + skjema.Melding.Should().BeNull(); + skjema.Melding = new() { Name = "FromDataWriteProcessor" }; + }, + Times.Once() + ); + + HttpClient client = GetRootedClient(_org, _app, 1337, null, authenticationLevel: 2); + var content = JsonContent.Create(new Skjema() { Melding = new() { Random = "fromClient" } }); + var response = await client.PostAsync( + $"/{_org}/{_app}/instances/{_instanceOwnerPartyId}/{_instanceGuid}/data/{dataTypeString}", + content + ); + var parsedResponse = await VerifyStatusAndDeserialize(response, HttpStatusCode.OK); + parsedResponse.NewDataModels.Should().HaveCount(2); + var original = parsedResponse + .NewDataModels.Should() + .ContainSingle(d => d.DataElementId.ToString() == _formElementId) + .Which.Data.Should() + .BeOfType() + .Which.Deserialize()!; + original.Melding!.Name.Should().Be("FromDataWriteProcessor"); + var posted = parsedResponse + .NewDataModels.Should() + .ContainSingle(d => d.DataElementId.ToString() != _formElementId) + .Which.Data.Should() + .BeOfType() + .Which.Deserialize()!; + posted.Melding?.Name.Should().Be("FromDataProcessor"); + posted.Melding!.Random.Should().Be("fromClient"); + _dataProcessor.Verify(); + _dataWriteProcessor.Verify(); + } + + [Fact] + public async Task PostBinaryElement_DataProcessorAbandons_ReturnsIssues() + { + // Setup test data + var binaryData = new byte[] { 1, 2, 3, 4, 5 }; + var binaryDataType = "specificFileType"; + + ProcessDataWrite((instance, data, previousData, language) => Task.CompletedTask, Times.Never()); + RegisterProcessDataRead((instance, dataId, data, language) => Task.CompletedTask, Times.Never()); + RegisterProcessWriterDataWrite( + "Task_1", + (instanceDataMutator, changes, language) => + { + language.Should().BeNull(); // No language is set + var data = changes.BinaryDataChanges.Should().ContainSingle().Which; + data.CurrentBinaryData.ToArray().Should().BeEquivalentTo(binaryData); + instanceDataMutator.AbandonAllChanges( + [ + new() + { + DataElementId = data.DataElement?.Id, + Severity = ValidationIssueSeverity.Error, + Code = "ABANDONED", + Description = "BinaryData is incorrect", + } + ] + ); + return Task.CompletedTask; + }, + Times.Exactly(1) + ); + + HttpClient client = GetRootedClient(_org, _app, 1337, null, authenticationLevel: 2); + + // Update data element + var updateDataElementContent = new ByteArrayContent(binaryData) + { + Headers = + { + ContentType = new MediaTypeHeaderValue("application/pdf"), + ContentDisposition = new ContentDispositionHeaderValue("attachment") { FileName = "test.pdf" } + } + }; + + var response = await client.PostAsync( + $"/{_org}/{_app}/instances/{_instanceOwnerPartyId}/{_instanceGuid}/data/{binaryDataType}", + updateDataElementContent + ); + + var responseParsed = await VerifyStatusAndDeserialize(response, HttpStatusCode.BadRequest); + + responseParsed.Should().NotBeNull(); + + var issue = responseParsed + .Extensions.Should() + .ContainKey("uploadValidationIssues") + .WhoseValue.Should() + .BeOfType() + .Which.Deserialize>() + .Should() + .ContainSingle() + .Which; + issue.Code.Should().Be("ABANDONED"); + issue.Description.Should().Be("BinaryData is incorrect"); + + // Verify stored data + var readInstanceResponse = await client.GetAsync( + $"/{_org}/{_app}/instances/{_instanceOwnerPartyId}/{_instanceGuid}" + ); + var endInstance = await VerifyStatusAndDeserialize(readInstanceResponse, HttpStatusCode.OK); + endInstance.Data.Should().HaveCount(1); + + _dataProcessor.Verify(); + _dataWriteProcessor.Verify(); + } + + [Fact] + public async Task PostBinaryElement_DataWriteProcessorAddsAndRemovesElements() + { + // Setup test data + var binaryData = new byte[] { 1, 2, 3, 4, 5 }; + var binaryDataType = "specificFileType"; + + ProcessDataWrite((instance, data, previousData, language) => Task.CompletedTask, Times.Never()); + RegisterProcessDataRead((instance, dataId, data, language) => Task.CompletedTask, Times.Never()); + RegisterProcessWriterDataWrite( + "Task_1", + async (instanceDataMutator, changes, language) => + { + var data = changes.BinaryDataChanges.Should().ContainSingle().Which; + data.CurrentBinaryData.ToArray().Should().BeEquivalentTo(binaryData); + instanceDataMutator.AddBinaryDataElement("specificFileType", "application/pdf", "test.pdf", binaryData); + var toDelete = instanceDataMutator.DataElements.Should().ContainSingle().Which; + toDelete.DataType.Should().Be("default"); + instanceDataMutator.RemoveDataElement(toDelete); + await Task.CompletedTask; + }, + Times.Exactly(1) + ); + + HttpClient client = GetRootedClient(_org, _app, 1337, null, authenticationLevel: 2); + + // Update data element + var updateDataElementContent = new ByteArrayContent(binaryData) + { + Headers = + { + ContentType = new MediaTypeHeaderValue("application/pdf"), + ContentDisposition = new ContentDispositionHeaderValue("attachment") { FileName = "test.pdf" } + } + }; + + var response = await client.PostAsync( + $"/{_org}/{_app}/instances/{_instanceOwnerPartyId}/{_instanceGuid}/data/{binaryDataType}", + updateDataElementContent + ); + var responseAsString = await response.Content.ReadAsStringAsync(); + OutputHelper.WriteLine(responseAsString); + response.Should().HaveStatusCode(HttpStatusCode.OK); + + var responseParsed = await VerifyStatusAndDeserialize(response, HttpStatusCode.OK); + + responseParsed.Should().NotBeNull(); + + responseParsed + .Instance.Data.Should() + .HaveCount(2) + .And.AllSatisfy(d => d.DataType.Should().Be("specificFileType")); + + var dataGuid = responseParsed.NewDataElementId.Should().NotBeEmpty().And.Subject; + + // Verify stored data + var readDataElementResponse = await client.GetAsync( + $"/{_org}/{_app}/instances/{_instanceOwnerPartyId}/{_instanceGuid}/data/{dataGuid}" + ); + readDataElementResponse.Should().HaveStatusCode(HttpStatusCode.OK); + var returnedBinary = await readDataElementResponse.Content.ReadAsByteArrayAsync(); + returnedBinary.Should().BeEquivalentTo(binaryData); + + _dataProcessor.Verify(); + _dataWriteProcessor.Verify(); + } + + private delegate Task ProcessDataWriterWriteDelegate( + IInstanceDataMutator instanceDataMutator, + DataElementChanges changes, + string? language + ); + + private void RegisterProcessWriterDataWrite(string taskId, ProcessDataWriterWriteDelegate write, Times times) + { + _dataWriteProcessor + .Setup(p => + p.ProcessDataWrite( + It.IsAny(), + taskId, + It.IsAny(), + It.IsAny() + ) + ) + .Returns( + (IInstanceDataMutator instanceDataMutator, string _, DataElementChanges changes, string? language) => + write(instanceDataMutator, changes, language) + ) + .Verifiable(times); + } + + private delegate Task ProcessDataReadDelegate(Instance instance, Guid dataId, object data, string? language); + + private void RegisterProcessDataRead(ProcessDataReadDelegate read, Times times) + { + _dataProcessor + .Setup(p => + p.ProcessDataRead(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()) + ) + .Returns( + (Instance instance, Guid dataId, object data, string? language) => + read(instance, dataId, data, language) + ) + .Verifiable(times); + } + + private delegate Task ProcessDataWriteDelegate( + Instance instance, + object data, + object previousData, + string? language + ); + + private void ProcessDataWrite(ProcessDataWriteDelegate write, Times times) + { + _dataProcessor + .Setup(p => + p.ProcessDataWrite( + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny(), + It.IsAny() + ) + ) + .Returns( + (Instance instance, Guid dataGuid, object data, object previousData, string? language) => + write(instance, data, previousData, language) + ) + .Verifiable(times); + } +} diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs index 4fa6e89d9..1a2eafe8f 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_ActiveInstancesTests.cs @@ -1,4 +1,5 @@ using Altinn.App.Api.Controllers; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Api.Models; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; @@ -8,7 +9,6 @@ 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; @@ -70,7 +70,7 @@ public InstancesController_ActiveInstancesTest() _oarganizationClientMock.Object, _envMock.Object, _modelSerializationService, - new PatchService( + new InternalPatchService( _appMetadata.Object, _data.Object, _instanceClient.Object, diff --git a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs index 78d558b1f..224f0cbf3 100644 --- a/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/InstancesController_CopyInstanceTests.cs @@ -1,4 +1,5 @@ using Altinn.App.Api.Controllers; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Api.Tests.Utils; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; @@ -9,7 +10,6 @@ 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; @@ -58,7 +58,7 @@ public InstancesController_CopyInstanceTests() ControllerContext controllerContext = new ControllerContext() { HttpContext = _httpContextMock.Object }; var modelSerializationService = new ModelSerializationService(_appModel.Object); - var patchService = new PatchService( + var patchService = new InternalPatchService( _appMetadata.Object, _data.Object, _instanceClient.Object, diff --git a/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4.pretest.json b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4.pretest.json new file mode 100644 index 000000000..4dbdc40aa --- /dev/null +++ b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4.pretest.json @@ -0,0 +1 @@ +{"id":"500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4","instanceOwner":{"partyId":"500600","organisationNumber":"897069631"},"appId":"tdd/contributer-restriction","org":"tdd","process":{"started":"2024-10-31T10:43:24.209802Z","startEvent":"StartEvent_1","currentTask":{"flow":2,"started":"2024-10-31T10:43:24.222631Z","elementId":"Task_1","name":"Utfylling","altinnTaskType":"data","flowType":"CompleteCurrentMoveToNext"}},"data":[],"lastChanged":"2024-10-31T10:43:24.325907Z"} \ No newline at end of file diff --git a/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/blob/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/blob/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest new file mode 100644 index 000000000..5b5674fe3 --- /dev/null +++ b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/blob/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest @@ -0,0 +1 @@ + \ No newline at end of file diff --git a/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest.json b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest.json new file mode 100644 index 000000000..b007dfe6f --- /dev/null +++ b/test/Altinn.App.Api.Tests/Data/Instances/tdd/contributer-restriction/500600/09e16a2d-e009-4f3a-940b-da1ea54a18b4/c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6.pretest.json @@ -0,0 +1,24 @@ +{ + "id": "c52f40dd-11d1-4e24-b13b-4fcfdf6ca1c6", + "instanceGuid": "09e16a2d-e009-4f3a-940b-da1ea54a18b4", + "dataType": "default", + "filename": null, + "contentType": "application/xml", + "blobStoragePath": null, + "selfLinks": null, + "size": 0, + "contentHash": null, + "locked": false, + "refs": null, + "isRead": true, + "tags": [], + "userDefinedMetadata": null, + "metadata": null, + "deleteStatus": null, + "fileScanResult": "NotApplicable", + "references": null, + "created": null, + "createdBy": null, + "lastChanged": null, + "lastChangedBy": null +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.Test_Ok.verified.txt similarity index 100% rename from test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.Test_Ok.verified.txt rename to test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.Test_Ok.verified.txt diff --git a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs similarity index 97% rename from test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs rename to test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs index 36a3fb371..86b806612 100644 --- a/test/Altinn.App.Core.Tests/Internal/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs @@ -1,4 +1,6 @@ using System.ComponentModel.DataAnnotations; +using System.Net; +using Altinn.App.Api.Helpers.Patch; using Altinn.App.Common.Tests; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; @@ -7,7 +9,6 @@ using Altinn.App.Core.Internal.AppModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Instances; -using Altinn.App.Core.Internal.Patch; using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; using Altinn.App.Core.Models.Validation; @@ -16,7 +17,6 @@ using Json.Patch; using Json.Pointer; using Microsoft.AspNetCore.Hosting; -using Microsoft.AspNetCore.Http; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -55,7 +55,7 @@ public sealed class PatchServiceTests : IDisposable private readonly Mock _dataElementValidator = new(MockBehavior.Strict); // System under test - private readonly PatchService _patchService; + private readonly InternalPatchService _patchService; private readonly ModelSerializationService _modelSerializationService; public PatchServiceTests() @@ -100,7 +100,7 @@ public PatchServiceTests() _modelSerializationService = new ModelSerializationService(_appModelMock.Object); - _patchService = new PatchService( + _patchService = new InternalPatchService( _appMetadataMock.Object, _dataClientMock.Object, _instanceClientMock.Object, @@ -234,7 +234,7 @@ public async Task Test_JsonPatchTest_fail() err.Should().NotBeNull(); err!.Title.Should().Be("Precondition in patch failed"); err.Detail.Should().Be("Path `/Name` is not equal to the indicated value."); - err.ErrorType.Should().Be(DataPatchErrorType.PatchTestFailed); + err.Status.Should().Be((int)HttpStatusCode.Conflict); err.Extensions.Should().ContainKey("previousModel"); err.Extensions.Should().ContainKey("patchOperationIndex"); } @@ -286,7 +286,7 @@ public async Task Test_JsonPatch_does_not_deserialize() .Be( "The JSON property 'Age' could not be mapped to any .NET member contained in type 'Altinn.App.Core.Tests.Internal.Patch.PatchServiceTests+MyModel'." ); - err.ErrorType.Should().Be(DataPatchErrorType.DeserializationFailed); + err.Status.Should().Be((int)HttpStatusCode.UnprocessableEntity); } private void SetupDataClient(MyModel oldModel)