Skip to content

Commit

Permalink
Add new GenericFormDataValidator2 without constructor injection of Da…
Browse files Browse the repository at this point in the history
…taType.

Keep the old GenericFormDataValidator for backwards compatibility.
  • Loading branch information
ivarne committed Jun 1, 2024
1 parent ad2cd05 commit 6477b1f
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 1 deletion.
1 change: 1 addition & 0 deletions src/Altinn.App.Core/Features/IFormDataValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ public interface IFormDataValidator

/// <summary>
/// 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.
/// </summary>
/// <param name="current">The current state of the form data</param>
/// <param name="previous">The previous state of the form data</param>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@ namespace Altinn.App.Core.Features.Validation;
/// Simple wrapper for validation of form data that does the type checking for you.
/// </summary>
/// <typeparam name="TModel">The type of the model this class will validate</typeparam>
// TODO: Consider marking this as obsolete and use the new GenericFormDataValidator2 instead

Check warning on line 13 in src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

Complete the task associated to this 'TODO' comment. (https://rules.sonarsource.com/csharp/RSPEC-1135)
//[Obsolete("Use GenericFormDataValidator2 instead")]
public abstract class GenericFormDataValidator<TModel> : IFormDataValidator
{
/// <summary>
Expand All @@ -28,7 +30,7 @@ protected GenericFormDataValidator(string dataType)
private static readonly AsyncLocal<List<ValidationIssue>> _validationIssues = new();

Check warning on line 30 in src/Altinn.App.Core/Features/Validation/GenericFormDataValidator.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

A static field in a generic type is not shared among instances of different close constructed types. (https://rules.sonarsource.com/csharp/RSPEC-2743)

/// <summary>
/// Default implementation that respects the runFor prefixes.
/// Default implementation that calls the same method with TModel arguments.
/// </summary>
public bool HasRelevantChanges(object current, object previous)
{
Expand Down Expand Up @@ -118,6 +120,7 @@ public async Task<List<ValidationIssue>> ValidateFormData(

/// <summary>
/// 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.
/// </summary>
/// <param name="current">The current data model after applying patches and data processing</param>
/// <param name="previous">The previous state before patches and data processing</param>
Expand Down
118 changes: 118 additions & 0 deletions src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs
Original file line number Diff line number Diff line change
@@ -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;

/// <summary>
/// Simple wrapper for validation of form data that does the type checking for you.
/// </summary>
/// <typeparam name="TModel">The type of the model this class will validate</typeparam>
public abstract class GenericFormDataValidator2<TModel> : IFormDataValidator
{
// ReSharper disable once StaticMemberInGenericType
private static readonly AsyncLocal<List<ValidationIssue>> _validationIssues = new();

Check warning on line 16 in src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

A static field in a generic type is not shared among instances of different close constructed types. (https://rules.sonarsource.com/csharp/RSPEC-2743)

/// <inheritdoc />
public abstract string DataType { get; }

/// <summary>
/// Default implementation that calls the same method with TModel arguments.
/// </summary>
public bool HasRelevantChanges(object current, object previous)
{
if (current is not TModel currentCast)
{
throw new Exception(

Check warning on line 28 in src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

'System.Exception' should not be thrown by user code. (https://rules.sonarsource.com/csharp/RSPEC-112)
$"{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(

Check warning on line 35 in src/Altinn.App.Core/Features/Validation/GenericFormDataValidator2.cs

View workflow job for this annotation

GitHub Actions / Static code analysis

'System.Exception' should not be thrown by user code. (https://rules.sonarsource.com/csharp/RSPEC-112)
$"{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);
}

/// <summary>
/// Convenience method to create a validation issue for a field using a linq expression instead of a json path for field
/// </summary>
/// <param name="selector">An expression that is used to attach the issue to a path in the data model</param>
/// <param name="textKey">The key used to lookup translations for the issue (displayed if lookup fails)</param>
/// <param name="severity">The severity for the issue (default Error)</param>
/// <param name="description">Optional description if you want to provide a user friendly message that don't rely on the translation system</param>
/// <param name="code">optional short code for the type of issue</param>
/// <param name="customTextParams">List of parameters to replace after looking up the translation. Zero indexed {0}</param>
protected void CreateValidationIssue<T>(
Expression<Func<TModel, T>> selector,
string textKey,
ValidationIssueSeverity severity = ValidationIssueSeverity.Error,
string? description = null,
string? code = null,
List<string>? 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
}
);
}

/// <summary>
/// Allows inheriting classes to add validation issues.
/// </summary>
protected void AddValidationIssue(ValidationIssue issue)
{
Debug.Assert(_validationIssues.Value is not null);
_validationIssues.Value.Add(issue);
}

/// <summary>
/// Implementation of the generic <see cref="IFormDataValidator"/> interface to call the correctly typed
/// validation method implemented by the inheriting class.
/// </summary>
public async Task<List<ValidationIssue>> 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<ValidationIssue>();
await ValidateFormData(instance, dataElement, model, language);
return _validationIssues.Value;
}

/// <summary>
/// Implement this method to validate the data.
/// </summary>
protected abstract Task ValidateFormData(Instance instance, DataElement dataElement, TModel data, string? language);

/// <summary>
/// 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.
/// </summary>
/// <param name="current">The current data model after applying patches and data processing</param>
/// <param name="previous">The previous state before patches and data processing</param>
/// <returns>true if the list of validation issues might be different on the two model states</returns>
protected abstract bool HasRelevantChanges(TModel current, TModel previous);
}
Original file line number Diff line number Diff line change
@@ -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<MyModel>? Children { get; set; }
}

private class TestValidator : GenericFormDataValidator2<MyModel>
{
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");
}
}

0 comments on commit 6477b1f

Please sign in to comment.