Skip to content

Commit

Permalink
Use extension methods instead of properties for convenience methods o…
Browse files Browse the repository at this point in the history 
…n IInstantanceDataAccessor (#907)

* Use extension methods instead of properties for convenience methods on IInstantanceDataAccessor

Interface properties with default implementations has a few drawbacks.

* They need to be mocked individually, because Moq does not use the default implementation
* They typically only serve as a starting point for futher filtering using linq. An extension methdod takes an argument (task/dataType) which is easier to understand than a linq .Where filter.

Another benefit is that we encapsulate appMetadata so that user code has even less reason to deal with dataType

* Fix sonar issues and improve test coverage
  • Loading branch information
@ivarne
ivarne authored Nov 22, 2024
1 parent b0ce0e5 commit f492381
Show file tree
Hide file tree
Showing 26 changed files with 364 additions and 271 deletions.
153 changes: 136 additions & 17 deletions src/Altinn.App.Core/Features/IInstanceDataAccessor.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,22 +13,6 @@ public interface IInstanceDataAccessor
/// </summary>
Instance Instance { get; }

/// <summary>
/// The data elements on the instance.
/// </summary>
IEnumerable<DataElement> DataElements => Instance.Data;

/// <summary>
/// Data elements of the instance that has appLogic (form data elements).
/// </summary>
IEnumerable<DataElement> FormDataElements =>
Instance.Data.Where(d => GetDataType(d).AppLogic?.ClassRef is not null);

/// <summary>
/// Data elements of the instance that represents attachments/binary data (no appLogic).
/// </summary>
IEnumerable<DataElement> BinaryDataElements => Instance.Data.Where(d => GetDataType(d).AppLogic?.ClassRef is null);

/// <summary>
/// Get the actual data represented in the data element.
/// </summary>
Expand All @@ -49,9 +33,144 @@ public interface IInstanceDataAccessor
/// <throws>Throws an InvalidOperationException if the data element is not found on the instance</throws>
DataElement GetDataElement(DataElementIdentifier dataElementIdentifier);

/// <summary>
/// Get the data type from application with the given type.
/// </summary>
/// <param name="dataTypeId">DataType.Id (from applicationmetadata.json)</param>
/// <returns>The data type (or null if it does not exist)</returns>
DataType? GetDataType(string dataTypeId);
}

/// <summary>
/// Extension methods for IInstanceDataAccessor to simplify usage.
/// </summary>
public static class IInstanceDataAccessorExtensions
{
/// <summary>
/// Get the dataType of a data element.
/// </summary>
/// <throws>Throws an InvalidOperationException if the data element is not found on the instance</throws>
DataType GetDataType(DataElementIdentifier dataElementIdentifier);
public static DataType GetDataType(
this IInstanceDataAccessor dataAccessor,
DataElementIdentifier dataElementIdentifier
)
{
var dataElement = dataAccessor.GetDataElement(dataElementIdentifier);
var dataType = dataAccessor.GetDataType(dataElement.DataType);
if (dataType is null)
{
throw new InvalidOperationException(
$"Data type {dataElement.DataType} not found in applicationmetadata.json"
);
}

return dataType;
}

/// <summary>
/// Get the actual data represented in the data element (cast into T).
/// </summary>
/// <returns>The deserialized data model for this data element or an exception for non-form data elements</returns>
public static async Task<T> GetFormData<T>(
this IInstanceDataAccessor accessor,
DataElementIdentifier dataElementIdentifier
)
where T : class
{
object data = await accessor.GetFormData(dataElementIdentifier);
if (data is T t)
{
return t;
}
throw new InvalidOperationException($"Data element {dataElementIdentifier} is not of type {typeof(T)}");
}

/// <summary>
/// Get all elements of a specific data type.
/// </summary>
public static IEnumerable<DataElement> GetDataElementsForType(
this IInstanceDataAccessor accessor,
string dataTypeId
) => accessor.Instance.Data.Where(dataElement => dataTypeId.Equals(dataElement.DataType, StringComparison.Ordinal));

/// <summary>
/// Get all elements of a specific data type.
/// </summary>
public static IEnumerable<DataElement> GetDataElementsForType(
this IInstanceDataAccessor accessor,
DataType dataType
) => accessor.GetDataElementsForType(dataType.Id);

/// <summary>
/// Retrieves the data elements associated with a specific task.
/// </summary>
/// <param name="accessor">The instance data accessor.</param>
/// <param name="taskId">The identifier of the task.</param>
/// <returns>An enumerable collection of tuples containing the data type and data element associated with the specified task.</returns>
public static IEnumerable<(DataType dataType, DataElement dataElement)> GetDataElementsForTask(
this IInstanceDataAccessor accessor,
string taskId
)
{
foreach (var dataElement in accessor.Instance.Data)
{
var dataType = accessor.GetDataType(dataElement.DataType);
if (taskId.Equals(dataType?.TaskId, StringComparison.Ordinal))
{
yield return (dataType, dataElement);
}
}
}

/// <summary>
/// Get all form data elements along with their data types.
/// </summary>
public static IEnumerable<(DataType dataType, DataElement dataElement)> GetDataElementsWithFormData(
this IInstanceDataAccessor accessor
)
{
foreach (var dataElement in accessor.Instance.Data)
{
var dataType = accessor.GetDataType(dataElement.DataType);
if (dataType?.AppLogic?.ClassRef is not null)
{
yield return (dataType, dataElement);
}
}
}

/// <summary>
/// Get all form data elements along with their data types for a specific task.
/// </summary>
public static IEnumerable<(DataType dataType, DataElement dataElement)> GetDataElementsWithFormDataForTask(
this IInstanceDataAccessor accessor,
string taskId
)
{
foreach (var dataElement in accessor.Instance.Data)
{
var dataType = accessor.GetDataType(dataElement.DataType);
if (dataType?.AppLogic?.ClassRef is not null && taskId.Equals(dataType.TaskId, StringComparison.Ordinal))
{
yield return (dataType, dataElement);
}
}
}

/// <summary>
/// Get all data elements along with their data types.
/// </summary>
public static IEnumerable<(DataType dataType, DataElement dataElement)> GetDataElements(
this IInstanceDataAccessor accessor
)
{
foreach (var dataElement in accessor.Instance.Data)
{
var dataType = accessor.GetDataType(dataElement.DataType);
if (dataType is not null)
{
yield return (dataType, dataElement);
}
}
}
}
30 changes: 11 additions & 19 deletions src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,14 @@ IAppMetadata appMetadata
/// <summary>
/// Only run for tasks that specifies a layout set
/// </summary>
public bool ShouldRunForTask(string taskId) => GetDataTypesWithExpressionsForTask(taskId).Any();
public bool ShouldRunForTask(string taskId) =>
_appMetadata
.GetApplicationMetadata()
.Result.DataTypes.Exists(dt =>
dt.TaskId == taskId
&& dt.AppLogic?.ClassRef is not null
&& _appResourceService.GetValidationConfiguration(dt.Id) is not null
);

/// <summary>
/// This validator has the code "Expression" and this is known by the frontend, who may request this validator to not run for incremental validation.
Expand All @@ -75,10 +82,10 @@ public async Task<List<ValidationIssue>> Validate(
)
{
var validationIssues = new List<ValidationIssue>();
foreach (var (dataType, validationConfig) in GetDataTypesWithExpressionsForTask(taskId))
foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsForTask(taskId))
{
var formDataElementsForTask = dataAccessor.DataElements.Where(d => d.DataType == dataType.Id);
foreach (var dataElement in formDataElementsForTask)
var validationConfig = _appResourceService.GetValidationConfiguration(dataType.Id);
if (!string.IsNullOrEmpty(validationConfig))
{
var issues = await ValidateFormData(dataElement, dataAccessor, validationConfig, taskId, language);
validationIssues.AddRange(issues);
Expand Down Expand Up @@ -426,19 +433,4 @@ out JsonElement validationsObject
}
return expressionValidations;
}

private IEnumerable<KeyValuePair<DataType, string>> GetDataTypesWithExpressionsForTask(string taskId)
{
var appMetadata = _appMetadata.GetApplicationMetadata().Result;
foreach (
var dataType in appMetadata.DataTypes.Where(dt => dt.TaskId == taskId && dt.AppLogic?.ClassRef is not null)
)
{
var validationConfig = _appResourceService.GetValidationConfiguration(dataType.Id);
if (validationConfig != null)
{
yield return KeyValuePair.Create(dataType, validationConfig);
}
}
}
}
13 changes: 2 additions & 11 deletions src/Altinn.App.Core/Features/Validation/Default/LegacyIInstanceValidatorFormDataValidator.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@
using System.Diagnostics;
using Altinn.App.Core.Configuration;
using Altinn.App.Core.Features.Validation.Helpers;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Models;
using Altinn.App.Core.Models.Validation;
using Microsoft.AspNetCore.Mvc.ModelBinding;
Expand All @@ -16,20 +15,17 @@ namespace Altinn.App.Core.Features.Validation.Default;
public class LegacyIInstanceValidatorFormDataValidator : IValidator
{
private readonly IInstanceValidator _instanceValidator;
private readonly IAppMetadata _appMetadata;
private readonly GeneralSettings _generalSettings;

/// <summary>
/// constructor
/// </summary>
public LegacyIInstanceValidatorFormDataValidator(
IOptions<GeneralSettings> generalSettings,
IInstanceValidator instanceValidator,
IAppMetadata appMetadata
IInstanceValidator instanceValidator
)
{
_instanceValidator = instanceValidator;
_appMetadata = appMetadata;
_generalSettings = generalSettings.Value;
}

Expand Down Expand Up @@ -57,12 +53,7 @@ public async Task<List<ValidationIssue>> Validate(
)
{
var issues = new List<ValidationIssue>();
var appMetadata = await _appMetadata.GetApplicationMetadata();
var dataTypes = appMetadata
.DataTypes.Where(d => d.TaskId == taskId && d.AppLogic?.ClassRef != null)
.Select(d => d.Id)
.ToList();
foreach (var dataElement in dataAccessor.DataElements.Where(d => dataTypes.Contains(d.DataType)))
foreach (var (_, dataElement) in dataAccessor.GetDataElementsWithFormDataForTask(taskId))
{
var data = await dataAccessor.GetFormData(dataElement);
var modelState = new ModelStateDictionary();
Expand Down
17 changes: 2 additions & 15 deletions src/Altinn.App.Core/Features/Validation/Wrappers/DataElementValidatorWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,11 @@ internal class DataElementValidatorWrapper : IValidator
{
private readonly IDataElementValidator _dataElementValidator;
private readonly string _taskId;
private readonly List<DataType> _dataTypes;

public DataElementValidatorWrapper(
IDataElementValidator dataElementValidator,
string taskId,
List<DataType> dataTypes
)
public DataElementValidatorWrapper(IDataElementValidator dataElementValidator, string taskId)
{
_dataElementValidator = dataElementValidator;
_taskId = taskId;
_dataTypes = dataTypes;
}

/// <inheritdoc />
Expand All @@ -47,17 +41,10 @@ public async Task<List<ValidationIssue>> Validate(
{
var issues = new List<ValidationIssue>();
var validateAllElements = _dataElementValidator.DataType == "*";
foreach (var dataElement in dataAccessor.DataElements)
foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsForTask(taskId))
{
if (validateAllElements || _dataElementValidator.DataType == dataElement.DataType)
{
var dataType = _dataTypes.Find(d => d.Id == dataElement.DataType);
if (dataType is null)
{
throw new InvalidOperationException(
$"DataType {dataElement.DataType} not found in dataTypes from applicationmetadata"
);
}
var dataElementValidationResult = await _dataElementValidator.ValidateDataElement(
dataAccessor.Instance,
dataElement,
Expand Down
9 changes: 2 additions & 7 deletions src/Altinn.App.Core/Features/Validation/Wrappers/FormDataValidatorWrapper.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,9 @@ public async Task<List<ValidationIssue>> Validate(
{
var issues = new List<ValidationIssue>();
var validateAllElements = _formDataValidator.DataType == "*";
foreach (var dataElement in dataAccessor.DataElements)
foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsWithFormData())
{
var dataType = dataAccessor.GetDataType(dataElement);
if (dataType.AppLogic?.ClassRef == null)
{
continue;
}
if (!validateAllElements && _formDataValidator.DataType != dataElement.DataType)
if (!validateAllElements && _formDataValidator.DataType != dataType.Id)
{
continue;
}
Expand Down
12 changes: 3 additions & 9 deletions src/Altinn.App.Core/Helpers/DataModel/DataModel.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,21 +17,15 @@ public class DataModel
/// <summary>
/// Constructor that wraps a POCO data model, and gives extra tool for working with the data
/// </summary>
public DataModel(IInstanceDataAccessor dataAccessor, ApplicationMetadata appMetadata)
public DataModel(IInstanceDataAccessor dataAccessor)
{
_dataAccessor = dataAccessor;
foreach (var dataElement in dataAccessor.DataElements)
foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsWithFormData())
{
var dataTypeId = dataElement.DataType;
var dataType = appMetadata.DataTypes.Find(d => d.Id == dataTypeId);
if (dataType is { MaxCount: 1, AppLogic.ClassRef: not null })
if (dataType is { MaxCount: 1 })
{
_dataIdsByType.TryAdd(dataElement.DataType, dataElement);
}
else
{
_dataIdsByType.TryAdd(dataElement.Id, null);
}
}
}

Expand Down
4 changes: 2 additions & 2 deletions src/Altinn.App.Core/Implementation/AppResourcesSI.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -498,8 +498,8 @@ private static byte[] ReadFileContentsFromLegalPath(string legalPath, string fil
public string? GetValidationConfiguration(string dataTypeId)
{
using var activity = _telemetry?.StartGetValidationConfigurationActivity();
string legalPath = $"{_settings.AppBasePath}{_settings.ModelsFolder}";
string filename = $"{legalPath}{dataTypeId}.{_settings.ValidationConfigurationFileName}";
string legalPath = Path.Join(_settings.AppBasePath, _settings.ModelsFolder);
string filename = Path.Join(legalPath, $"{dataTypeId}.{_settings.ValidationConfigurationFileName}");
PathHelper.EnsureLegalPath(legalPath, filename);

string? filedata = null;
Expand Down
Loading

0 comments on commit f492381

Please sign in to comment.