From 4634c80d3c5d7142bffd84829b0a145ec7112684 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 25 Aug 2025 17:04:24 +0200 Subject: [PATCH 1/6] Let validators opt in to run on clean data Introduce CleanInstanceDataAccessor and PreviousInstanceDataAccessor that we use to clean the changes for HasRelevantChanges and that can be used for IDataWriteProcessor tasks. Also add missing functionality to set NoIncrementalValidation on IFormDataValidator and a virtual override in GenericFormDataValidator --- .../Features/IFormDataValidator.cs | 5 + .../Features/IInstanceDataAccessor.cs | 12 + src/Altinn.App.Core/Features/IValidator.cs | 9 + .../Validation/GenericFormDataValidator.cs | 3 + .../Wrappers/FormDataValidatorWrapper.cs | 3 + .../Data/CleanInstanceDataAccessor.cs | 139 ++++++++ .../Internal/Data/InstanceDataUnitOfWork.cs | 50 ++- .../Internal/Data/PreviousDataAccessor.cs | 92 +++++ .../LayoutEvaluatorStateInitializer.cs | 11 + .../Internal/Validation/ValidationService.cs | 73 +++- .../Controllers/DataController_PatchTests.cs | 2 + .../Controllers/ProcessControllerTests.cs | 7 + ...alidateController_ValidateInstanceTests.cs | 2 + .../Helpers/Patch/PatchServiceTests.cs | 2 + .../Validators/GenericValidatorTests.cs | 2 + .../ValidationServiceTests.cs | 3 + .../Validators/ValidationServiceTests.cs | 7 + .../TestCleanDataAccessor.cs | 315 ++++++++++++++++++ .../CleanDataAccessor/mainLayout/page1.json | 50 +++ .../CleanDataAccessor/subLayout/subPage.json | 36 ++ .../FullTests/DataAccessorFixture.cs | 190 +++++++++++ ...idateAllRequiredFieldsMissing.verified.txt | 136 ++++++++ ...ateEmpty_IssuesRequiredIssues.verified.txt | 50 +++ ...redValidatorTests.VerifyAllOk.verified.txt | 10 + .../RequiredValidatorTests.cs | 230 +++++++++++++ .../RequiredValidator/mainLayout/page1.json | 205 ++++++++++++ .../RequiredValidator/subLayout/subPage.json | 147 ++++++++ .../TestUtilities/InstanceDataAccessorFake.cs | 10 + ...ouldNotChange_Unintentionally.verified.txt | 5 + 29 files changed, 1791 insertions(+), 15 deletions(-) create mode 100644 src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs create mode 100644 src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/mainLayout/page1.json create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/subLayout/subPage.json create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateAllRequiredFieldsMissing.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateEmpty_IssuesRequiredIssues.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.VerifyAllOk.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/subLayout/subPage.json diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs index c9221afac8..bf7d434d86 100644 --- a/src/Altinn.App.Core/Features/IFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -37,6 +37,11 @@ public interface IFormDataValidator /// bool NoIncrementalValidation => false; + /// + /// Proxy for + /// + bool ShouldRunAfterRemovingHiddenData => false; + /// /// The actual validation function /// diff --git a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs index 37435d5798..8534151073 100644 --- a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs @@ -1,3 +1,4 @@ +using Altinn.App.Core.Helpers; using Altinn.App.Core.Models; using Altinn.Platform.Storage.Interface.Models; @@ -32,6 +33,17 @@ public interface IInstanceDataAccessor /// when identifier does not exist in instance.Data with an applogic data type Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier); + /// + /// Get a that provides access to the cleaned data (where all fields marked as "hidden" is removed). + /// + /// The strategy for "hiddenRow" on group components + IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull); + + /// + /// Get a that provides access to the data before the current change. + /// + IInstanceDataAccessor GetPreviousDataAccessor(); + /// /// Gets the raw binary data from a DataElement. /// diff --git a/src/Altinn.App.Core/Features/IValidator.cs b/src/Altinn.App.Core/Features/IValidator.cs index a5b93955ab..31b27c2603 100644 --- a/src/Altinn.App.Core/Features/IValidator.cs +++ b/src/Altinn.App.Core/Features/IValidator.cs @@ -39,6 +39,15 @@ public interface IValidator /// bool NoIncrementalValidation => false; + /// + /// Indicates whether the validator should execute after removing data elements marked as hidden. + /// + /// + /// The default implementation returns false. Override this property in implementations if validation + /// needs to execute after hidden data removal. + /// + bool ShouldRunAfterRemovingHiddenData => false; + /// /// Run this validator and return all the issues this validator is aware of. /// diff --git a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs index a420be5b94..7a5fca5462 100644 --- a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs @@ -32,6 +32,9 @@ protected GenericFormDataValidator(string dataType) /// public virtual bool NoIncrementalValidation => false; + /// + public virtual bool ShouldRunAfterRemovingHiddenData => false; + // ReSharper disable once StaticMemberInGenericType private static readonly AsyncLocal> _validationIssues = new(); diff --git a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs index 0f0825fe9e..24dd0d53bd 100644 --- a/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs +++ b/src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs @@ -35,6 +35,9 @@ IDataElementAccessChecker dataElementAccessChecker /// public bool NoIncrementalValidation => _formDataValidator.NoIncrementalValidation; + /// + public bool ShouldRunAfterRemovingHiddenData => _formDataValidator.ShouldRunAfterRemovingHiddenData; + /// /// Run all legacy instances for the given . /// diff --git a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs new file mode 100644 index 0000000000..4d8db46703 --- /dev/null +++ b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs @@ -0,0 +1,139 @@ +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.Expressions; +using Altinn.App.Core.Internal.Texts; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Layout; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Internal.Data; + +internal class CleanInstanceDataAccessor : IInstanceDataAccessor +{ + private readonly IInstanceDataAccessor _dataAccessor; + private readonly string? _taskId; + private readonly IAppResources _appResources; + private readonly FrontEndSettings _frontEndSettings; + private readonly RowRemovalOption _rowRemovalOption; + private readonly string? _language; + private readonly ITranslationService _translationService; + private readonly Telemetry? _telemetry; + + public CleanInstanceDataAccessor( + IInstanceDataAccessor dataAccessor, + string? taskId, + IAppResources appResources, + ITranslationService translationService, + FrontEndSettings frontEndSettings, + RowRemovalOption rowRemovalOption, + string? language, + Telemetry? telemetry + ) + { + _dataAccessor = dataAccessor; + _taskId = taskId; + _appResources = appResources; + _frontEndSettings = frontEndSettings; + _rowRemovalOption = rowRemovalOption; + _language = language; + _telemetry = telemetry; + + LayoutModel? layouts = taskId is not null ? appResources.GetLayoutModelForTask(taskId) : null; + var state = new LayoutEvaluatorState( + dataAccessor, + layouts, + translationService, + frontEndSettings, + gatewayAction: null, + language + ); + _hiddenFieldsTask = new(() => + { + using var activity = telemetry?.StartRemoveHiddenDataForValidation(); + return LayoutEvaluator.GetHiddenFieldsForRemoval(state); + }); + _translationService = translationService; + } + + private readonly DataElementCache _cleanCache = new(); + + private readonly Lazy>> _hiddenFieldsTask; + + public Instance Instance => _dataAccessor.Instance; + + public IReadOnlyCollection DataTypes => _dataAccessor.DataTypes; + + public async Task GetFormData(DataElementIdentifier dataElementIdentifier) + { + return (await GetFormDataWrapper(dataElementIdentifier)).BackingData(); + } + + public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) + { + return await _cleanCache.GetOrCreate( + dataElementIdentifier, + async () => + { + var data = await _dataAccessor.GetFormDataWrapper(dataElementIdentifier); + var hiddenFields = await _hiddenFieldsTask.Value; + return CleanModel(data.Copy(), dataElementIdentifier, hiddenFields, _rowRemovalOption); + } + ); + } + + private static IFormDataWrapper CleanModel( + IFormDataWrapper data, + DataElementIdentifier dataElementIdentifier, + List hiddenFields, + RowRemovalOption rowRemovalOption + ) + { + foreach (var dataReference in hiddenFields) + { + if (dataReference.DataElementIdentifier != dataElementIdentifier) + { + continue; + } + + // Note that the paths for lists is in reverse order from GetHiddenFieldsForRemoval, so we can remove them here in order + data.RemoveField(dataReference.Field, rowRemovalOption); + } + + return data; + } + + public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) + { + if (rowRemovalOption == _rowRemovalOption) + { + return this; + } + return new CleanInstanceDataAccessor( + _dataAccessor, + _taskId, + _appResources, + _translationService, + _frontEndSettings, + rowRemovalOption, + _language, + _telemetry + ); + } + + public IInstanceDataAccessor GetPreviousDataAccessor() + { + return _dataAccessor.GetPreviousDataAccessor(); + } + + public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) + { + return await _dataAccessor.GetBinaryData(dataElementIdentifier); + } + + public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) + { + return _dataAccessor.GetDataElement(dataElementIdentifier); + } +} diff --git a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs index 375923e616..6d65eeaff9 100644 --- a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs +++ b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs @@ -33,10 +33,11 @@ internal sealed class InstanceDataUnitOfWork : IInstanceDataMutator private readonly ApplicationMetadata _appMetadata; private readonly ModelSerializationService _modelSerializationService; - // private readonly IAppResources _appResources; - // private readonly IOptions _frontEndSettings; - // private readonly string? _taskId; - // private readonly string? _language; + private readonly IAppResources _appResources; + private readonly IOptions _frontEndSettings; + private readonly string? _taskId; + private readonly string? _language; + private readonly ITranslationService _translationService; private readonly Telemetry? _telemetry; // Cache for the most up-to-date form data (can be mutated or replaced with SetFormData(dataElementId, data)) @@ -51,8 +52,6 @@ internal sealed class InstanceDataUnitOfWork : IInstanceDataMutator // Form data not yet saved to storage (thus no dataElementId) private readonly ConcurrentBag _changesForCreation = []; - // private readonly ITranslationService _translationService; - public InstanceDataUnitOfWork( Instance instance, IDataClient dataClient, @@ -78,12 +77,12 @@ public InstanceDataUnitOfWork( DataTypes = appMetadata.DataTypes; _dataClient = dataClient; _appMetadata = appMetadata; - // _translationService = translationService; + _translationService = translationService; _modelSerializationService = modelSerializationService; - // _taskId = taskId; - // _language = language; - // _frontEndSettings = frontEndSettings; - // _appResources = appResources; + _taskId = taskId; + _language = language; + _frontEndSettings = frontEndSettings; + _appResources = appResources; _instanceClient = instanceClient; _telemetry = telemetry; } @@ -121,6 +120,35 @@ public async Task GetFormDataWrapper(DataElementIdentifier dat ); } + /// + public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) + { + return new CleanInstanceDataAccessor( + this, + _taskId, + _appResources, + _translationService, + _frontEndSettings.Value, + rowRemovalOption, + _language, + _telemetry + ); + } + + public IInstanceDataAccessor GetPreviousDataAccessor() + { + return new PreviousDataAccessor( + this, + _taskId, + _appResources, + _translationService, + _modelSerializationService, + _frontEndSettings.Value, + _language, + _telemetry + ); + } + /// public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) { diff --git a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs new file mode 100644 index 0000000000..035c166562 --- /dev/null +++ b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs @@ -0,0 +1,92 @@ +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Helpers.DataModel; +using Altinn.App.Core.Helpers.Serialization; +using Altinn.App.Core.Internal.App; +using Altinn.App.Core.Internal.Texts; +using Altinn.App.Core.Models; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Internal.Data; + +internal class PreviousDataAccessor : IInstanceDataAccessor +{ + private readonly IInstanceDataAccessor _dataAccessor; + private readonly string? _taskId; + private readonly IAppResources _appResources; + private readonly ModelSerializationService _modelSerializationService; + private readonly FrontEndSettings _frontEndSettings; + private readonly string? _language; + private readonly ITranslationService _translationService; + private readonly Telemetry? _telemetry; + + public PreviousDataAccessor( + IInstanceDataAccessor dataAccessor, + string? taskId, + IAppResources appResources, + ITranslationService translationService, + ModelSerializationService modelSerializationService, + FrontEndSettings frontEndSettings, + string? language, + Telemetry? telemetry + ) + { + _dataAccessor = dataAccessor; + _taskId = taskId; + _appResources = appResources; + _translationService = translationService; + _modelSerializationService = modelSerializationService; + _frontEndSettings = frontEndSettings; + _language = language; + _telemetry = telemetry; + } + + public Instance Instance => _dataAccessor.Instance; + + public IReadOnlyCollection DataTypes => _dataAccessor.DataTypes; + + public async Task GetFormData(DataElementIdentifier dataElementIdentifier) + { + var binaryData = await _dataAccessor.GetBinaryData(dataElementIdentifier); + return _modelSerializationService.DeserializeFromStorage( + binaryData.Span, + this.GetDataType(dataElementIdentifier) + ); + } + + public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) + { + var dataModel = await GetFormData(dataElementIdentifier); + return FormDataWrapperFactory.Create(dataModel); + } + + public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) + { + return new CleanInstanceDataAccessor( + this, + _taskId, + _appResources, + _translationService, + _frontEndSettings, + rowRemovalOption, + _language, + _telemetry + ); + } + + public IInstanceDataAccessor GetPreviousDataAccessor() + { + return this; + } + + public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) + { + return await _dataAccessor.GetBinaryData(dataElementIdentifier); + } + + public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) + { + return _dataAccessor.GetDataElement(dataElementIdentifier); + } +} diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index 7777003355..14577180f9 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -1,6 +1,7 @@ using System.Diagnostics; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers; using Altinn.App.Core.Helpers.DataModel; using Altinn.App.Core.Internal.App; using Altinn.App.Core.Internal.Texts; @@ -84,6 +85,16 @@ public Task GetFormDataWrapper(DataElementIdentifier dataEleme return Task.FromResult(_data); } + public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption) + { + throw new NotImplementedException("Legacy data accessor does not implement GetCleanAccessorForTask"); + } + + public IInstanceDataAccessor GetPreviousDataAccessor() + { + throw new NotImplementedException("Legacy data accessor does not implement GetPreviousDataAccessor"); + } + public Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) { return Task.FromException>(new NotImplementedException()); diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index 44b382fb75..bff2da58d5 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -50,21 +50,35 @@ public async Task> ValidateInstanceAtTask( var validators = _validatorFactory.GetValidators(taskId); // Filter out validators that should be ignored or not run incrementally if (onlyIncrementalValidators == true) + { validators = validators.Where(v => !v.NoIncrementalValidation); + } else if (onlyIncrementalValidators == false) + { validators = validators.Where(v => v.NoIncrementalValidation); + } + + // Remove ignored validators if (ignoredValidators is not null) validators = validators.Where(v => !ignoredValidators.Contains(v.ValidationSource, StringComparer.InvariantCulture) ); + var cleanAccessor = dataAccessor; + validators = validators.ToArray(); + if (validators.Any(c => c.ShouldRunAfterRemovingHiddenData)) + { + cleanAccessor = dataAccessor.GetCleanAccessor(); + } + // Start the validation tasks (but don't await yet, so that they can run in parallel) var validationTasks = validators.Select(async v => { using var validatorActivity = _telemetry?.StartRunValidatorActivity(v); try { - var issues = await v.Validate(dataAccessor, taskId, language); + var accessor = v.ShouldRunAfterRemovingHiddenData ? cleanAccessor : dataAccessor; + var issues = await v.Validate(accessor, taskId, language); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorIssueCount, issues.Count); await TranslateValidationIssues(issues, language); return KeyValuePair.Create( @@ -119,17 +133,30 @@ public async Task> ValidateIncrementalFormData( ThrowIfDuplicateValidators(validators, taskId); + DataElementChanges cleanChanges = changes; + IInstanceDataAccessor cleanAccessor = dataAccessor; + if (validators.Any(p => p.ShouldRunAfterRemovingHiddenData)) + { + // Run validations on clean + cleanAccessor = dataAccessor.GetCleanAccessor(); + var previousCleanAccessor = dataAccessor.GetPreviousDataAccessor().GetCleanAccessor(); + + cleanChanges = await CleanFormDataChanges(changes, previousCleanAccessor, cleanAccessor); + } + // Start the validation tasks (but don't await yet, so that they can run in parallel) var validationTasks = validators.Select(async validator => { using var validatorActivity = _telemetry?.StartRunValidatorActivity(validator); try { - var hasRelevantChanges = await validator.HasRelevantChanges(dataAccessor, taskId, changes); + var localAccessor = validator.ShouldRunAfterRemovingHiddenData ? cleanAccessor : dataAccessor; + var localChanges = validator.ShouldRunAfterRemovingHiddenData ? cleanChanges : changes; + var hasRelevantChanges = await validator.HasRelevantChanges(localAccessor, taskId, localChanges); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorHasRelevantChanges, hasRelevantChanges); if (hasRelevantChanges) { - var issues = await validator.Validate(dataAccessor, taskId, language); + var issues = await validator.Validate(localAccessor, taskId, language); validatorActivity?.SetTag(Telemetry.InternalLabels.ValidatorIssueCount, issues.Count); await TranslateValidationIssues(issues, language); var issuesWithSource = issues @@ -169,11 +196,49 @@ public async Task> ValidateIncrementalFormData( return lists.OfType().ToList(); } + private static async Task CleanFormDataChanges( + DataElementChanges changes, + IInstanceDataAccessor previousAccessor, + IInstanceDataAccessor cleanAccessor + ) + { + var cleanedChangeList = new List(); + + foreach (var change in changes.AllChanges) + { + // Clean FormDataChange updates but keep other changes as is + if (change is FormDataChange { DataElement: not null, Type: ChangeType.Updated } fdc) + { + cleanedChangeList.Add( + new FormDataChange() + { + ContentType = fdc.ContentType, + DataElement = fdc.DataElement, + DataType = fdc.DataType, + Type = fdc.Type, + + PreviousFormData = await previousAccessor.GetFormData(fdc.DataElementIdentifier), + CurrentFormData = await cleanAccessor.GetFormData(fdc.DataElementIdentifier), + // The binary data is kept as is, because logic is assumed to not use it + CurrentBinaryData = fdc.CurrentBinaryData, + PreviousBinaryData = fdc.PreviousBinaryData, + } + ); + } + else + { + cleanedChangeList.Add(change); + } + } + + return new DataElementChanges(cleanedChangeList); + } + private async Task TranslateValidationIssues(IEnumerable issues, string? language) { foreach (var issue in issues) { - if (String.IsNullOrEmpty(issue.Description) && !String.IsNullOrEmpty(issue.CustomTextKey)) + if (string.IsNullOrEmpty(issue.Description) && !string.IsNullOrEmpty(issue.CustomTextKey)) { if ( await _translationService.TranslateTextKey( diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs index 5b29f13f16..a727489f87 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs @@ -61,6 +61,8 @@ public class DataControllerPatchTests : ApiTestBase, IClassFixture factory, ITestOutputHelper outputHelper) : base(factory, outputHelper) { + _formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); _formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs index d19e4cb9fe..54bf81ebc9 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs @@ -50,6 +50,8 @@ public class ProcessControllerTests : ApiTestBase, IClassFixture factory, ITestOutputHelper outputHelper) : base(factory, outputHelper) { + _formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); _formDataValidatorMock.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(false); @@ -249,6 +251,8 @@ public async Task RunProcessNext_PdfFails_DataIsUnlocked() public async Task RunProcessNext_FailingValidator_ReturnsValidationErrors() { var dataValidator = new Mock(MockBehavior.Strict); + dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); + dataValidator.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); @@ -310,6 +314,7 @@ public async Task RunProcessNext_FailingValidator_ReturnsValidationErrors() public async Task RunProcessNext_FailingValidator_Reject_ReturnsOk() { var dataValidator = new Mock(MockBehavior.Strict); + dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); @@ -560,6 +565,8 @@ public async Task RunProcessNext_ShadowFields_GetsRemoved(string? saveToDataType public async Task RunProcessNext_NonErrorValidations_ReturnsOk() { var dataValidator = new Mock(MockBehavior.Strict); + dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); + dataValidator.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs index 3e171f2ee7..ac5d14513d 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs @@ -43,6 +43,8 @@ ITestOutputHelper outputHelper ) : base(factory, outputHelper) { + _formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); _formDataValidatorMock.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(false); diff --git a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs index addc235b5e..ce044714c1 100644 --- a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs @@ -76,6 +76,8 @@ public PatchServiceTests() .Setup(a => a.GetModelType("Altinn.App.Core.Tests.Internal.Patch.PatchServiceTests+MyModel")) .Returns(typeof(MyModel)) .Verifiable(); + _formDataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); + _formDataValidator.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidator.Setup(fdv => fdv.DataType).Returns(_dataType.Id); _formDataValidator.Setup(fdv => fdv.ValidationSource).Returns("formDataValidator"); _formDataValidator.Setup(fdv => fdv.HasRelevantChanges(It.IsAny(), It.IsAny())).Returns(true); diff --git a/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs index ca7d2bba14..f6695ef2d5 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests.cs @@ -25,6 +25,8 @@ private class TestValidator : GenericFormDataValidator public TestValidator() : base("MyType") { } + public override bool ShouldRunAfterRemovingHiddenData => true; + protected override bool HasRelevantChanges(MyModel current, MyModel previous) { throw new NotImplementedException(); 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 dc2503eb2b..46ce50b260 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs @@ -238,6 +238,9 @@ private void SetupFormDataValidatorType( string validationSource ) { + formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); + // DataType formDataValidatorMock.Setup(v => v.DataType).Returns(dataType); diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 590e5e8940..6ab126f059 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -88,6 +88,7 @@ private Mock RegisterValidatorMock( } mock.SetupGet(v => v.NoIncrementalValidation).Returns(noIncrementalValidation); + mock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _services.AddSingleton(mock.Object); return mock; @@ -316,6 +317,8 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() { Name = "FormDataValidatorNoAppLogic", }; + formDataValidatorNoAppLogicMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + formDataValidatorNoAppLogicMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); formDataValidatorNoAppLogicMock .SetupGet(v => v.DataType) .Returns("dataTypeNoAppLogic") @@ -332,6 +335,8 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() { Name = "FormDataValidatorWrongTask", }; + formDataValidatorWrongTaskMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + formDataValidatorWrongTaskMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); formDataValidatorWrongTaskMock .SetupGet(v => v.DataType) .Returns("dataTypeWrongTask") @@ -352,6 +357,8 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() ); var formDataValidatorMock = new Mock(MockBehavior.Strict) { Name = "FormDataValidator" }; + formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); + formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); formDataValidatorMock.SetupGet(v => v.DataType).Returns("dataType").Verifiable(Times.AtLeastOnce); formDataValidatorMock .SetupGet(v => v.ValidationSource) diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs new file mode 100644 index 0000000000..344ee55a2d --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs @@ -0,0 +1,315 @@ +using System.Text.Json; +using System.Text.Json.Serialization; +using Altinn.App.Core.Features; +using Altinn.App.Core.Internal.Data; +using Microsoft.Extensions.DependencyInjection; +using Xunit.Abstractions; + +namespace Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor; + +public class TestCleanDataAccessor +{ + private readonly ITestOutputHelper _outputHelper; + + public TestCleanDataAccessor(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + public record SubModel + { + public bool? HideSubPage { get; set; } + public bool? HideSubPageTitle { get; set; } + public string? SubPageTitle { get; set; } + public string? UnboundField { get; set; } + + public bool? HideSubComponentGroup { get; set; } + public List? SubComponentGroup { get; set; } + + public record SubGroup + { + [JsonPropertyName("altinnRowId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Guid AltinnRowId { get; set; } + + public bool? HideRow { get; set; } + public bool? HideName { get; set; } + public string? Name { get; set; } + public string? Description { get; set; } + } + } + + public record MainModel + { + public bool? HideMainTitle { get; set; } + public string? MainTitle { get; set; } + public string? UnboundField { get; set; } + public bool? HidePage1 { get; set; } + + public bool? HideMainComponentGroup { get; set; } + public List? MainComponentGroup { get; set; } + + public bool? HideSubLayout { get; set; } + + public record MainComponentGroupItem + { + [JsonPropertyName("altinnRowId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Guid AltinnRowId { get; set; } + public string? Name { get; set; } + public bool? HideRow { get; set; } + public string? Description { get; set; } + public bool? HideName { get; set; } + } + } + + [Fact] + public async Task TestEverythingEmpty() + { + var data = new MainModel(); + + var (cleanModel, cleanSubModels) = await GetMainAndSubClean(data, []); + + // Assert + Assert.Empty(cleanSubModels); + Assert.NotSame(cleanModel, data); + Assert.Equivalent(data, cleanModel); + } + + [Fact] + public async Task TestEverythingHidden() + { + var data = new MainModel() + { + HideMainTitle = false, + HidePage1 = true, // Everything is hidden + MainTitle = "Title1", + UnboundField = "Not deleted", + MainComponentGroup = new() + { + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 1", + Description = "Description 1", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 2", + Description = "Description 2", + }, + }, + }; + + var subData = new SubModel(); + + var expectedModel = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(data)); + var (cleanModel, cleanSubModels) = await GetMainAndSubClean(data, [subData]); + var cleanSubModel = cleanSubModels.Single(); + // Assert + Assert.NotSame(cleanModel, data); + Assert.NotSame(cleanSubModel, subData); + + // Make expected changes + // ReSharper disable PossibleNullReferenceException +#nullable disable + expectedModel.MainTitle = null; + expectedModel.MainComponentGroup = null; +#nullable restore + + Assert.Equivalent(expectedModel, cleanModel); + } + + [Fact] + public async Task TestHideRow2AndNameInRow1() + { + var data = new MainModel() + { + HidePage1 = false, + + MainComponentGroup = new() + { + new() + { + AltinnRowId = Guid.NewGuid(), + HideName = true, + Name = "Item 1", + Description = "Description 1", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + HideRow = true, + Name = "Item 2", + Description = "Description 2", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 2", + Description = "Description 2", + }, + }, + }; + + var expectedModel = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(data)); + var (cleanModel, cleanSubModels) = await GetMainAndSubClean(data, []); + + // Assert + Assert.Empty(cleanSubModels); + Assert.NotSame(cleanModel, data); + + // Make expected changes + // ReSharper disable PossibleNullReferenceException +#nullable disable + expectedModel.MainComponentGroup[0].Name = null; + expectedModel.MainComponentGroup[1] = null; +#nullable restore + + Assert.Equivalent(expectedModel, cleanModel); + } + + [Fact] + public async Task HideRowAndNameInSubComponent() + { + var data = new MainModel() + { + MainComponentGroup = new() + { + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 1", + Description = "Description 1", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 2", + Description = "Description 2", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Item 2", + Description = "Description 2", + }, + }, + }; + + var sub1 = new SubModel() + { + HideSubPageTitle = true, + SubPageTitle = "removeMe", + HideSubComponentGroup = true, + UnboundField = "Unbound", + SubComponentGroup = new() + { + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Name 1", + Description = "Description 1", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Name 2", + Description = "Description 2", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Name 3", + Description = "Description 3", + }, + }, + }; + var sub2 = new SubModel() + { + SubPageTitle = "doNotRemove", + UnboundField = "Unbound", + SubComponentGroup = new() + { + new() + { + AltinnRowId = Guid.NewGuid(), + HideName = true, + Name = "Name 1", + Description = "Description 1", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + HideRow = true, + Name = "Name 2", + Description = "Description 2", + }, + new() + { + AltinnRowId = Guid.NewGuid(), + Name = "Name 3", + Description = "Description 3", + }, + }, + }; + + var expectedModel = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(data)); + var expectedSub1 = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(sub1)); + var expectedSub2 = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(sub2)); + var (cleanModel, cleanSubModels) = await GetMainAndSubClean(data, [sub1, sub2]); + + // Assert + Assert.Equal(2, cleanSubModels.Length); + + var cleanSub1 = cleanSubModels[0]; + var cleanSub2 = cleanSubModels[1]; + + Assert.NotSame(cleanModel, data); + Assert.NotSame(cleanSub1, sub1); + Assert.NotSame(cleanSub2, sub2); + Assert.NotSame(cleanSub1, sub2); + Assert.NotSame(cleanSub2, sub1); + + // Make expected changes + // ReSharper disable PossibleNullReferenceException +#nullable disable + expectedSub1.SubPageTitle = null; + expectedSub1.SubComponentGroup = null; + expectedSub2.SubComponentGroup[0].Name = null; + expectedSub2.SubComponentGroup[1] = null; +#nullable restore + + Assert.Equivalent(expectedModel, cleanModel); + Assert.Equivalent(expectedSub1, cleanSub1); + Assert.Equivalent(expectedSub2, cleanSub2); + } + + private async Task<(T1?, T2?[])> GetMainAndSubClean(T1 data, T2[] subDatas) + where T1 : class + where T2 : class + { + var fixture = await DataAccessorFixture.CreateAsync( + [new("mainLayout", typeof(T1), MaxCount: 1), new("subLayout", typeof(T2), MaxCount: 1)], + _outputHelper + ); + fixture.AddFormData(data); + foreach (var subData in subDatas) + { + fixture.AddFormData(subData); + } + await using var sp = fixture.ServiceCollection.BuildServiceProvider(); + var dataUnitOfWorkInitializer = sp.GetRequiredService(); + var dataMutator = await dataUnitOfWorkInitializer.Init( + fixture.Instance, + DataAccessorFixture.TaskId, + "test-language" + ); + + var cleanDataAccessor = dataMutator.GetCleanAccessor(); + var mainModel = await cleanDataAccessor.GetFormData(); + var subModels = await cleanDataAccessor.GetAllFormData(); + return (mainModel, subModels); + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/mainLayout/page1.json b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/mainLayout/page1.json new file mode 100644 index 0000000000..8d817e75bd --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/mainLayout/page1.json @@ -0,0 +1,50 @@ +{ + "$schema": "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json", + "data": { + "hidden": ["dataModel", "HidePage1"], + "layout": [ + { + "id": "mainTitle", + "type": "Input", + "hidden": ["dataModel", "HideMainTitle"], + "dataModelBindings": { + "simpleBinding": "MainTitle" + } + }, + { + "id": "mainComponentGroup", + "type": "RepeatingGroup", + "dataModelBindings": { + "group": "MainComponentGroup" + }, + "hidden": ["dataModel", "HideMainComponentGroup"], + "hiddenRow": ["dataModel", "MainComponentGroup.HideRow"], + "children": [ + "mainComponentInput" + ] + }, + { + "id": "mainComponentInput", + "type": "Input", + "hidden": ["dataModel", "MainComponentGroup.HideName"], + "dataModelBindings": { + "simpleBinding": "MainComponentGroup.Name" + } + }, + { + "id": "subLayout", + "type": "Subform", + "layoutSet": "subLayout", + "hidden": ["dataModel", "HideSubLayout"], + "tableColumns": [ + { + "headerContent": "SubModelTitle", + "cellContent": { + "query": "SubModelTitle" + } + } + ] + } + ] + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/subLayout/subPage.json b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/subLayout/subPage.json new file mode 100644 index 0000000000..3aefe18965 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/subLayout/subPage.json @@ -0,0 +1,36 @@ +{ + "$schema": "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json", + "data": { + "hidden": ["dataModel", "HideSubPage"], + "layout": [ + { + "id": "subPageTitle", + "type": "Input", + "hidden": ["dataModel", "HideSubPageTitle"], + "dataModelBindings": { + "simpleBinding": "SubPageTitle" + } + }, + { + "id": "subComponentGroup", + "type": "RepeatingGroup", + "hidden": ["dataModel", "HideSubComponentGroup"], + "hiddenRow": ["dataModel", "SubComponentGroup.HideRow"], + "dataModelBindings": { + "group": "SubComponentGroup" + }, + "children": [ + "subComponentInput" + ] + }, + { + "id": "subComponentInput", + "type": "Input", + "hidden": ["dataModel", "SubComponentGroup.HideName"], + "dataModelBindings": { + "simpleBinding": "SubComponentGroup.Name" + } + } + ] + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs new file mode 100644 index 0000000000..c08d99bb7a --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs @@ -0,0 +1,190 @@ +using System.Runtime.CompilerServices; +using System.Text.Json; +using Altinn.App.Core.Configuration; +using Altinn.App.Core.Features; +using Altinn.App.Core.Features.Validation.Default; +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.Expressions; +using Altinn.App.Core.Internal.Instances; +using Altinn.App.Core.Internal.Texts; +using Altinn.App.Core.Internal.Validation; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Layout; +using Altinn.App.Core.Models.Layout.Components; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Options; +using Moq; +using Xunit.Abstractions; + +namespace Altinn.App.Core.Tests.LayoutExpressions.FullTests; + +public sealed class DataAccessorFixture +{ + public const string Org = "ttd"; + public const string App = "data-accessor-fixture"; + public const string TaskId = "Task_1-access"; + public const int InstanceOwnerPartyId = 1337; + public static readonly Guid InstanceGuid = Guid.Parse("00000000-BABE-0000-0000-000000000001"); + + public Mock AppResourcesMock { get; } = new(MockBehavior.Strict); + public Mock AppMetadataMock { get; } = new(MockBehavior.Strict); + public Mock AppModelMock { get; } = new(MockBehavior.Strict); + + public Mock DataClientMock { get; } = new(MockBehavior.Strict); + public Mock InstanceClientMock { get; } = new(MockBehavior.Strict); + + public Mock TranslationServiceMock { get; } = new(MockBehavior.Strict); + + internal Mock DataElementAccessCheckerMock { get; } = new(MockBehavior.Strict); + + public FrontEndSettings FrontEndSettings { get; } = new(); + public GeneralSettings GeneralSettings { get; } = new(); + public AppSettings AppSettings { get; } = new(); + public ApplicationMetadata ApplicationMetadata { get; } = new($"{Org}/{App}") { DataTypes = [] }; + + public Instance Instance = new() + { + Id = $"{InstanceOwnerPartyId}/{InstanceGuid}", + InstanceOwner = new() { PartyId = InstanceOwnerPartyId.ToString() }, + Data = [], + }; + + public IServiceCollection ServiceCollection { get; } = new ServiceCollection(); + + private DataAccessorFixture(ITestOutputHelper outputHelper) + { + AppMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(ApplicationMetadata); + ServiceCollection.AddSingleton(AppResourcesMock.Object); + ServiceCollection.AddSingleton(AppMetadataMock.Object); + ServiceCollection.AddSingleton(Options.Create(FrontEndSettings)); + ServiceCollection.AddSingleton(Options.Create(GeneralSettings)); + ServiceCollection.AddSingleton(Options.Create(AppSettings)); + ServiceCollection.AddSingleton(AppModelMock.Object); + ServiceCollection.AddSingleton(DataClientMock.Object); + ServiceCollection.AddSingleton(TranslationServiceMock.Object); + ServiceCollection.AddSingleton(InstanceClientMock.Object); + ServiceCollection.AddSingleton(DataElementAccessCheckerMock.Object); + ServiceCollection.AddSingleton(); + ServiceCollection.AddSingleton(); + ServiceCollection.AddFakeLoggingWithXunit(outputHelper); + ServiceCollection.AddTransient(); + ServiceCollection.AddTransient(); + ServiceCollection.AddTransient(); + ServiceCollection.AddTransient(); + ServiceCollection.AddTransient(); + ServiceCollection.AddSingleton(TranslationServiceMock.Object); + ServiceCollection.AddFakeLoggingWithXunit(outputHelper); + AppResourcesMock + .Setup(ar => ar.GetLayoutSet()) + .Returns( + new LayoutSets() + { + // RequiredLayoutValidator checks to see if TaskId has a layout to see if it should run + Sets = new() + { + new() + { + Id = "default", + DataType = "fake", + Tasks = new() { TaskId }, + }, + }, + } + ); + } + + public static async Task CreateAsync( + List specs, + ITestOutputHelper outputHelper, + [CallerFilePath] string callerFilePath = "" + ) + { + var fixture = new DataAccessorFixture(outputHelper); + await fixture.AddLayouts(specs, callerFilePath); + return fixture; + } + + public record LayoutSetSpec(string LayoutSetName, Type Type, int MaxCount); + + /// + /// The first spec is the default layout set. The remaining can be referenced as subforms + /// + private async Task AddLayouts(List specs, string callerFilePath) + { + var directory = + Path.GetDirectoryName(callerFilePath) ?? throw new InvalidOperationException("Could not get directory"); + List layouts = []; + + foreach (var spec in specs) + { + var pageNames = Directory + .GetFiles(Path.Join(directory, spec.LayoutSetName), "*.json") + .Select(Path.GetFileNameWithoutExtension); + + var pages = await Task.WhenAll( + pageNames.Select(async pageName => + { + var pageText = await File.ReadAllTextAsync( + Path.Join(directory, spec.LayoutSetName, $"{pageName}.json") + ); + using var document = JsonDocument.Parse(pageText); + var pageComponent = new PageComponent(document.RootElement, pageName!, spec.LayoutSetName); + return pageComponent; + }) + ); + + var dataType = new DataType() + { + Id = spec.LayoutSetName + "_dataType", + TaskId = TaskId, + AppLogic = new() { ClassRef = spec.Type.FullName }, + MaxCount = spec.MaxCount, + }; + ApplicationMetadata.DataTypes.Add(dataType); + + AppModelMock.Setup(am => am.GetModelType(spec.Type.FullName!)).Returns(spec.Type); + AppModelMock.Setup(am => am.Create(spec.Type.FullName!)).Returns(Activator.CreateInstance(spec.Type)!); + + var layoutSet = new LayoutSetComponent(pages.ToList(), spec.LayoutSetName, dataType); + layouts.Add(layoutSet); + } + + var layoutModel = new LayoutModel(layouts, null); + AppResourcesMock.Setup(ar => ar.GetLayoutModelForTask(TaskId)).Returns(layoutModel); + } + + public void AddFormData(object data, int? maxCount = null) + { + var fullName = data.GetType().FullName; + var dataType = ApplicationMetadata.DataTypes.Find(dt => dt.AppLogic?.ClassRef == fullName); + + if (dataType == null && maxCount != null) + { + dataType = new DataType() + { + Id = data.GetType().Name, + TaskId = TaskId, + MaxCount = maxCount.Value, + AppLogic = new() { ClassRef = fullName }, + }; + ApplicationMetadata.DataTypes.Add(dataType); + AppModelMock.Setup(am => am.GetModelType(fullName!)).Returns(data.GetType()); + AppModelMock.Setup(am => am.Create(fullName!)).Returns(Activator.CreateInstance(data.GetType())!); + } + else if (dataType is null) + { + throw new ArgumentException($"Data type {fullName} not found in ApplicationMetadata"); + } + var dataGuid = Guid.NewGuid(); + var dataElement = new DataElement() { Id = dataGuid.ToString(), DataType = dataType.Id }; + Instance.Data.Add(dataElement); + var serializationService = new ModelSerializationService(AppModelMock.Object); + DataClientMock + .Setup(dc => dc.GetDataBytes(Org, App, InstanceOwnerPartyId, InstanceGuid, dataGuid, null, default)) + .ReturnsAsync(serializationService.SerializeToStorage(data, dataType).data.ToArray()); + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateAllRequiredFieldsMissing.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateAllRequiredFieldsMissing.verified.txt new file mode 100644 index 0000000000..796fe3505f --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateAllRequiredFieldsMissing.verified.txt @@ -0,0 +1,136 @@ +{ + IncrementalIssues: [ + { + Source: Required, + Issues: [ + { + Severity: Error, + DataElementId: Guid_1, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id subLayout.subPage.sub-input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_1, + Field: Group[1].VisibleRequired, + Code: required, + Description: Group[1].VisibleRequired is required in component with id subLayout.subPage.sub-input-group1-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_1, + Field: Group[2].VisibleRequired, + Code: required, + Description: Group[2].VisibleRequired is required in component with id subLayout.subPage.sub-input-group2-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id mainLayout.page1.input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: Group[1].VisibleRequired, + Code: required, + Description: Group[1].VisibleRequired is required in component with id mainLayout.page1.input-group1-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: Group[2].VisibleRequired, + Code: required, + Description: Group[2].VisibleRequired is required in component with id mainLayout.page1.input-group2-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + } + ] + }, + { + Source: Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator.RequiredValidatorTests+TestValidator-Task_1-access, + Issues: [ + { + Severity: Error, + Description: ServerValidationNotTest contains test in Model, + Source: Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator.RequiredValidatorTests+TestValidator-Task_1-access, + NoIncrementalUpdates: false + } + ] + } + ], + FullIssues: [ + { + Severity: Error, + DataElementId: Guid_1, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id subLayout.subPage.sub-input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_1, + Field: Group[1].VisibleRequired, + Code: required, + Description: Group[1].VisibleRequired is required in component with id subLayout.subPage.sub-input-group1-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_1, + Field: Group[2].VisibleRequired, + Code: required, + Description: Group[2].VisibleRequired is required in component with id subLayout.subPage.sub-input-group2-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id mainLayout.page1.input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: Group[1].VisibleRequired, + Code: required, + Description: Group[1].VisibleRequired is required in component with id mainLayout.page1.input-group1-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: Group[2].VisibleRequired, + Code: required, + Description: Group[2].VisibleRequired is required in component with id mainLayout.page1.input-group2-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + Description: ServerValidationNotTest contains test in Model, + Source: Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator.RequiredValidatorTests+TestValidator-Task_1-access, + NoIncrementalUpdates: false + } + ] +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateEmpty_IssuesRequiredIssues.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateEmpty_IssuesRequiredIssues.verified.txt new file mode 100644 index 0000000000..51281f08b6 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.ValidateEmpty_IssuesRequiredIssues.verified.txt @@ -0,0 +1,50 @@ +{ + IncrementalIssues: [ + { + Source: Required, + Issues: [ + { + Severity: Error, + DataElementId: Guid_1, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id subLayout.subPage.sub-input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id mainLayout.page1.input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + } + ] + }, + { + Source: Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator.RequiredValidatorTests+TestValidator-Task_1-access + } + ], + FullIssues: [ + { + Severity: Error, + DataElementId: Guid_1, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id subLayout.subPage.sub-input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + }, + { + Severity: Error, + DataElementId: Guid_2, + Field: VisibleRequired, + Code: required, + Description: VisibleRequired is required in component with id mainLayout.page1.input-visible-required for binding simpleBinding, + Source: Required, + NoIncrementalUpdates: false + } + ] +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.VerifyAllOk.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.VerifyAllOk.verified.txt new file mode 100644 index 0000000000..4aa2453404 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.VerifyAllOk.verified.txt @@ -0,0 +1,10 @@ +{ + IncrementalIssues: [ + { + Source: Required + }, + { + Source: Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator.RequiredValidatorTests+TestValidator-Task_1-access + } + ] +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs new file mode 100644 index 0000000000..190c11941c --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs @@ -0,0 +1,230 @@ +using System.Text.Json.Serialization; +using Altinn.App.Core.Features; +using Altinn.App.Core.Internal.Data; +using Altinn.App.Core.Internal.Validation; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Validation; +using Microsoft.Extensions.DependencyInjection; +using Xunit.Abstractions; + +namespace Altinn.App.Core.Tests.LayoutExpressions.FullTests.RequiredValidator; + +public class RequiredValidatorTests +{ + public class Model + { + public string? HiddenRequired { get; set; } + public string? HiddenNotRequired { get; set; } + public string? VisibleRequired { get; set; } + public string? VisibleNotRequired { get; set; } + public string? ServerValidatedNotTest { get; set; } + public string? ServerValidatedHidden { get; set; } + + public List? Group { get; set; } + + public class MainComponentGroupItem + { + [JsonPropertyName("altinnRowId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Guid AltinnRowId { get; set; } + public int? GroupNumber { get; set; } + public string? HiddenRequired { get; set; } + public string? HiddenNotRequired { get; set; } + public string? VisibleRequired { get; set; } + public string? VisibleNotRequired { get; set; } + } + } + + // This test sets DataType.Id to data.GetType().Name, so we need different classes for + // distinct data types + public class SubModel : Model { } + + private readonly ITestOutputHelper _outputHelper; + + public RequiredValidatorTests(ITestOutputHelper outputHelper) + { + _outputHelper = outputHelper; + } + + [Fact] + public async Task ValidateEmpty_IssuesRequiredIssues() + { + await VerifyValidationIssues(new(), [new()]); + } + + [Fact] + public async Task ValidateAllRequiredFieldsMissing() + { + await VerifyValidationIssues( + new() + { + VisibleRequired = null, + ServerValidatedHidden = "test", + ServerValidatedNotTest = "test", + Group = + [ + new() { GroupNumber = 0, VisibleRequired = null }, + new() { GroupNumber = 1, VisibleRequired = null }, + new() { GroupNumber = 2, VisibleRequired = null }, + ], + }, + [ + new() + { + VisibleRequired = null, + Group = + [ + new() { GroupNumber = 0, VisibleRequired = null }, + new() { GroupNumber = 1, VisibleRequired = null }, + new() { GroupNumber = 2, VisibleRequired = null }, + ], + }, + ] + ); + } + + [Fact] + public async Task VerifyAllOk() + { + await VerifyValidationIssues( + new() + { + ServerValidatedHidden = "test", + ServerValidatedNotTest = "something valid", + VisibleRequired = "something valid", + Group = + [ + new() { GroupNumber = 1, VisibleRequired = "something valid" }, + new() + { + GroupNumber = -1, + VisibleRequired = null, // The row with GroupNumber -1 is not rendered in any group, so required properties can be null + }, + ], + }, + [ + new() + { + VisibleRequired = "something valid", + ServerValidatedHidden = "test", + ServerValidatedNotTest = "something valid", // This is not required, so it should not be an issue + Group = + [ + new() { GroupNumber = 1, VisibleRequired = "something valid" }, + new() + { + GroupNumber = -1, + VisibleRequired = null, // The row with GroupNumber -1 is not rendered in any group, so required properties can be null + }, + ], + }, + ] + ); + } + + private async Task VerifyValidationIssues(Model data, SubModel[] subDatas) + { + var fixture = await DataAccessorFixture.CreateAsync( + [new("mainLayout", typeof(Model), MaxCount: 1), new("subLayout", typeof(SubModel), MaxCount: 0)], + _outputHelper + ); + fixture.AddFormData(data); + foreach (var subData in subDatas) + { + fixture.AddFormData(subData); + } + + fixture.ServiceCollection.AddTransient(); + await using var sp = fixture.ServiceCollection.BuildServiceProvider(); + + var dataUnitOfWorkInitializer = sp.GetRequiredService(); + var dataMutator = await dataUnitOfWorkInitializer.Init( + fixture.Instance, + DataAccessorFixture.TaskId, + "test-language" + ); + + var validationService = sp.GetRequiredService(); + var changes = new DataElementChanges( + [ + new FormDataChange() + { + ContentType = "application/xml", + DataType = dataMutator.GetDataType("mainLayout_dataType"), + DataElement = null, + CurrentBinaryData = null, + PreviousBinaryData = null, + CurrentFormData = data, + PreviousFormData = new Model(), + Type = ChangeType.Created, + }, + ] + ); + var incrementalIssues = await validationService.ValidateIncrementalFormData( + dataMutator, + DataAccessorFixture.TaskId, + changes, + [], + "test-language" + ); + var fullIssues = await validationService.ValidateInstanceAtTask( + dataMutator, + DataAccessorFixture.TaskId, + [], + null, + "test-language" + ); + + await Verify(new { IncrementalIssues = incrementalIssues, FullIssues = fullIssues }); + } + + private class TestValidator : IValidator + { + public string TaskId { get; } = DataAccessorFixture.TaskId; + + public bool ShouldRunAfterRemovingHiddenData => true; + + public async Task> Validate( + IInstanceDataAccessor dataAccessor, + string taskId, + string? language + ) + { + var issues = new List(); + var models = await dataAccessor.GetAllFormData(); + foreach (var model in models) + { + if (model.ServerValidatedHidden?.Contains("test") == true) + { + issues.Add( + new() + { + Severity = ValidationIssueSeverity.Error, + Description = $"ServerValidationHidden contains test in {model.GetType().Name}", + } + ); + } + if (model.ServerValidatedNotTest?.Contains("test") == true) + { + issues.Add( + new() + { + Severity = ValidationIssueSeverity.Error, + Description = $"ServerValidationNotTest contains test in {model.GetType().Name}", + } + ); + } + } + return issues; + } + + public Task HasRelevantChanges( + IInstanceDataAccessor dataAccessor, + string taskId, + DataElementChanges changes + ) + { + return Task.FromResult(changes.FormDataChanges.Any(c => c.DataType.Id == "mainLayout_dataType")); + } + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json new file mode 100644 index 0000000000..76f8cd18ad --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json @@ -0,0 +1,205 @@ +{ + "$schema": "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json", + "data": { + "hidden": [ + "dataModel", + "HidePage" + ], + "layout": [ + { + "id": "subPage", + "type": "Subform", + "layoutSet": "subLayout", + "tableColumns": [ + { + "headerContent": "blah", + "cellContent": { + "value": [ + "dataModel", + "Group.GroupNumber" + ] + } + } + ] + }, + { + "id": "sub-server-validated-hidden", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "ServerValidatedHidden" + } + }, + { + "id": "input-hidden-requried", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "HiddenRequired" + } + }, + { + "id": "input-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "HiddenNotRequired" + } + }, + { + "id": "input-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "VisibleRequired" + } + }, + { + "id": "input-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "VisibleNotRequired" + } + }, + { + "id": "group1", + "type": "RepeatingGroup", + "hiddenRow": [ + "or", + [ + "equals", + [ + "dataModel", + "Group.GroupNumber" + ], + -1 + ], + [ + "equals", + [ + "dataModel", + "Group.GroupNumber" + ], + 0 + ], + [ + "equals", + [ + "dataModel", + "Group.GroupNumber" + ], + 2 + ] + ], + "dataModelBindings": { + "group": "Group" + }, + "children": [ + "input-group1-visible-required", + "input-group1-visible-not-required", + "input-group1-hidden-required", + "input-group1-hidden-not-required" + ] + }, + { + "id": "input-group1-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleRequired" + } + }, + { + "id": "input-group1-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleNotRequired" + } + }, + { + "id": "input-group1-hidden-required", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenRequired" + } + }, + { + "id": "input-group1-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenNotRequired" + } + }, + { + "id": "group2", + "type": "RepeatingGroup", + "hiddenRow": [ + "notEquals", + [ + "dataModel", + "Group.GroupNumber" + ], + 2 + ], + "dataModelBindings": { + "group": "Group" + }, + "children": [ + "input-group2-visible-required", + "input-group2-visible-not-required", + "input-group2-hidden-required", + "input-group2-hidden-not-required" + ] + }, + { + "id": "input-group2-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleRequired" + } + }, + { + "id": "input-group2-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleNotRequired" + } + }, + { + "id": "input-group2-hidden-required", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenRequired" + } + }, + { + "id": "input-group2-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenNotRequired" + } + } + ] + } +} \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/subLayout/subPage.json b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/subLayout/subPage.json new file mode 100644 index 0000000000..ab5eea2d12 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/subLayout/subPage.json @@ -0,0 +1,147 @@ +{ + "$schema": "https://altinncdn.no/toolkits/altinn-app-frontend/4/schemas/json/layout/layout.schema.v1.json", + "data": { + "hidden": [ + "dataModel", + "HideSubPage" + ], + "layout": [ + { + "id": "sub-input-hidden-required", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "HiddenRequired" + } + }, + { + "id": "sub-input-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "HiddenNotRequired" + } + }, + { + "id": "sub-input-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "VisibleRequired" + } + }, + { + "id": "sub-input-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "VisibleNotRequired" + } + }, + { + "id": "sub-group1", + "type": "RepeatingGroup", + "hiddenRow": ["notEquals", ["dataModel", "Group.GroupNumber"], 1], + "dataModelBindings": { + "group": "Group" + }, + "children": [ + "sub-input-group1-visible-required", + "sub-input-group1-visible-not-required", + "sub-input-group1-hidden-required", + "sub-input-group1-hidden-not-required" + ] + }, + { + "id": "sub-input-group1-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleRequired" + } + }, + { + "id": "sub-input-group1-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleNotRequired" + } + }, + { + "id": "sub-input-group1-hidden-required", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenRequired" + } + }, + { + "id": "sub-input-group1-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenNotRequired" + } + }, + { + "id": "sub-group2", + "type": "RepeatingGroup", + "hiddenRow": ["notEquals", ["dataModel", "Group.GroupNumber"], 2], + "dataModelBindings": { + "group": "Group" + }, + "children": [ + "sub-input-group2-visible-required", + "sub-input-group2-visible-not-required", + "sub-input-group2-hidden-required", + "sub-input-group2-hidden-not-required" + ] + }, + { + "id": "sub-input-group2-visible-required", + "type": "Input", + "required": true, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleRequired" + } + }, + { + "id": "sub-input-group2-visible-not-required", + "type": "Input", + "required": false, + "hidden": false, + "dataModelBindings": { + "simpleBinding": "Group.VisibleNotRequired" + } + }, + { + "id": "sub-input-group2-hidden-required", + "type": "Input", + "required": true, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenRequired" + } + }, + { + "id": "sub-input-group2-hidden-not-required", + "type": "Input", + "required": false, + "hidden": true, + "dataModelBindings": { + "simpleBinding": "Group.HiddenNotRequired" + } + } + ] + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs index 218001a949..ea7da2e14e 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs @@ -80,6 +80,16 @@ public Task GetFormDataWrapper(DataElementIdentifier dataEleme return Task.FromResult(FormDataWrapperFactory.Create(_dataById[dataElementIdentifier])); } + public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) + { + throw new NotImplementedException(); + } + + public IInstanceDataAccessor GetPreviousDataAccessor() + { + throw new NotImplementedException(); + } + public Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) { throw new NotImplementedException(); diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index fe462cae8c..0274b9c7eb 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -1248,6 +1248,7 @@ namespace Altinn.App.Core.Features { string DataType { get; } bool NoIncrementalValidation { get; } + bool ShouldRunAfterRemovingHiddenData { get; } string ValidationSource { get; } bool HasRelevantChanges(object current, object previous); System.Threading.Tasks.Task> ValidateFormData(Altinn.Platform.Storage.Interface.Models.Instance instance, Altinn.Platform.Storage.Interface.Models.DataElement dataElement, object data, string? language); @@ -1277,9 +1278,11 @@ namespace Altinn.App.Core.Features System.Collections.Generic.IReadOnlyCollection DataTypes { get; } Altinn.Platform.Storage.Interface.Models.Instance Instance { get; } System.Threading.Tasks.Task> GetBinaryData(Altinn.App.Core.Models.DataElementIdentifier dataElementIdentifier); + Altinn.App.Core.Features.IInstanceDataAccessor GetCleanAccessor(Altinn.App.Core.Helpers.RowRemovalOption rowRemovalOption = 1); Altinn.Platform.Storage.Interface.Models.DataElement GetDataElement(Altinn.App.Core.Models.DataElementIdentifier dataElementIdentifier); System.Threading.Tasks.Task GetFormData(Altinn.App.Core.Models.DataElementIdentifier dataElementIdentifier); System.Threading.Tasks.Task GetFormDataWrapper(Altinn.App.Core.Models.DataElementIdentifier dataElementIdentifier); + Altinn.App.Core.Features.IInstanceDataAccessor GetPreviousDataAccessor(); } public static class IInstanceDataAccessorExtensions { @@ -1407,6 +1410,7 @@ namespace Altinn.App.Core.Features public interface IValidator { bool NoIncrementalValidation { get; } + bool ShouldRunAfterRemovingHiddenData { get; } string TaskId { get; } string ValidationSource { get; } System.Threading.Tasks.Task HasRelevantChanges(Altinn.App.Core.Features.IInstanceDataAccessor dataAccessor, string taskId, Altinn.App.Core.Models.DataElementChanges changes); @@ -2053,6 +2057,7 @@ namespace Altinn.App.Core.Features.Validation protected GenericFormDataValidator(string dataType) { } public string DataType { get; } public virtual bool NoIncrementalValidation { get; } + public virtual bool ShouldRunAfterRemovingHiddenData { get; } public virtual string ValidationSource { get; } protected void AddValidationIssue(Altinn.App.Core.Models.Validation.ValidationIssue issue) { } protected void CreateValidationIssue(System.Linq.Expressions.Expression> selector, string textKey, Altinn.App.Core.Models.Validation.ValidationIssueSeverity severity = 1, string? description = null, string? code = null, System.Collections.Generic.Dictionary? customTextParameters = null) { } From 86cb25512b15c9830946733db1458038c64dcab6 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Thu, 18 Sep 2025 13:27:11 +0200 Subject: [PATCH 2/6] Follow coderabit suggestions --- .../Features/IInstanceDataAccessor.cs | 5 ++-- .../Internal/Data/PreviousDataAccessor.cs | 13 +++++++--- .../LayoutEvaluatorStateInitializer.cs | 8 +++--- .../Internal/Validation/ValidationService.cs | 6 +++-- .../Controllers/DataController_PatchTests.cs | 7 ------ .../Controllers/ProcessControllerTests.cs | 4 --- ...alidateController_ValidateInstanceTests.cs | 1 - .../Helpers/Patch/PatchServiceTests.cs | 1 - .../ValidationServiceTests.cs | 2 -- .../Validators/ValidationServiceTests.cs | 3 --- .../TestCleanDataAccessor.cs | 2 +- .../FullTests/DataAccessorFixture.cs | 25 ++++++++++++++++--- .../RequiredValidatorTests.cs | 7 +++--- .../RequiredValidator/mainLayout/page1.json | 4 +-- .../TestUtilities/InstanceDataAccessorFake.cs | 6 +++-- 15 files changed, 53 insertions(+), 41 deletions(-) diff --git a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs index 8534151073..0a1bb43fd1 100644 --- a/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Features/IInstanceDataAccessor.cs @@ -34,13 +34,14 @@ public interface IInstanceDataAccessor Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier); /// - /// Get a that provides access to the cleaned data (where all fields marked as "hidden" is removed). + /// Get a that provides access to the cleaned data (where all fields marked as "hidden" are removed). /// /// The strategy for "hiddenRow" on group components IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull); /// - /// Get a that provides access to the data before the current change. + /// Get a that provides access to the + /// storage persisted before any in-memory changes in this request. /// IInstanceDataAccessor GetPreviousDataAccessor(); diff --git a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs index 035c166562..617c959cb2 100644 --- a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs @@ -48,11 +48,16 @@ public PreviousDataAccessor( public async Task GetFormData(DataElementIdentifier dataElementIdentifier) { + var dataType = this.GetDataType(dataElementIdentifier); + if (dataType.AppLogic?.ClassRef is null) + { + throw new InvalidOperationException( + $"Data element {dataElementIdentifier.Id} is of data type {dataType.Id} which doesn't have app logic in application metadata and cant be used as form data" + ); + } + var binaryData = await _dataAccessor.GetBinaryData(dataElementIdentifier); - return _modelSerializationService.DeserializeFromStorage( - binaryData.Span, - this.GetDataType(dataElementIdentifier) - ); + return _modelSerializationService.DeserializeFromStorage(binaryData.Span, dataType); } public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index 14577180f9..b33d0c04cf 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -87,17 +87,19 @@ public Task GetFormDataWrapper(DataElementIdentifier dataEleme public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption) { - throw new NotImplementedException("Legacy data accessor does not implement GetCleanAccessorForTask"); + throw new NotSupportedException("Legacy single data accessor does not implement GetCleanAccessorForTask"); } public IInstanceDataAccessor GetPreviousDataAccessor() { - throw new NotImplementedException("Legacy data accessor does not implement GetPreviousDataAccessor"); + throw new NotSupportedException("Legacy single data accessor does not implement GetPreviousDataAccessor"); } public Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) { - return Task.FromException>(new NotImplementedException()); + return Task.FromException>( + new NotSupportedException("Legacy single data accessor does not implement GetBinaryData") + ); } public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index bff2da58d5..78a97e8a24 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -66,6 +66,8 @@ public async Task> ValidateInstanceAtTask( var cleanAccessor = dataAccessor; validators = validators.ToArray(); + // Initialize the clean accessor if any validator requires it + // or skip initialization if all validators can run with the complete data if (validators.Any(c => c.ShouldRunAfterRemovingHiddenData)) { cleanAccessor = dataAccessor.GetCleanAccessor(); @@ -217,8 +219,8 @@ IInstanceDataAccessor cleanAccessor DataType = fdc.DataType, Type = fdc.Type, - PreviousFormData = await previousAccessor.GetFormData(fdc.DataElementIdentifier), - CurrentFormData = await cleanAccessor.GetFormData(fdc.DataElementIdentifier), + PreviousFormDataWrapper = await previousAccessor.GetFormDataWrapper(fdc.DataElementIdentifier), + CurrentFormDataWrapper = await cleanAccessor.GetFormDataWrapper(fdc.DataElementIdentifier), // The binary data is kept as is, because logic is assumed to not use it CurrentBinaryData = fdc.CurrentBinaryData, PreviousBinaryData = fdc.PreviousBinaryData, diff --git a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs index a727489f87..766692755c 100644 --- a/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/DataController_PatchTests.cs @@ -65,7 +65,6 @@ public DataControllerPatchTests(WebApplicationFactory factory, ITestOut _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); - _formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); @@ -92,12 +91,6 @@ TResponse parsedResponse url += $"?language={language}"; } OutputHelper.WriteLine($"Calling PATCH {url}"); - using var httpClient = GetRootedClient(Org, App); - string token = TestAuthentication.GetUserToken(userId: 1337); - httpClient.DefaultRequestHeaders.Authorization = new AuthenticationHeaderValue( - AuthorizationSchemes.Bearer, - token - ); var serializedPatch = JsonSerializer.Serialize( new DataPatchRequest() { Patch = patch, IgnoredValidators = ignoredValidators }, _jsonSerializerOptions diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs index 54bf81ebc9..47ac5547d6 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.cs @@ -54,7 +54,6 @@ public ProcessControllerTests(WebApplicationFactory factory, ITestOutpu _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); - _formDataValidatorMock.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(false); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); @@ -255,7 +254,6 @@ public async Task RunProcessNext_FailingValidator_ReturnsValidationErrors() dataValidator.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); - dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); dataValidator .Setup(v => v.ValidateFormData( @@ -317,7 +315,6 @@ public async Task RunProcessNext_FailingValidator_Reject_ReturnsOk() dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); - dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); dataValidator .Setup(v => v.ValidateFormData( @@ -569,7 +566,6 @@ public async Task RunProcessNext_NonErrorValidations_ReturnsOk() dataValidator.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); dataValidator.Setup(v => v.DataType).Returns("*"); dataValidator.Setup(v => v.ValidationSource).Returns("test-source"); - dataValidator.SetupGet(v => v.NoIncrementalValidation).Returns(false); dataValidator .Setup(v => v.ValidateFormData( diff --git a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs index ac5d14513d..3af8765e20 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs +++ b/test/Altinn.App.Api.Tests/Controllers/ValidateController_ValidateInstanceTests.cs @@ -47,7 +47,6 @@ ITestOutputHelper outputHelper _formDataValidatorMock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(false); _formDataValidatorMock.Setup(v => v.DataType).Returns("9edd53de-f46f-40a1-bb4d-3efb93dc113d"); _formDataValidatorMock.Setup(v => v.ValidationSource).Returns("Not a valid validation source"); - _formDataValidatorMock.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(false); OverrideServicesForAllTests = (services) => { services.AddSingleton(_dataProcessorMock.Object); diff --git a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs index ce044714c1..194e696815 100644 --- a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs @@ -81,7 +81,6 @@ public PatchServiceTests() _formDataValidator.Setup(fdv => fdv.DataType).Returns(_dataType.Id); _formDataValidator.Setup(fdv => fdv.ValidationSource).Returns("formDataValidator"); _formDataValidator.Setup(fdv => fdv.HasRelevantChanges(It.IsAny(), It.IsAny())).Returns(true); - _formDataValidator.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(false); _dataElementValidator.Setup(dev => dev.DataType).Returns(_dataType.Id); _dataElementValidator.Setup(dev => dev.ValidationSource).Returns("dataElementValidator"); _dataElementValidator.SetupGet(fdv => fdv.NoIncrementalValidation).Returns(true); 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 46ce50b260..b3901f2c27 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs @@ -246,8 +246,6 @@ string validationSource // ValidatorName (used for source) formDataValidatorMock.Setup(v => v.ValidationSource).Returns(validationSource); - - formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); } private void SetupFormDataValidatorReturn( diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 6ab126f059..6cb5aa572c 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -327,7 +327,6 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() .SetupGet(v => v.ValidationSource) .Returns("FormDataValidatorNoAppLogic") .Verifiable(Times.AtLeastOnce); - formDataValidatorNoAppLogicMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); _services.AddSingleton(formDataValidatorNoAppLogicMock.Object); _appMetadata.DataTypes.Add(new DataType { Id = "dataTypeNoAppLogic", TaskId = TaskId }); @@ -345,7 +344,6 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() .SetupGet(v => v.ValidationSource) .Returns("FormDataValidatorWrongTask") .Verifiable(Times.AtLeastOnce); - formDataValidatorWrongTaskMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); _services.AddSingleton(formDataValidatorWrongTaskMock.Object); _appMetadata.DataTypes.Add( new DataType @@ -377,7 +375,6 @@ public async Task FormDataValidator_DataTypeNoAppLogic_IsNotCalled() }, } ); - formDataValidatorMock.SetupGet(v => v.NoIncrementalValidation).Returns(false); _services.AddSingleton(formDataValidatorMock.Object); _appMetadata.DataTypes.Add( new DataType diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs index 344ee55a2d..9dab33d04c 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs @@ -299,7 +299,7 @@ public async Task HideRowAndNameInSubComponent() { fixture.AddFormData(subData); } - await using var sp = fixture.ServiceCollection.BuildServiceProvider(); + await using var sp = fixture.BuildServiceProvider(); var dataUnitOfWorkInitializer = sp.GetRequiredService(); var dataMutator = await dataUnitOfWorkInitializer.Init( fixture.Instance, diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs index c08d99bb7a..32676b493c 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs @@ -55,6 +55,8 @@ public sealed class DataAccessorFixture public IServiceCollection ServiceCollection { get; } = new ServiceCollection(); + public ServiceProvider BuildServiceProvider() => ServiceCollection.BuildServiceProvider(validateScopes: true); + private DataAccessorFixture(ITestOutputHelper outputHelper) { AppMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(ApplicationMetadata); @@ -76,7 +78,6 @@ private DataAccessorFixture(ITestOutputHelper outputHelper) ServiceCollection.AddTransient(); ServiceCollection.AddTransient(); ServiceCollection.AddTransient(); - ServiceCollection.AddSingleton(TranslationServiceMock.Object); ServiceCollection.AddFakeLoggingWithXunit(outputHelper); AppResourcesMock .Setup(ar => ar.GetLayoutSet()) @@ -121,9 +122,15 @@ private async Task AddLayouts(List specs, string callerFilePath) foreach (var spec in specs) { + var layoutDir = Path.Join(directory, spec.LayoutSetName); + if (!Directory.Exists(layoutDir)) + { + throw new DirectoryNotFoundException($"Missing layout directory: {layoutDir}"); + } var pageNames = Directory - .GetFiles(Path.Join(directory, spec.LayoutSetName), "*.json") - .Select(Path.GetFileNameWithoutExtension); + .GetFiles(layoutDir, "*.json") + .Select(Path.GetFileNameWithoutExtension) + .OrderBy(n => n, StringComparer.OrdinalIgnoreCase); var pages = await Task.WhenAll( pageNames.Select(async pageName => @@ -184,7 +191,17 @@ public void AddFormData(object data, int? maxCount = null) Instance.Data.Add(dataElement); var serializationService = new ModelSerializationService(AppModelMock.Object); DataClientMock - .Setup(dc => dc.GetDataBytes(Org, App, InstanceOwnerPartyId, InstanceGuid, dataGuid, null, default)) + .Setup(dc => + dc.GetDataBytes( + Org, + App, + InstanceOwnerPartyId, + InstanceGuid, + dataGuid, + It.IsAny(), + It.IsAny() + ) + ) .ReturnsAsync(serializationService.SerializeToStorage(data, dataType).data.ToArray()); } } diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs index 190c11941c..69c8fdeb18 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/RequiredValidatorTests.cs @@ -1,5 +1,6 @@ using System.Text.Json.Serialization; using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers.DataModel; using Altinn.App.Core.Internal.Data; using Altinn.App.Core.Internal.Validation; using Altinn.App.Core.Models; @@ -135,7 +136,7 @@ private async Task VerifyValidationIssues(Model data, SubModel[] subDatas) } fixture.ServiceCollection.AddTransient(); - await using var sp = fixture.ServiceCollection.BuildServiceProvider(); + await using var sp = fixture.BuildServiceProvider(); var dataUnitOfWorkInitializer = sp.GetRequiredService(); var dataMutator = await dataUnitOfWorkInitializer.Init( @@ -154,8 +155,8 @@ private async Task VerifyValidationIssues(Model data, SubModel[] subDatas) DataElement = null, CurrentBinaryData = null, PreviousBinaryData = null, - CurrentFormData = data, - PreviousFormData = new Model(), + CurrentFormDataWrapper = FormDataWrapperFactory.Create(data), + PreviousFormDataWrapper = FormDataWrapperFactory.Create(new Model()), Type = ChangeType.Created, }, ] diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json index 76f8cd18ad..58d6053f02 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/RequiredValidator/mainLayout/page1.json @@ -32,7 +32,7 @@ } }, { - "id": "input-hidden-requried", + "id": "input-hidden-required", "type": "Input", "required": true, "hidden": true, @@ -202,4 +202,4 @@ } ] } -} \ No newline at end of file +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs index ea7da2e14e..8b8988405e 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/TestUtilities/InstanceDataAccessorFake.cs @@ -82,12 +82,14 @@ public Task GetFormDataWrapper(DataElementIdentifier dataEleme public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) { - throw new NotImplementedException(); + throw new NotImplementedException("GetCleanAccessor is not yet implemented for InstanceDataAccessorFake"); } public IInstanceDataAccessor GetPreviousDataAccessor() { - throw new NotImplementedException(); + throw new NotImplementedException( + "GetPreviousDataAccessor is not yet implemented for InstanceDataAccessorFake" + ); } public Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) From d4592c099fec4ae50de70954ba4781dd92379ed8 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Sun, 21 Sep 2025 09:29:00 +0200 Subject: [PATCH 3/6] More coderabit advice and add more tests --- Directory.Packages.props | 1 + .../Features/IFormDataValidator.cs | 16 +- .../Telemetry/Telemetry.Validation.cs | 3 +- .../Features/Telemetry/Telemetry.cs | 1 + .../Data/CleanInstanceDataAccessor.cs | 37 +- .../Internal/Data/PreviousDataAccessor.cs | 36 +- .../LayoutEvaluatorStateInitializer.cs | 2 +- .../Internal/Validation/ValidationService.cs | 18 +- .../Altinn.App.Core.Tests.csproj | 1 + ...lidatorFunctionForIncremental.verified.txt | 3 + ...CallsValidatorFunctionForTask.verified.txt | 3 + .../FullTests/CleanDataAccessor/MainModel.cs | 51 +++ .../TestCleanDataAccessor.cs | 53 +-- ...stValidateCleanData.CleanFull.verified.txt | 104 +++++ ...ateCleanData.CleanIncremental.verified.txt | 407 +++++++++++++++++ ...stValidateCleanData.DirtyFull.verified.txt | 105 +++++ ...ateCleanData.DirtyIncremental.verified.txt | 432 ++++++++++++++++++ .../TestValidateCleanData.cs | 192 ++++++++ .../FullTests/DataAccessorFixture.cs | 1 - 19 files changed, 1373 insertions(+), 93 deletions(-) create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/MainModel.cs create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt create mode 100644 test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs diff --git a/Directory.Packages.props b/Directory.Packages.props index c2f185ffbd..bba4a14a78 100644 --- a/Directory.Packages.props +++ b/Directory.Packages.props @@ -55,6 +55,7 @@ + diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs index bf7d434d86..0c74116b5d 100644 --- a/src/Altinn.App.Core/Features/IFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -30,16 +30,16 @@ public interface IFormDataValidator /// string ValidationSource => $"{this.GetType().FullName}-{DataType}"; - /// - /// If you override this to return true, the validator will only run on process/next, and not continuously. - /// will never get called - /// - /// + /// + /// + /// will run on incremental changes using . + /// bool NoIncrementalValidation => false; - /// - /// Proxy for - /// + /// + /// + /// By default will run on the full data + /// bool ShouldRunAfterRemovingHiddenData => false; /// diff --git a/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs b/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs index b309f800ff..a5678dba1f 100644 --- a/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs +++ b/src/Altinn.App.Core/Features/Telemetry/Telemetry.Validation.cs @@ -47,7 +47,8 @@ private static void InitValidation(InitContext context) ActivitySource .StartActivity($"{Prefix}.RunValidator") ?.SetTag(InternalLabels.ValidatorType, validator.GetType().Name) - .SetTag(InternalLabels.ValidatorSource, validator.ValidationSource); + .SetTag(InternalLabels.ValidatorSource, validator.ValidationSource) + .SetTag(InternalLabels.ValidatorRemoveHiddenData, validator.ShouldRunAfterRemovingHiddenData); internal static class Validation { diff --git a/src/Altinn.App.Core/Features/Telemetry/Telemetry.cs b/src/Altinn.App.Core/Features/Telemetry/Telemetry.cs index 9f6c218167..5570042179 100644 --- a/src/Altinn.App.Core/Features/Telemetry/Telemetry.cs +++ b/src/Altinn.App.Core/Features/Telemetry/Telemetry.cs @@ -228,6 +228,7 @@ internal static class InternalLabels internal const string AuthorizerTaskId = "authorization.authorizer.task.id"; internal const string ValidatorType = "validator.type"; internal const string ValidatorSource = "validator.source"; + internal const string ValidatorRemoveHiddenData = "validator.remove_hidden_data"; internal const string ValidatorHasRelevantChanges = "validator.has_relevant_changes"; internal const string ValidatorChangedElementsIds = "validator.changed_elements_ids"; internal const string ValidatorIssueCount = "validation.issue_count"; diff --git a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs index 4d8db46703..8e1c565573 100644 --- a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs @@ -39,22 +39,29 @@ public CleanInstanceDataAccessor( _rowRemovalOption = rowRemovalOption; _language = language; _telemetry = telemetry; + _translationService = translationService; LayoutModel? layouts = taskId is not null ? appResources.GetLayoutModelForTask(taskId) : null; - var state = new LayoutEvaluatorState( - dataAccessor, - layouts, - translationService, - frontEndSettings, - gatewayAction: null, - language - ); - _hiddenFieldsTask = new(() => + if (layouts is null) { - using var activity = telemetry?.StartRemoveHiddenDataForValidation(); - return LayoutEvaluator.GetHiddenFieldsForRemoval(state); - }); - _translationService = translationService; + _hiddenFieldsTask = new(() => Task.FromResult(new List())); + } + else + { + var state = new LayoutEvaluatorState( + dataAccessor, + layouts, + translationService, + frontEndSettings, + gatewayAction: null, + language + ); + _hiddenFieldsTask = new(() => + { + using var activity = telemetry?.StartRemoveHiddenDataForValidation(); + return LayoutEvaluator.GetHiddenFieldsForRemoval(state); + }); + } } private readonly DataElementCache _cleanCache = new(); @@ -72,7 +79,7 @@ public async Task GetFormData(DataElementIdentifier dataElementIdentifie public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) { - return await _cleanCache.GetOrCreate( + var dataWrapper = await _cleanCache.GetOrCreate( dataElementIdentifier, async () => { @@ -81,6 +88,8 @@ public async Task GetFormDataWrapper(DataElementIdentifier dat return CleanModel(data.Copy(), dataElementIdentifier, hiddenFields, _rowRemovalOption); } ); + + return dataWrapper.Copy(); } private static IFormDataWrapper CleanModel( diff --git a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs index 617c959cb2..e9baa97667 100644 --- a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs @@ -1,3 +1,4 @@ +using System.Collections.Concurrent; using Altinn.App.Core.Configuration; using Altinn.App.Core.Features; using Altinn.App.Core.Helpers; @@ -21,6 +22,8 @@ internal class PreviousDataAccessor : IInstanceDataAccessor private readonly ITranslationService _translationService; private readonly Telemetry? _telemetry; + private ConcurrentDictionary> _previousDataCache = new(); + public PreviousDataAccessor( IInstanceDataAccessor dataAccessor, string? taskId, @@ -48,22 +51,31 @@ public PreviousDataAccessor( public async Task GetFormData(DataElementIdentifier dataElementIdentifier) { - var dataType = this.GetDataType(dataElementIdentifier); - if (dataType.AppLogic?.ClassRef is null) - { - throw new InvalidOperationException( - $"Data element {dataElementIdentifier.Id} is of data type {dataType.Id} which doesn't have app logic in application metadata and cant be used as form data" - ); - } - - var binaryData = await _dataAccessor.GetBinaryData(dataElementIdentifier); - return _modelSerializationService.DeserializeFromStorage(binaryData.Span, dataType); + return (await GetFormDataWrapper(dataElementIdentifier)).BackingData(); } public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) { - var dataModel = await GetFormData(dataElementIdentifier); - return FormDataWrapperFactory.Create(dataModel); + var data = await _previousDataCache.GetOrAdd( + dataElementIdentifier, + async id => + { + var dataType = this.GetDataType(id); + if (dataType.AppLogic?.ClassRef is null) + { + throw new InvalidOperationException( + $"Data element {id.Id} is of data type {dataType.Id} which doesn't have app logic in application metadata and cant be used as form data" + ); + } + + var binaryData = await _dataAccessor.GetBinaryData(id); + return FormDataWrapperFactory.Create( + _modelSerializationService.DeserializeFromStorage(binaryData.Span, dataType) + ); + } + ); + + return data.Copy(); } public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption = RowRemovalOption.SetToNull) diff --git a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs index b33d0c04cf..0eddc67bdd 100644 --- a/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs +++ b/src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorStateInitializer.cs @@ -87,7 +87,7 @@ public Task GetFormDataWrapper(DataElementIdentifier dataEleme public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption) { - throw new NotSupportedException("Legacy single data accessor does not implement GetCleanAccessorForTask"); + throw new NotSupportedException("Legacy single data accessor does not implement GetCleanAccessor"); } public IInstanceDataAccessor GetPreviousDataAccessor() diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index 78a97e8a24..195c997beb 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -61,7 +61,7 @@ public async Task> ValidateInstanceAtTask( // Remove ignored validators if (ignoredValidators is not null) validators = validators.Where(v => - !ignoredValidators.Contains(v.ValidationSource, StringComparer.InvariantCulture) + !ignoredValidators.Contains(v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) ); var cleanAccessor = dataAccessor; @@ -130,7 +130,12 @@ public async Task> ValidateIncrementalFormData( var validators = _validatorFactory .GetValidators(taskId) - .Where(v => !v.NoIncrementalValidation && !(ignoredValidators?.Contains(v.ValidationSource) ?? false)) + .Where(v => + !v.NoIncrementalValidation + && !( + ignoredValidators?.Contains(v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) ?? false + ) + ) .ToArray(); ThrowIfDuplicateValidators(validators, taskId); @@ -209,7 +214,7 @@ IInstanceDataAccessor cleanAccessor foreach (var change in changes.AllChanges) { // Clean FormDataChange updates but keep other changes as is - if (change is FormDataChange { DataElement: not null, Type: ChangeType.Updated } fdc) + if (change is FormDataChange { DataElement: not null } fdc) { cleanedChangeList.Add( new FormDataChange() @@ -264,9 +269,14 @@ private static void ThrowIfDuplicateValidators(IValidator[] validators, string t .Distinct(StringComparer.InvariantCultureIgnoreCase); if (sourceNames.Count() != validators.Length) { + var duplicates = validators + .GroupBy(v => v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) + .Where(g => g.Count() > 1) + .Select(g => g.Key); + var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); throw new InvalidOperationException( - $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{sources}" + $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{string.Join(", ", duplicates)}" ); } } diff --git a/test/Altinn.App.Core.Tests/Altinn.App.Core.Tests.csproj b/test/Altinn.App.Core.Tests/Altinn.App.Core.Tests.csproj index af3721cbf5..acad206e8b 100644 --- a/test/Altinn.App.Core.Tests/Altinn.App.Core.Tests.csproj +++ b/test/Altinn.App.Core.Tests/Altinn.App.Core.Tests.csproj @@ -20,6 +20,7 @@ + diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForIncremental.verified.txt b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForIncremental.verified.txt index b400c57e3e..307e3a0f36 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForIncremental.verified.txt +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForIncremental.verified.txt @@ -11,6 +11,9 @@ { validator.has_relevant_changes: true }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+GenericValidatorFake-default }, diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForTask.verified.txt b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForTask.verified.txt index e3dddbfbec..f85a5ed525 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForTask.verified.txt +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.GenericFormDataValidator_serviceModelIsString_CallsValidatorFunctionForTask.verified.txt @@ -8,6 +8,9 @@ { validation.issue_count: 1 }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Tests.Features.Validators.ValidationServiceTests+GenericValidatorFake-default }, diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/MainModel.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/MainModel.cs new file mode 100644 index 0000000000..b1cbc4db24 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/MainModel.cs @@ -0,0 +1,51 @@ +using System.Text.Json.Serialization; + +namespace Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor; + +public record MainModel +{ + public bool? HideMainTitle { get; set; } + public string? MainTitle { get; set; } + public string? UnboundField { get; set; } + public bool? HidePage1 { get; set; } + + public bool? HideMainComponentGroup { get; set; } + public List? MainComponentGroup { get; set; } + + public bool? HideSubLayout { get; set; } + + public record MainComponentGroupItem + { + [JsonPropertyName("altinnRowId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Guid AltinnRowId { get; set; } + + public string? Name { get; set; } + public bool? HideRow { get; set; } + public string? Description { get; set; } + public bool? HideName { get; set; } + } +} + +public record SubModel +{ + public bool? HideSubPage { get; set; } + public bool? HideSubPageTitle { get; set; } + public string? SubPageTitle { get; set; } + public string? UnboundField { get; set; } + + public bool? HideSubComponentGroup { get; set; } + public List? SubComponentGroup { get; set; } + + public record SubGroup + { + [JsonPropertyName("altinnRowId")] + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] + public Guid AltinnRowId { get; set; } + + public bool? HideRow { get; set; } + public bool? HideName { get; set; } + public string? Name { get; set; } + public string? Description { get; set; } + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs index 9dab33d04c..8bdef9cbe5 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs @@ -1,5 +1,4 @@ using System.Text.Json; -using System.Text.Json.Serialization; using Altinn.App.Core.Features; using Altinn.App.Core.Internal.Data; using Microsoft.Extensions.DependencyInjection; @@ -16,53 +15,6 @@ public TestCleanDataAccessor(ITestOutputHelper outputHelper) _outputHelper = outputHelper; } - public record SubModel - { - public bool? HideSubPage { get; set; } - public bool? HideSubPageTitle { get; set; } - public string? SubPageTitle { get; set; } - public string? UnboundField { get; set; } - - public bool? HideSubComponentGroup { get; set; } - public List? SubComponentGroup { get; set; } - - public record SubGroup - { - [JsonPropertyName("altinnRowId")] - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] - public Guid AltinnRowId { get; set; } - - public bool? HideRow { get; set; } - public bool? HideName { get; set; } - public string? Name { get; set; } - public string? Description { get; set; } - } - } - - public record MainModel - { - public bool? HideMainTitle { get; set; } - public string? MainTitle { get; set; } - public string? UnboundField { get; set; } - public bool? HidePage1 { get; set; } - - public bool? HideMainComponentGroup { get; set; } - public List? MainComponentGroup { get; set; } - - public bool? HideSubLayout { get; set; } - - public record MainComponentGroupItem - { - [JsonPropertyName("altinnRowId")] - [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingDefault)] - public Guid AltinnRowId { get; set; } - public string? Name { get; set; } - public bool? HideRow { get; set; } - public string? Description { get; set; } - public bool? HideName { get; set; } - } - } - [Fact] public async Task TestEverythingEmpty() { @@ -104,7 +56,7 @@ public async Task TestEverythingHidden() var subData = new SubModel(); - var expectedModel = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(data)); + var expectedModel = JsonSerializer.Deserialize(JsonSerializer.SerializeToUtf8Bytes(data))!; var (cleanModel, cleanSubModels) = await GetMainAndSubClean(data, [subData]); var cleanSubModel = cleanSubModels.Single(); // Assert @@ -112,11 +64,8 @@ public async Task TestEverythingHidden() Assert.NotSame(cleanSubModel, subData); // Make expected changes - // ReSharper disable PossibleNullReferenceException -#nullable disable expectedModel.MainTitle = null; expectedModel.MainComponentGroup = null; -#nullable restore Assert.Equivalent(expectedModel, cleanModel); } diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt new file mode 100644 index 0000000000..b95b554a0c --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt @@ -0,0 +1,104 @@ +[ + { + Method: IValidator.ShouldRunForTask(string taskId), + Arguments: { + Arguments: { + taskId: Task_1-access + } + }, + ReturnValue: true + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: true + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: true + }, + { + Method: IValidator.Validate(IInstanceDataAccessor dataAccessor, string taskId, string language), + Arguments: { + Arguments: { + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_1, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_2, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_3, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ] + }, + language: test-language, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion + } + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + } +] \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt new file mode 100644 index 0000000000..87b3e02c8d --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt @@ -0,0 +1,407 @@ +[ + { + Method: IValidator.ShouldRunForTask(string taskId), + Arguments: { + Arguments: { + taskId: Task_1-access + } + }, + ReturnValue: true + }, + { + Method: IValidator.get_NoIncrementalValidation(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: true + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: true + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: true + }, + { + Method: IValidator.HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes), + Arguments: { + Arguments: { + changes: { + AllChanges: [ + { + PreviousFormData: { + HideMainTitle: false, + MainTitle: Main Title, + HidePage1: false, + HideMainComponentGroup: false, + MainComponentGroup: [ + { + Name: row1, + HideRow: false, + Description: row1 description, + HideName: false + }, + null, + { + HideRow: false, + Description: row3 description, + HideName: true + }, + null, + { + Name: row5, + HideRow: false + } + ], + HideSubLayout: false + }, + PreviousFormDataWrapper: { + BackingDataType: MainModel + }, + CurrentFormData: { + HideMainTitle: true, + HidePage1: false, + HideMainComponentGroup: false, + MainComponentGroup: [ + { + AltinnRowId: Guid_1, + Name: row1, + HideRow: false, + Description: row1 description, + HideName: false + }, + null, + { + AltinnRowId: Guid_2, + HideRow: false, + Description: row3 description, + HideName: true + }, + null, + { + AltinnRowId: Guid_3, + Name: row5, + HideRow: false + } + ], + HideSubLayout: false + }, + CurrentFormDataWrapper: { + BackingDataType: MainModel + }, + PreviousBinaryData: { + Length: 1411, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 1410, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_4, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_4, + Id: Guid_4, + DataTypeId: mainLayout_dataType + }, + DataType: { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + }, + { + PreviousFormData: { + SubPageTitle: , + UnboundField: unbound1 + }, + PreviousFormDataWrapper: { + BackingDataType: SubModel + }, + CurrentFormData: { + HideSubPageTitle: true, + UnboundField: unbound1 + }, + CurrentFormDataWrapper: { + BackingDataType: SubModel + }, + PreviousBinaryData: { + Length: 337, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 343, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_5, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_5, + Id: Guid_5, + DataTypeId: subLayout_dataType + }, + DataType: { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + }, + { + PreviousFormData: { + HideSubPageTitle: false, + SubPageTitle: sub2 title, + SubComponentGroup: [ + { + Name: subGroup1 + } + ] + }, + PreviousFormDataWrapper: { + BackingDataType: SubModel + }, + CurrentFormData: { + HideSubPageTitle: true, + SubComponentGroup: [ + { + AltinnRowId: Guid_6, + Name: subGroup1 + } + ] + }, + CurrentFormDataWrapper: { + BackingDataType: SubModel + }, + PreviousBinaryData: { + Length: 507, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 506, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_7, + Id: Guid_7, + DataTypeId: subLayout_dataType + }, + DataType: { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + } + ] + }, + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_4, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_5, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ] + }, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion, + Result: true + } + }, + { + Method: IValidator.Validate(IInstanceDataAccessor dataAccessor, string taskId, string language), + Arguments: { + Arguments: { + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_4, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_5, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ] + }, + language: test-language, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion + } + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + } +] \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt new file mode 100644 index 0000000000..fcd15ea44b --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt @@ -0,0 +1,105 @@ +[ + { + Method: IValidator.ShouldRunForTask(string taskId), + Arguments: { + Arguments: { + taskId: Task_1-access + } + }, + ReturnValue: true + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.Validate(IInstanceDataAccessor dataAccessor, string taskId, string language), + Arguments: { + Arguments: { + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_1, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_2, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_3, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ], + HasAbandonIssues: false + }, + language: test-language, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion + } + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + } +] \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt new file mode 100644 index 0000000000..6f8540a692 --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt @@ -0,0 +1,432 @@ +[ + { + Method: IValidator.ShouldRunForTask(string taskId), + Arguments: { + Arguments: { + taskId: Task_1-access + } + }, + ReturnValue: true + }, + { + Method: IValidator.get_NoIncrementalValidation(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), + Arguments: {}, + ReturnValue: false + }, + { + Method: IValidator.HasRelevantChanges(IInstanceDataAccessor dataAccessor, string taskId, DataElementChanges changes), + Arguments: { + Arguments: { + changes: { + AllChanges: [ + { + PreviousFormData: { + HideMainTitle: false, + MainTitle: Main Title, + HidePage1: false, + HideMainComponentGroup: false, + MainComponentGroup: [ + { + Name: row1, + HideRow: false, + Description: row1 description, + HideName: false + }, + { + Name: row2, + HideRow: true, + Description: row2 description, + HideName: false + }, + { + Name: row3, + HideRow: false, + Description: row3 description, + HideName: true + }, + { + Name: row4, + HideRow: true + }, + { + Name: row5, + HideRow: false + } + ], + HideSubLayout: false + }, + PreviousFormDataWrapper: { + BackingDataType: MainModel + }, + CurrentFormData: { + HideMainTitle: true, + MainTitle: Main Title, + HidePage1: false, + HideMainComponentGroup: false, + MainComponentGroup: [ + { + AltinnRowId: Guid_1, + Name: row1, + HideRow: false, + Description: row1 description, + HideName: false + }, + { + AltinnRowId: Guid_2, + Name: row2, + HideRow: true, + Description: row2 description, + HideName: false + }, + { + AltinnRowId: Guid_3, + Name: row3, + HideRow: false, + Description: row3 description, + HideName: true + }, + { + AltinnRowId: Guid_4, + Name: row4, + HideRow: true + }, + { + AltinnRowId: Guid_5, + Name: row5, + HideRow: false + } + ], + HideSubLayout: false + }, + CurrentFormDataWrapper: { + BackingDataType: MainModel + }, + PreviousBinaryData: { + Length: 1411, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 1410, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_6, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_6, + Id: Guid_6, + DataTypeId: mainLayout_dataType + }, + DataType: { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + }, + { + PreviousFormData: { + SubPageTitle: , + UnboundField: unbound1 + }, + PreviousFormDataWrapper: { + BackingDataType: SubModel + }, + CurrentFormData: { + HideSubPageTitle: true, + SubPageTitle: , + UnboundField: unbound1 + }, + CurrentFormDataWrapper: { + BackingDataType: SubModel + }, + PreviousBinaryData: { + Length: 337, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 343, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_7, + Id: Guid_7, + DataTypeId: subLayout_dataType + }, + DataType: { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + }, + { + PreviousFormData: { + HideSubPageTitle: false, + SubPageTitle: sub2 title, + SubComponentGroup: [ + { + Name: subGroup1 + } + ] + }, + PreviousFormDataWrapper: { + BackingDataType: SubModel + }, + CurrentFormData: { + HideSubPageTitle: true, + SubPageTitle: sub2 title, + SubComponentGroup: [ + { + AltinnRowId: Guid_8, + Name: subGroup1 + } + ] + }, + CurrentFormDataWrapper: { + BackingDataType: SubModel + }, + PreviousBinaryData: { + Length: 507, + IsEmpty: false + }, + CurrentBinaryData: { + Length: 506, + IsEmpty: false + }, + Type: Updated, + DataElement: { + Id: Guid_9, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + DataElementIdentifier: { + Guid: Guid_9, + Id: Guid_9, + DataTypeId: subLayout_dataType + }, + DataType: { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + } + ] + }, + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_6, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_9, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ], + HasAbandonIssues: false + }, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion, + Result: true + } + }, + { + Method: IValidator.Validate(IInstanceDataAccessor dataAccessor, string taskId, string language), + Arguments: { + Arguments: { + dataAccessor: { + Instance: { + Id: 1337/00000000-babe-0000-0000-000000000001, + InstanceOwner: { + PartyId: 1337 + }, + Data: [ + { + Id: Guid_6, + DataType: mainLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_7, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + }, + { + Id: Guid_9, + DataType: subLayout_dataType, + Locked: false, + IsRead: true + } + ] + }, + DataTypes: [ + { + Id: mainLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.MainModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + }, + { + Id: subLayout_dataType, + AppLogic: { + ClassRef: Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor.SubModel, + AllowAnonymousOnStateless: false, + AutoDeleteOnProcessEnd: false, + DisallowUserCreate: false, + DisallowUserDelete: false + }, + TaskId: Task_1-access, + MaxCount: 0, + MinCount: 0, + EnablePdfCreation: true, + EnableFileScan: false, + ValidationErrorOnPendingFileScan: false + } + ], + HasAbandonIssues: false + }, + language: test-language, + taskId: Task_1-access + } + }, + ReturnValue: { + Status: RanToCompletion + } + }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + } +] \ No newline at end of file diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs new file mode 100644 index 0000000000..6b1c0a497a --- /dev/null +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs @@ -0,0 +1,192 @@ +using System.Diagnostics; +using Altinn.App.Core.Features; +using Altinn.App.Core.Internal.Data; +using Altinn.App.Core.Internal.Validation; +using Altinn.App.Core.Models; +using Altinn.App.Core.Models.Validation; +using Microsoft.Extensions.DependencyInjection; +using Moq; +using Xunit.Abstractions; + +namespace Altinn.App.Core.Tests.LayoutExpressions.FullTests.CleanDataAccessor; + +public class TestValidateCleanData(ITestOutputHelper outputHelper) +{ + [Fact] + public async Task CleanIncremental() + { + await RunTest(incremental: true, removeHidden: true); + } + + [Fact] + public async Task CleanFull() + { + await RunTest(incremental: false, removeHidden: true); + } + + [Fact] + public async Task DirtyIncremental() + { + await RunTest(incremental: true, removeHidden: false); + } + + [Fact] + public async Task DirtyFull() + { + await RunTest(incremental: false, removeHidden: false); + } + + private async Task RunTest(bool incremental, bool removeHidden) + { + var data = new MainModel() + { + HideMainComponentGroup = false, + HideMainTitle = false, // This is changed in the diff + HidePage1 = false, + HideSubLayout = false, + MainTitle = "Main Title", + MainComponentGroup = + [ + new() + { + Name = "row1", + Description = "row1 description", + HideName = false, + HideRow = false, + }, + new() + { + Name = "row2", + Description = "row2 description", + HideName = false, + HideRow = true, + }, + new() + { + Name = "row3", + Description = "row3 description", + HideName = true, + HideRow = false, + }, + new() { Name = "row4", HideRow = true }, + new() { Name = "row5", HideRow = false }, + ], + }; + var subData1 = new SubModel() + { + SubPageTitle = "", + UnboundField = "unbound1", + // TODO: add more data + }; + var subData2 = new SubModel() + { + HideSubPageTitle = false, + SubPageTitle = "sub2 title", + SubComponentGroup = [new() { Name = "subGroup1" }], + }; + + var validatorMock = GetValidatorMock( + shouldRunAfterRemovingHiddenData: removeHidden, + noIncrementalValidation: false + ); + + var fixture = await DataAccessorFixture.CreateAsync( + [new("mainLayout", typeof(MainModel), MaxCount: 1), new("subLayout", typeof(SubModel), MaxCount: 0)], + outputHelper + ); + fixture.AddFormData(data); + fixture.AddFormData(subData1); + fixture.AddFormData(subData2); + + fixture.ServiceCollection.AddSingleton(validatorMock.Object); + + await using var sp = fixture.BuildServiceProvider(); + + var dataUnitOfWorkInitializer = sp.GetRequiredService(); + var dataMutator = await dataUnitOfWorkInitializer.Init( + fixture.Instance, + DataAccessorFixture.TaskId, + "test-language" + ); + + var validationService = sp.GetRequiredService(); + + var element = await dataMutator.GetFormData(); + Assert.NotNull(element); + element.HideMainTitle = true; + var subElements = await dataMutator.GetAllFormData(); + foreach (var subElement in subElements) + { + subElement.HideSubPageTitle = true; + } + //TODO: add changes + + var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); + if (incremental) + { + await validationService.ValidateIncrementalFormData( + dataMutator, + DataAccessorFixture.TaskId, + changes, + [], + "test-language" + ); + } + else + { + await validationService.ValidateInstanceAtTask( + dataMutator, + DataAccessorFixture.TaskId, + [], + null, + "test-language" + ); + } + await Verify(validatorMock) + .IgnoreMember(nameof(DataElementChanges.FormDataChanges)) // Also included in AllChanges + .IgnoreMember(nameof(DataElementChanges.BinaryDataChanges)) // Also included in AllChanges + .AddNamedGuid(DataAccessorFixture.InstanceGuid, "instanceGuid"); + } + + private static Mock GetValidatorMock( + bool shouldRunAfterRemovingHiddenData, + bool noIncrementalValidation + ) + { + var mock = new Mock(MockBehavior.Strict); + mock.Setup(v => v.Validate(It.IsAny(), It.IsAny(), It.IsAny())) + .ReturnsAsync(new List()); + mock.SetupGet(v => v.ShouldRunAfterRemovingHiddenData).Returns(shouldRunAfterRemovingHiddenData); + mock.SetupGet(v => v.NoIncrementalValidation).Returns(noIncrementalValidation); + mock.Setup(v => v.ShouldRunForTask(DataAccessorFixture.TaskId)).Returns(true); + mock.SetupGet(v => v.ValidationSource).Returns("mockValidator"); + if (!noIncrementalValidation) + { + mock.Setup(v => + v.HasRelevantChanges( + It.IsAny(), + It.IsAny(), + It.IsAny() + ) + ) + .ReturnsAsync(true); + } + else + { + mock.Setup(v => + v.HasRelevantChanges( + It.IsAny(), + It.IsAny(), + It.IsAny() + ) + ) + .Throws( + new UnreachableException( + "HasRelevantChanges should never be called when not using incremental validation" + ) + ); + } + + return mock; + } +} diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs index 32676b493c..3d2e38a8cc 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs @@ -72,7 +72,6 @@ private DataAccessorFixture(ITestOutputHelper outputHelper) ServiceCollection.AddSingleton(DataElementAccessCheckerMock.Object); ServiceCollection.AddSingleton(); ServiceCollection.AddSingleton(); - ServiceCollection.AddFakeLoggingWithXunit(outputHelper); ServiceCollection.AddTransient(); ServiceCollection.AddTransient(); ServiceCollection.AddTransient(); From d4f0639e989d37fc750c57bd544e35ab9d96438e Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Sun, 21 Sep 2025 09:49:32 +0200 Subject: [PATCH 4/6] Fix broken tests --- .../Internal/Data/InstanceDataUnitOfWork.cs | 11 +++++++++- ...dator_ReturnsValidationErrors.verified.txt | 21 +++++++++++++++++++ ...sNext_PdfFails_DataIsUnlocked.verified.txt | 18 ++++++++++++++++ 3 files changed, 49 insertions(+), 1 deletion(-) diff --git a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs index 6d65eeaff9..4a0ff5c4bf 100644 --- a/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs +++ b/src/Altinn.App.Core/Internal/Data/InstanceDataUnitOfWork.cs @@ -135,9 +135,17 @@ public IInstanceDataAccessor GetCleanAccessor(RowRemovalOption rowRemovalOption ); } + // Non thread safe cache, because the previous data is always the same. + private PreviousDataAccessor? _previousDataAccessorCache; + public IInstanceDataAccessor GetPreviousDataAccessor() { - return new PreviousDataAccessor( + if (_previousDataAccessorCache is not null) + { + return _previousDataAccessorCache; + } + + _previousDataAccessorCache = new PreviousDataAccessor( this, _taskId, _appResources, @@ -147,6 +155,7 @@ public IInstanceDataAccessor GetPreviousDataAccessor() _language, _telemetry ); + return _previousDataAccessorCache; } /// diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_FailingValidator_ReturnsValidationErrors.verified.txt b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_FailingValidator_ReturnsValidationErrors.verified.txt index 429328b5d5..60baf06638 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_FailingValidator_ReturnsValidationErrors.verified.txt +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_FailingValidator_ReturnsValidationErrors.verified.txt @@ -206,6 +206,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Required }, @@ -222,6 +225,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Expression }, @@ -238,6 +244,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Features.Validation.Default.DefaultTaskValidator-* }, @@ -254,6 +263,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Features.Validation.Default.DefaultDataElementValidator-* }, @@ -270,6 +282,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: DataAnnotations }, @@ -286,6 +301,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Not a valid validation source }, @@ -302,6 +320,9 @@ { validation.issue_count: 1 }, + { + validator.remove_hidden_data: false + }, { validator.source: test-source }, diff --git a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_PdfFails_DataIsUnlocked.verified.txt b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_PdfFails_DataIsUnlocked.verified.txt index 939951d1f3..d1d3f84aec 100644 --- a/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_PdfFails_DataIsUnlocked.verified.txt +++ b/test/Altinn.App.Api.Tests/Controllers/ProcessControllerTests.RunProcessNext_PdfFails_DataIsUnlocked.verified.txt @@ -389,6 +389,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Required }, @@ -405,6 +408,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Expression }, @@ -421,6 +427,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Features.Validation.Default.DefaultTaskValidator-* }, @@ -437,6 +446,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Altinn.App.Core.Features.Validation.Default.DefaultDataElementValidator-* }, @@ -453,6 +465,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: DataAnnotations }, @@ -469,6 +484,9 @@ { validation.issue_count: 0 }, + { + validator.remove_hidden_data: false + }, { validator.source: Not a valid validation source }, From 916853687126a0176d71c92f8a463f7e61eec69c Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Sun, 21 Sep 2025 12:02:27 +0200 Subject: [PATCH 5/6] Another batch of coderabbit suggestions --- .../Features/IFormDataValidator.cs | 3 +- src/Altinn.App.Core/Features/IValidator.cs | 6 +-- .../Data/CleanInstanceDataAccessor.cs | 4 +- .../Internal/Data/DataElementCache.cs | 2 +- .../Internal/Data/PreviousDataAccessor.cs | 38 ++++++++++--------- .../Internal/Validation/IValidatorFactory.cs | 19 ++++++++++ .../Internal/Validation/ValidationService.cs | 27 +------------ .../TestCleanDataAccessor.cs | 2 +- .../TestValidateCleanData.cs | 8 +--- .../FullTests/DataAccessorFixture.cs | 10 +++-- 10 files changed, 57 insertions(+), 62 deletions(-) diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs index 0c74116b5d..7caa3f2eec 100644 --- a/src/Altinn.App.Core/Features/IFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -38,7 +38,8 @@ public interface IFormDataValidator /// /// - /// By default will run on the full data + /// Defaults to full data. When true, the pipeline provides a cleaned data accessor to + /// both and for consistent visibility. /// bool ShouldRunAfterRemovingHiddenData => false; diff --git a/src/Altinn.App.Core/Features/IValidator.cs b/src/Altinn.App.Core/Features/IValidator.cs index 31b27c2603..17c463311f 100644 --- a/src/Altinn.App.Core/Features/IValidator.cs +++ b/src/Altinn.App.Core/Features/IValidator.cs @@ -40,11 +40,11 @@ public interface IValidator bool NoIncrementalValidation => false; /// - /// Indicates whether the validator should execute after removing data elements marked as hidden. + /// Indicates whether this validator should run against a cleaned view of the data where fields marked as hidden are removed. /// /// - /// The default implementation returns false. Override this property in implementations if validation - /// needs to execute after hidden data removal. + /// Defaults to false. When true, the validation pipeline will supply a cleaned accessor for both + /// and , ensuring consistent visibility semantics. /// bool ShouldRunAfterRemovingHiddenData => false; diff --git a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs index 8e1c565573..ab572c7ea7 100644 --- a/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/CleanInstanceDataAccessor.cs @@ -83,8 +83,8 @@ public async Task GetFormDataWrapper(DataElementIdentifier dat dataElementIdentifier, async () => { - var data = await _dataAccessor.GetFormDataWrapper(dataElementIdentifier); - var hiddenFields = await _hiddenFieldsTask.Value; + var data = await _dataAccessor.GetFormDataWrapper(dataElementIdentifier).ConfigureAwait(false); + var hiddenFields = await _hiddenFieldsTask.Value.ConfigureAwait(false); return CleanModel(data.Copy(), dataElementIdentifier, hiddenFields, _rowRemovalOption); } ); diff --git a/src/Altinn.App.Core/Internal/Data/DataElementCache.cs b/src/Altinn.App.Core/Internal/Data/DataElementCache.cs index 89eeff4ffd..fbc7d24179 100644 --- a/src/Altinn.App.Core/Internal/Data/DataElementCache.cs +++ b/src/Altinn.App.Core/Internal/Data/DataElementCache.cs @@ -21,7 +21,7 @@ public async Task GetOrCreate(DataElementIdentifier key, Func> valueF _cache.Add(key.Guid, lazyTask); } } - return await lazyTask.Value; + return await lazyTask.Value.ConfigureAwait(false); } public void Set(DataElementIdentifier key, T data) diff --git a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs index e9baa97667..a3c4decc6c 100644 --- a/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs +++ b/src/Altinn.App.Core/Internal/Data/PreviousDataAccessor.cs @@ -22,7 +22,7 @@ internal class PreviousDataAccessor : IInstanceDataAccessor private readonly ITranslationService _translationService; private readonly Telemetry? _telemetry; - private ConcurrentDictionary> _previousDataCache = new(); + private readonly ConcurrentDictionary> _previousDataCache = new(); public PreviousDataAccessor( IInstanceDataAccessor dataAccessor, @@ -51,29 +51,31 @@ public PreviousDataAccessor( public async Task GetFormData(DataElementIdentifier dataElementIdentifier) { - return (await GetFormDataWrapper(dataElementIdentifier)).BackingData(); + return (await GetFormDataWrapper(dataElementIdentifier).ConfigureAwait(false)).BackingData(); } public async Task GetFormDataWrapper(DataElementIdentifier dataElementIdentifier) { - var data = await _previousDataCache.GetOrAdd( - dataElementIdentifier, - async id => - { - var dataType = this.GetDataType(id); - if (dataType.AppLogic?.ClassRef is null) + var data = await _previousDataCache + .GetOrAdd( + dataElementIdentifier, + async id => { - throw new InvalidOperationException( - $"Data element {id.Id} is of data type {dataType.Id} which doesn't have app logic in application metadata and cant be used as form data" + var dataType = this.GetDataType(id); + if (dataType.AppLogic?.ClassRef is null) + { + throw new InvalidOperationException( + $"Data element {id.Id} is of data type {dataType.Id} which doesn't have app logic in application metadata and cant be used as form data" + ); + } + + var binaryData = await _dataAccessor.GetBinaryData(id).ConfigureAwait(false); + return FormDataWrapperFactory.Create( + _modelSerializationService.DeserializeFromStorage(binaryData.Span, dataType) ); } - - var binaryData = await _dataAccessor.GetBinaryData(id); - return FormDataWrapperFactory.Create( - _modelSerializationService.DeserializeFromStorage(binaryData.Span, dataType) - ); - } - ); + ) + .ConfigureAwait(false); return data.Copy(); } @@ -99,7 +101,7 @@ public IInstanceDataAccessor GetPreviousDataAccessor() public async Task> GetBinaryData(DataElementIdentifier dataElementIdentifier) { - return await _dataAccessor.GetBinaryData(dataElementIdentifier); + return await _dataAccessor.GetBinaryData(dataElementIdentifier).ConfigureAwait(false); } public DataElement GetDataElement(DataElementIdentifier dataElementIdentifier) diff --git a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs index c9d4cc2b49..451072d241 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs @@ -142,6 +142,25 @@ public IEnumerable GetValidators(string taskId) validators.Add(new LegacyIInstanceValidatorFormDataValidator(_generalSettings, instanceValidator)); } + ThrowIfDuplicateValidators(validators, taskId); + return validators; } + + private static void ThrowIfDuplicateValidators(List validators, string taskId) + { + var sourceNames = validators.Select(v => v.ValidationSource).Distinct(StringComparer.OrdinalIgnoreCase); + if (sourceNames.Count() != validators.Count) + { + var duplicates = validators + .GroupBy(v => v.ValidationSource, StringComparer.OrdinalIgnoreCase) + .Where(g => g.Count() > 1) + .Select(g => g.Key); + + var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); + throw new InvalidOperationException( + $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{string.Join(", ", duplicates)}\n\nAll sources:\n{sources}" + ); + } + } } diff --git a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs index 195c997beb..77f7720e91 100644 --- a/src/Altinn.App.Core/Internal/Validation/ValidationService.cs +++ b/src/Altinn.App.Core/Internal/Validation/ValidationService.cs @@ -61,7 +61,7 @@ public async Task> ValidateInstanceAtTask( // Remove ignored validators if (ignoredValidators is not null) validators = validators.Where(v => - !ignoredValidators.Contains(v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) + !ignoredValidators.Contains(v.ValidationSource, StringComparer.OrdinalIgnoreCase) ); var cleanAccessor = dataAccessor; @@ -132,14 +132,10 @@ public async Task> ValidateIncrementalFormData( .GetValidators(taskId) .Where(v => !v.NoIncrementalValidation - && !( - ignoredValidators?.Contains(v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) ?? false - ) + && !(ignoredValidators?.Contains(v.ValidationSource, StringComparer.OrdinalIgnoreCase) ?? false) ) .ToArray(); - ThrowIfDuplicateValidators(validators, taskId); - DataElementChanges cleanChanges = changes; IInstanceDataAccessor cleanAccessor = dataAccessor; if (validators.Any(p => p.ShouldRunAfterRemovingHiddenData)) @@ -261,23 +257,4 @@ is string translated } } } - - private static void ThrowIfDuplicateValidators(IValidator[] validators, string taskId) - { - var sourceNames = validators - .Select(v => v.ValidationSource) - .Distinct(StringComparer.InvariantCultureIgnoreCase); - if (sourceNames.Count() != validators.Length) - { - var duplicates = validators - .GroupBy(v => v.ValidationSource, StringComparer.InvariantCultureIgnoreCase) - .Where(g => g.Count() > 1) - .Select(g => g.Key); - - var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); - throw new InvalidOperationException( - $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{string.Join(", ", duplicates)}" - ); - } - } } diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs index 8bdef9cbe5..cd0795e6a1 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestCleanDataAccessor.cs @@ -240,7 +240,7 @@ public async Task HideRowAndNameInSubComponent() where T2 : class { var fixture = await DataAccessorFixture.CreateAsync( - [new("mainLayout", typeof(T1), MaxCount: 1), new("subLayout", typeof(T2), MaxCount: 1)], + [new("mainLayout", typeof(T1), MaxCount: 1), new("subLayout", typeof(T2), MaxCount: 0)], _outputHelper ); fixture.AddFormData(data); diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs index 6b1c0a497a..6101ee19da 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.cs @@ -72,12 +72,7 @@ private async Task RunTest(bool incremental, bool removeHidden) new() { Name = "row5", HideRow = false }, ], }; - var subData1 = new SubModel() - { - SubPageTitle = "", - UnboundField = "unbound1", - // TODO: add more data - }; + var subData1 = new SubModel() { SubPageTitle = "", UnboundField = "unbound1" }; var subData2 = new SubModel() { HideSubPageTitle = false, @@ -119,7 +114,6 @@ private async Task RunTest(bool incremental, bool removeHidden) { subElement.HideSubPageTitle = true; } - //TODO: add changes var changes = dataMutator.GetDataElementChanges(initializeAltinnRowId: true); if (incremental) diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs index 3d2e38a8cc..7b4ca024a2 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs @@ -108,7 +108,7 @@ public static async Task CreateAsync( return fixture; } - public record LayoutSetSpec(string LayoutSetName, Type Type, int MaxCount); + public record LayoutSetSpec(string LayoutSetName, Type ModelType, int MaxCount); /// /// The first spec is the default layout set. The remaining can be referenced as subforms @@ -147,13 +147,15 @@ private async Task AddLayouts(List specs, string callerFilePath) { Id = spec.LayoutSetName + "_dataType", TaskId = TaskId, - AppLogic = new() { ClassRef = spec.Type.FullName }, + AppLogic = new() { ClassRef = spec.ModelType.FullName }, MaxCount = spec.MaxCount, }; ApplicationMetadata.DataTypes.Add(dataType); - AppModelMock.Setup(am => am.GetModelType(spec.Type.FullName!)).Returns(spec.Type); - AppModelMock.Setup(am => am.Create(spec.Type.FullName!)).Returns(Activator.CreateInstance(spec.Type)!); + AppModelMock.Setup(am => am.GetModelType(spec.ModelType.FullName!)).Returns(spec.ModelType); + AppModelMock + .Setup(am => am.Create(spec.ModelType.FullName!)) + .Returns(Activator.CreateInstance(spec.ModelType)!); var layoutSet = new LayoutSetComponent(pages.ToList(), spec.LayoutSetName, dataType); layouts.Add(layoutSet); From e2d601c7a83cbea3fe5230eb9ada3688fba4f5e1 Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Mon, 22 Sep 2025 21:16:10 +0200 Subject: [PATCH 6/6] Fix validation of unique ValidationSource after codeRabbit suggestion --- ...gacyIInstanceValidatorFormDataValidator.cs | 2 +- .../LegacyIInstanceValidatorTaskValidator.cs | 2 +- .../Internal/Validation/IValidatorFactory.cs | 38 ++++++++++++------- .../Helpers/Patch/PatchServiceTests.cs | 4 ++ .../ValidationServiceOldTests.cs | 4 ++ .../ValidationServiceTests.cs | 4 ++ .../Validators/ValidationServiceTests.cs | 4 ++ ...stValidateCleanData.CleanFull.verified.txt | 5 +++ ...ateCleanData.CleanIncremental.verified.txt | 8 ++-- ...stValidateCleanData.DirtyFull.verified.txt | 5 +++ ...ateCleanData.DirtyIncremental.verified.txt | 8 ++-- .../FullTests/DataAccessorFixture.cs | 4 ++ .../FullTests/SubForm/SubFormTests.cs | 4 ++ ...ouldNotChange_Unintentionally.verified.txt | 2 +- 14 files changed, 70 insertions(+), 24 deletions(-) diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs index d2d9ebf4cc..e20e66bd61 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs @@ -42,7 +42,7 @@ public string ValidationSource { var type = _instanceValidator?.GetType() ?? GetType(); Debug.Assert(type.FullName is not null, "FullName does not return null on class/struct types"); - return type.FullName; + return type.FullName + "_FormData"; } } diff --git a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs index 89ff5ff318..8d449a1205 100644 --- a/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorTaskValidator.cs @@ -43,7 +43,7 @@ public string ValidationSource { var type = _instanceValidator.GetType(); Debug.Assert(type.FullName is not null, "FullName does not return null on class/struct types"); - return type.FullName; + return type.FullName + "_Task"; } } diff --git a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs index 451072d241..2c84ef2df8 100644 --- a/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs +++ b/src/Altinn.App.Core/Internal/Validation/IValidatorFactory.cs @@ -6,6 +6,7 @@ using Altinn.App.Core.Internal.Data; using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; namespace Altinn.App.Core.Internal.Validation; @@ -30,6 +31,7 @@ public class ValidatorFactory : IValidatorFactory private readonly IAppMetadata _appMetadata; private readonly AppImplementationFactory _appImplementationFactory; private readonly IDataElementAccessChecker _dataElementAccessChecker; + private readonly IHostEnvironment _hostEnvironment; /// /// Initializes a new instance of the class. @@ -37,13 +39,15 @@ public class ValidatorFactory : IValidatorFactory public ValidatorFactory( IOptions generalSettings, IAppMetadata appMetadata, - IServiceProvider serviceProvider + IServiceProvider serviceProvider, + IHostEnvironment hostEnvironment ) { _generalSettings = generalSettings; _appMetadata = appMetadata; _appImplementationFactory = serviceProvider.GetRequiredService(); _dataElementAccessChecker = serviceProvider.GetRequiredService(); + _hostEnvironment = hostEnvironment; } private IEnumerable GetIValidators(string taskId) @@ -147,20 +151,28 @@ public IEnumerable GetValidators(string taskId) return validators; } - private static void ThrowIfDuplicateValidators(List validators, string taskId) + private void ThrowIfDuplicateValidators(List validators, string taskId) { - var sourceNames = validators.Select(v => v.ValidationSource).Distinct(StringComparer.OrdinalIgnoreCase); - if (sourceNames.Count() != validators.Count) + // Only run verification in development + if (!_hostEnvironment.IsDevelopment()) { - var duplicates = validators - .GroupBy(v => v.ValidationSource, StringComparer.OrdinalIgnoreCase) - .Where(g => g.Count() > 1) - .Select(g => g.Key); - - var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); - throw new InvalidOperationException( - $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\n{string.Join(", ", duplicates)}\n\nAll sources:\n{sources}" - ); + return; } + + HashSet uniqueSources = new(StringComparer.OrdinalIgnoreCase); + if (validators.All(v => uniqueSources.Add(v.ValidationSource))) + { + return; + } + + var duplicates = validators + .GroupBy(v => v.ValidationSource, StringComparer.OrdinalIgnoreCase) + .Where(g => g.Count() > 1) + .Select(g => g.Key); + + var sources = string.Join('\n', validators.Select(v => $"{v.ValidationSource} {v.GetType().FullName}")); + throw new InvalidOperationException( + $"Duplicate validators found for task {taskId}. Ensure that each validator has a unique ValidationSource.\n\nDuplicates: {string.Join(", ", duplicates)}\n\nAll sources:\n{sources}" + ); } } diff --git a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs index 194e696815..1a2e44ef5c 100644 --- a/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs +++ b/test/Altinn.App.Api.Tests/Helpers/Patch/PatchServiceTests.cs @@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Hosting; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using Moq; @@ -51,6 +52,7 @@ public sealed class PatchServiceTests : IDisposable private readonly Mock _appMetadataMock = new(MockBehavior.Strict); private readonly Mock _translationServiceMock = new(MockBehavior.Strict); private readonly TelemetrySink _telemetrySink = new(); + private readonly Mock _hostEnvironment = new(MockBehavior.Strict); private readonly Mock _webHostEnvironment = new(MockBehavior.Strict); private readonly Mock _appResourcesMock = new(MockBehavior.Strict); private readonly Mock _dataElementAccessCheckerMock = new(MockBehavior.Strict); @@ -103,6 +105,7 @@ public PatchServiceTests() .Setup(x => x.CanRead(It.IsAny(), It.IsAny())) .ReturnsAsync(true); + _hostEnvironment.SetupGet(h => h.EnvironmentName).Returns("Development"); _webHostEnvironment.SetupGet(whe => whe.EnvironmentName).Returns("Development"); var services = new ServiceCollection(); services.AddAppImplementationFactory(); @@ -120,6 +123,7 @@ public PatchServiceTests() _modelSerializationService = new ModelSerializationService(_appModelMock.Object); services.AddSingleton(_modelSerializationService); services.Configure(_ => { }); + services.AddSingleton(_hostEnvironment.Object); _serviceProvider = services.BuildStrictServiceProvider(); var validatorFactory = _serviceProvider.GetRequiredService(); 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 e1c9aeedc4..64f2f1a319 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceOldTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceOldTests.cs @@ -16,6 +16,7 @@ using FluentAssertions; using Microsoft.AspNetCore.Http; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Moq; @@ -29,6 +30,7 @@ public class ValidationServiceOldTests private readonly Mock _appMetadataMock = new(MockBehavior.Strict); private readonly Mock _translationServiceMock = new(MockBehavior.Loose); private readonly Mock _dataElementAccessCheckerMock = new(MockBehavior.Strict); + private readonly Mock _hostEnvironmentMock = new(MockBehavior.Strict); private readonly ServiceCollection _serviceCollection = new(); private readonly ApplicationMetadata _applicationMetadata = new("tdd/test") @@ -50,6 +52,7 @@ public ValidationServiceOldTests() _dataElementAccessCheckerMock .Setup(x => x.CanRead(It.IsAny(), It.IsAny())) .ReturnsAsync(true); + _hostEnvironmentMock.SetupGet(h => h.EnvironmentName).Returns("Development"); _serviceCollection.AddAppImplementationFactory(); _serviceCollection.AddSingleton(_loggerMock.Object); @@ -62,6 +65,7 @@ public ValidationServiceOldTests() _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(_dataElementAccessCheckerMock.Object); + _serviceCollection.AddSingleton(_hostEnvironmentMock.Object); _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new GeneralSettings())); _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new AppSettings())); _appMetadataMock.Setup(am => am.GetApplicationMetadata()).ReturnsAsync(_applicationMetadata); 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 b3901f2c27..747a5f5a0f 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/LegacyValidationServiceTests/ValidationServiceTests.cs @@ -15,6 +15,7 @@ using FluentAssertions; using Microsoft.Extensions.DependencyInjection; using Microsoft.Extensions.DependencyInjection.Extensions; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Logging; using Moq; @@ -79,6 +80,7 @@ public class MyModel private readonly Mock _dataClientMock = new(MockBehavior.Strict); private readonly Mock _instanceClientMock = new(MockBehavior.Strict); private readonly Mock _dataElementAccessCheckerMock = new(MockBehavior.Strict); + private readonly Mock _hostEnvironmentMock = new(MockBehavior.Strict); private readonly IInstanceDataAccessor _dataAccessor; @@ -136,6 +138,7 @@ public ValidationServiceTests() _dataElementAccessCheckerMock .Setup(x => x.CanRead(It.IsAny(), It.IsAny())) .ReturnsAsync(true); + _hostEnvironmentMock.SetupGet(h => h.EnvironmentName).Returns(Environments.Development); _modelSerialization = new ModelSerializationService(_appModelMock.Object); _dataAccessor = new InstanceDataUnitOfWork( @@ -162,6 +165,7 @@ public ValidationServiceTests() _appMetadataMock.Setup(a => a.GetApplicationMetadata()).ReturnsAsync(_defaultAppMetadata); _serviceCollection.AddSingleton(); _serviceCollection.AddSingleton(_dataElementAccessCheckerMock.Object); + _serviceCollection.AddSingleton(_hostEnvironmentMock.Object); _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new GeneralSettings())); _serviceCollection.AddSingleton(Microsoft.Extensions.Options.Options.Create(new AppSettings())); diff --git a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs index 6cb5aa572c..031a9ba083 100644 --- a/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs +++ b/test/Altinn.App.Core.Tests/Features/Validators/ValidationServiceTests.cs @@ -11,6 +11,7 @@ using Altinn.Platform.Storage.Interface.Models; using FluentAssertions; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Moq; using Xunit.Abstractions; using Exception = System.Exception; @@ -35,6 +36,7 @@ public class ValidationServiceTests : IAsyncLifetime private readonly Mock _appMetadataMock = new(MockBehavior.Strict); private readonly Mock _translationServiceMock = new(MockBehavior.Loose); private readonly Mock _dataElementAccessCheckerMock = new(MockBehavior.Strict); + private readonly Mock _hostEnvironmentMock = new(MockBehavior.Strict); private readonly InstanceDataAccessorFake _instanceDataAccessor; private readonly IServiceCollection _services = new ServiceCollection(); private readonly Lazy _serviceProvider; @@ -44,9 +46,11 @@ public ValidationServiceTests(ITestOutputHelper output) _dataElementAccessCheckerMock .Setup(x => x.CanRead(It.IsAny(), It.IsAny())) .ReturnsAsync(true); + _hostEnvironmentMock.SetupGet(h => h.EnvironmentName).Returns(Environments.Development); _instanceDataAccessor = new InstanceDataAccessorFake(_instance, _appMetadata, TaskId); _services.AddTransient(); + _services.AddSingleton(_hostEnvironmentMock.Object); _services.AddTelemetrySink(); _services.AddFakeLoggingWithXunit(output); _services.AddTransient(); diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt index b95b554a0c..14e6435451 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanFull.verified.txt @@ -13,6 +13,11 @@ Arguments: {}, ReturnValue: mockValidator }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, { Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), Arguments: {}, diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt index 87b3e02c8d..8fe0d9dab8 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.CleanIncremental.verified.txt @@ -9,14 +9,14 @@ ReturnValue: true }, { - Method: IValidator.get_NoIncrementalValidation(), + Method: IValidator.get_ValidationSource(), Arguments: {}, - ReturnValue: false + ReturnValue: mockValidator }, { - Method: IValidator.get_ValidationSource(), + Method: IValidator.get_NoIncrementalValidation(), Arguments: {}, - ReturnValue: mockValidator + ReturnValue: false }, { Method: IValidator.get_ValidationSource(), diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt index fcd15ea44b..0138163ce9 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyFull.verified.txt @@ -13,6 +13,11 @@ Arguments: {}, ReturnValue: mockValidator }, + { + Method: IValidator.get_ValidationSource(), + Arguments: {}, + ReturnValue: mockValidator + }, { Method: IValidator.get_ShouldRunAfterRemovingHiddenData(), Arguments: {}, diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt index 6f8540a692..b892dd1c36 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/CleanDataAccessor/TestValidateCleanData.DirtyIncremental.verified.txt @@ -9,14 +9,14 @@ ReturnValue: true }, { - Method: IValidator.get_NoIncrementalValidation(), + Method: IValidator.get_ValidationSource(), Arguments: {}, - ReturnValue: false + ReturnValue: mockValidator }, { - Method: IValidator.get_ValidationSource(), + Method: IValidator.get_NoIncrementalValidation(), Arguments: {}, - ReturnValue: mockValidator + ReturnValue: false }, { Method: IValidator.get_ValidationSource(), diff --git a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs index 7b4ca024a2..0fcbf07600 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/DataAccessorFixture.cs @@ -16,6 +16,7 @@ using Altinn.App.Core.Models.Layout.Components; using Altinn.Platform.Storage.Interface.Models; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; using Moq; using Xunit.Abstractions; @@ -40,6 +41,7 @@ public sealed class DataAccessorFixture public Mock TranslationServiceMock { get; } = new(MockBehavior.Strict); internal Mock DataElementAccessCheckerMock { get; } = new(MockBehavior.Strict); + public Mock HostEnvironmentMock { get; } = new(MockBehavior.Strict); public FrontEndSettings FrontEndSettings { get; } = new(); public GeneralSettings GeneralSettings { get; } = new(); @@ -70,6 +72,7 @@ private DataAccessorFixture(ITestOutputHelper outputHelper) ServiceCollection.AddSingleton(TranslationServiceMock.Object); ServiceCollection.AddSingleton(InstanceClientMock.Object); ServiceCollection.AddSingleton(DataElementAccessCheckerMock.Object); + ServiceCollection.AddSingleton(HostEnvironmentMock.Object); ServiceCollection.AddSingleton(); ServiceCollection.AddSingleton(); ServiceCollection.AddTransient(); @@ -95,6 +98,7 @@ private DataAccessorFixture(ITestOutputHelper outputHelper) }, } ); + HostEnvironmentMock.SetupGet(h => h.EnvironmentName).Returns("Development"); } public static async Task CreateAsync( 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 1db2e43cc1..34ac780b45 100644 --- a/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs +++ b/test/Altinn.App.Core.Tests/LayoutExpressions/FullTests/SubForm/SubFormTests.cs @@ -20,6 +20,7 @@ using Microsoft.AspNetCore.Http; using Microsoft.AspNetCore.Mvc.ModelBinding.Validation; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Hosting; using Microsoft.Extensions.Options; using Moq; using Xunit.Abstractions; @@ -259,6 +260,7 @@ private record SubFormModel( private readonly Mock _translationServiceMock = new(MockBehavior.Loose); private readonly Mock _httpContextAccessorMock = new(MockBehavior.Loose); private readonly Mock _dataElementAccessCheckerMock = new(MockBehavior.Strict); + private readonly Mock _hostEnvironmentMock = new(MockBehavior.Strict); private readonly IServiceCollection _services = new ServiceCollection(); private static readonly JsonSerializerOptions _options = new() @@ -292,6 +294,7 @@ public SubFormTests(ITestOutputHelper output, DataAnnotationsTestFixture fixture _dataElementAccessCheckerMock .Setup(x => x.CanRead(It.IsAny(), It.IsAny())) .ReturnsAsync(true); + _hostEnvironmentMock.SetupGet(h => h.EnvironmentName).Returns("Development"); _output = output; _services.AddAppImplementationFactory(); @@ -300,6 +303,7 @@ public SubFormTests(ITestOutputHelper output, DataAnnotationsTestFixture fixture _services.AddSingleton(_translationServiceMock.Object); _services.AddSingleton(_httpContextAccessorMock.Object); _services.AddSingleton(_dataElementAccessCheckerMock.Object); + _services.AddSingleton(_hostEnvironmentMock.Object); _services.AddSingleton(fixture.App.Services.GetRequiredService()); _services.AddSingleton(_generalSettings); _services.AddTransient(); diff --git a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt index 0274b9c7eb..fd030bed18 100644 --- a/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt +++ b/test/Altinn.App.Core.Tests/PublicApiTests.PublicApi_ShouldNotChange_Unintentionally.verified.txt @@ -3817,7 +3817,7 @@ namespace Altinn.App.Core.Internal.Validation } public class ValidatorFactory : Altinn.App.Core.Internal.Validation.IValidatorFactory { - public ValidatorFactory(Microsoft.Extensions.Options.IOptions generalSettings, Altinn.App.Core.Internal.App.IAppMetadata appMetadata, System.IServiceProvider serviceProvider) { } + public ValidatorFactory(Microsoft.Extensions.Options.IOptions generalSettings, Altinn.App.Core.Internal.App.IAppMetadata appMetadata, System.IServiceProvider serviceProvider, Microsoft.Extensions.Hosting.IHostEnvironment hostEnvironment) { } public System.Collections.Generic.IEnumerable GetValidators(string taskId) { } } }