From 6477b1ff54a175be24d84b75f484786e60fc93be Mon Sep 17 00:00:00 2001 From: Ivar Nesje Date: Wed, 3 Apr 2024 22:29:49 +0200 Subject: [PATCH] Add new GenericFormDataValidator2 without constructor injection of DataType. Keep the old GenericFormDataValidator for backwards compatibility. --- .../Features/IFormDataValidator.cs | 1 + .../Validation/GenericFormDataValidator.cs | 5 +- .../Validation/GenericFormDataValidator2.cs | 118 ++++++++++++++++++ .../Validators/GenericValidatorTests2.cs | 80 ++++++++++++ 4 files changed, 203 insertions(+), 1 deletion(-) create mode 100644 src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs create mode 100644 test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests2.cs diff --git a/src/Altinn.App.Core/Features/IFormDataValidator.cs b/src/Altinn.App.Core/Features/IFormDataValidator.cs index bb0e31f51..799aca9d1 100644 --- a/src/Altinn.App.Core/Features/IFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/IFormDataValidator.cs @@ -20,6 +20,7 @@ public interface IFormDataValidator /// /// Used for partial validation to ensure that the validator only runs when relevant fields have changed. + /// A default "return true" implementation can be used if the validator is quick and does not call external APIs. /// /// The current state of the form data /// The previous state of the form data diff --git a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs index 672b3aee4..538899b4d 100644 --- a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs +++ b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs @@ -10,6 +10,8 @@ namespace Altinn.App.Core.Features.Validation; /// Simple wrapper for validation of form data that does the type checking for you. /// /// The type of the model this class will validate +// TODO: Consider marking this as obsolete and use the new GenericFormDataValidator2 instead +//[Obsolete("Use GenericFormDataValidator2 instead")] public abstract class GenericFormDataValidator : IFormDataValidator { /// @@ -28,7 +30,7 @@ protected GenericFormDataValidator(string dataType) private static readonly AsyncLocal> _validationIssues = new(); /// - /// Default implementation that respects the runFor prefixes. + /// Default implementation that calls the same method with TModel arguments. /// public bool HasRelevantChanges(object current, object previous) { @@ -118,6 +120,7 @@ public async Task> ValidateFormData( /// /// Implement this method to check if the data has changed in a way that requires validation. + /// A default "return true" implementation can be used if the validator is quick and does not call external APIs. /// /// The current data model after applying patches and data processing /// The previous state before patches and data processing diff --git a/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs new file mode 100644 index 000000000..e74734c4e --- /dev/null +++ b/src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs @@ -0,0 +1,118 @@ +using System.Diagnostics; +using System.Linq.Expressions; +using Altinn.App.Core.Helpers; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; + +namespace Altinn.App.Core.Features.Validation; + +/// +/// Simple wrapper for validation of form data that does the type checking for you. +/// +/// The type of the model this class will validate +public abstract class GenericFormDataValidator2 : IFormDataValidator +{ + // ReSharper disable once StaticMemberInGenericType + private static readonly AsyncLocal> _validationIssues = new(); + + /// + public abstract string DataType { get; } + + /// + /// Default implementation that calls the same method with TModel arguments. + /// + public bool HasRelevantChanges(object current, object previous) + { + if (current is not TModel currentCast) + { + throw new Exception( + $"{GetType().Name} wants to run on data type {DataType}, but the data is of type {current?.GetType().Name}. It should be of type {typeof(TModel).Name}" + ); + } + + if (previous is not TModel previousCast) + { + throw new Exception( + $"{GetType().Name} wants to run on data type {DataType}, but the previous of type {previous?.GetType().Name}. It should be of type {typeof(TModel).Name}" + ); + } + + return HasRelevantChanges(currentCast, previousCast); + } + + /// + /// Convenience method to create a validation issue for a field using a linq expression instead of a json path for field + /// + /// An expression that is used to attach the issue to a path in the data model + /// The key used to lookup translations for the issue (displayed if lookup fails) + /// The severity for the issue (default Error) + /// Optional description if you want to provide a user friendly message that don't rely on the translation system + /// optional short code for the type of issue + /// List of parameters to replace after looking up the translation. Zero indexed {0} + protected void CreateValidationIssue( + Expression> selector, + string textKey, + ValidationIssueSeverity severity = ValidationIssueSeverity.Error, + string? description = null, + string? code = null, + List? customTextParams = null + ) + { + Debug.Assert(_validationIssues.Value is not null); + AddValidationIssue( + new ValidationIssue + { + Field = LinqExpressionHelpers.GetJsonPath(selector), + Description = description ?? textKey, + Code = code ?? textKey, + CustomTextKey = textKey, + CustomTextParams = customTextParams, + Severity = severity + } + ); + } + + /// + /// Allows inheriting classes to add validation issues. + /// + protected void AddValidationIssue(ValidationIssue issue) + { + Debug.Assert(_validationIssues.Value is not null); + _validationIssues.Value.Add(issue); + } + + /// + /// Implementation of the generic interface to call the correctly typed + /// validation method implemented by the inheriting class. + /// + public async Task> ValidateFormData( + Instance instance, + DataElement dataElement, + object data, + string? language + ) + { + if (data is not TModel model) + { + throw new ArgumentException($"Data is not of type {typeof(TModel)}"); + } + + _validationIssues.Value = new List(); + await ValidateFormData(instance, dataElement, model, language); + return _validationIssues.Value; + } + + /// + /// Implement this method to validate the data. + /// + protected abstract Task ValidateFormData(Instance instance, DataElement dataElement, TModel data, string? language); + + /// + /// Implement this method to check if the data has changed in a way that requires validation. + /// A default "return true" implementation can be used if the validator is quick and does not call external APIs. + /// + /// The current data model after applying patches and data processing + /// The previous state before patches and data processing + /// true if the list of validation issues might be different on the two model states + protected abstract bool HasRelevantChanges(TModel current, TModel previous); +} diff --git a/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests2.cs b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests2.cs new file mode 100644 index 000000000..c0ba361d1 --- /dev/null +++ b/test/Altinn.App.Core.Tests/Features/Validators/GenericValidatorTests2.cs @@ -0,0 +1,80 @@ +using System.Text.Json.Serialization; +using Altinn.App.Core.Features.Validation; +using Altinn.App.Core.Models.Validation; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; + +namespace Altinn.App.Core.Tests.Features.Validators; + +public class GenericValidatorTests2 +{ + private class MyModel + { + [JsonPropertyName("name")] + public string? Name { get; set; } + + [JsonPropertyName("age")] + public int? Age { get; set; } + + [JsonPropertyName("children")] + public List? Children { get; set; } + } + + private class TestValidator : GenericFormDataValidator2 + { + public override string DataType => "MyType"; + + protected override bool HasRelevantChanges(MyModel current, MyModel previous) + { + throw new NotImplementedException(); + } + + protected override Task ValidateFormData( + Instance instance, + DataElement dataElement, + MyModel data, + string? language + ) + { + AddValidationIssue( + new ValidationIssue() { Severity = ValidationIssueSeverity.Informational, Description = "Test info", } + ); + + CreateValidationIssue(c => c.Name, "Test warning", severity: ValidationIssueSeverity.Warning); + var childIndex = 4; + CreateValidationIssue( + c => c.Children![childIndex].Children![0].Name, + "childrenError", + severity: ValidationIssueSeverity.Error + ); + + return Task.CompletedTask; + } + } + + [Fact] + public async Task VerifyTestValidator() + { + var testValidator = new TestValidator(); + var instance = new Instance(); + var dataElement = new DataElement(); + var data = new MyModel(); + + var validationIssues = await testValidator.ValidateFormData(instance, dataElement, data, null); + validationIssues.Should().HaveCount(3); + + var info = validationIssues + .Should() + .ContainSingle(c => c.Severity == ValidationIssueSeverity.Informational) + .Which; + info.Description.Should().Be("Test info"); + + var warning = validationIssues.Should().ContainSingle(c => c.Severity == ValidationIssueSeverity.Warning).Which; + warning.Description.Should().Be("Test warning"); + warning.Field.Should().Be("name"); + + var error = validationIssues.Should().ContainSingle(c => c.Severity == ValidationIssueSeverity.Error).Which; + error.Description.Should().Be("childrenError"); + error.Field.Should().Be("children[4].children[0].name"); + } +}