Skip to content

Commit

Permalink
Fix sonar cloud issues
Browse files Browse the repository at this point in the history
  • Loading branch information
ivarne committed Aug 19, 2024
1 parent f7b91e0 commit e6cda48
Show file tree
Hide file tree
Showing 19 changed files with 36 additions and 55 deletions.
8 changes: 4 additions & 4 deletions src/Altinn.App.Api/Controllers/ActionsController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -202,7 +202,7 @@ Dictionary<string, object> resultUpdatedDataModels
continue;
}
var dataElement = instance.Data.First(d => d.Id.Equals(elementId, StringComparison.OrdinalIgnoreCase));
var previousData = await dataAccessor.Get(dataElement);
var previousData = await dataAccessor.GetData(dataElement);

ObjectUtils.InitializeAltinnRowId(newModel);
ObjectUtils.PrepareModelForXmlStorage(newModel);
Expand Down Expand Up @@ -254,7 +254,7 @@ await _dataClient.UpdateData(
return PartitionValidationIssuesByDataElement(validationIssues);
}

private Dictionary<
private static Dictionary<
string,
Dictionary<string, List<ValidationIssueWithSource>>
> PartitionValidationIssuesByDataElement(Dictionary<string, List<ValidationIssueWithSource>> validationIssues)
Expand All @@ -266,12 +266,12 @@ private Dictionary<
{
if (!updatedValidationIssues.TryGetValue(issue.DataElementId ?? "", out var elementIssues))
{
elementIssues = new Dictionary<string, List<ValidationIssueWithSource>>();
elementIssues = [];
updatedValidationIssues[issue.DataElementId ?? ""] = elementIssues;
}
if (!elementIssues.TryGetValue(validationSource, out var sourceIssues))
{
sourceIssues = new List<ValidationIssueWithSource>();
sourceIssues = [];
elementIssues[validationSource] = sourceIssues;
}
sourceIssues.Add(issue);
Expand Down
7 changes: 0 additions & 7 deletions src/Altinn.App.Api/Controllers/DataController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -511,13 +511,6 @@ public async Task<ActionResult<DataPatchResponseMultiple>> PatchFormDataMultiple
);
}

CachedInstanceDataAccessor dataAccessor = new CachedInstanceDataAccessor(
instance,
_dataClient,
_appMetadata,
_appModel
);

foreach (Guid dataGuid in dataPatchRequest.Patches.Keys)
{
var dataElement = instance.Data.First(m => m.Id.Equals(dataGuid.ToString(), StringComparison.Ordinal));
Expand Down
9 changes: 4 additions & 5 deletions src/Altinn.App.Api/Controllers/ValidateController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ public async Task<IActionResult> ValidateData(
throw new ValidationException("Unable to validate instance without a started process.");
}

List<ValidationIssueWithSource> messages = new List<ValidationIssueWithSource>();
List<ValidationIssueWithSource> messages = [];

DataElement? element = instance.Data.FirstOrDefault(d => d.Id == dataGuid.ToString());

Expand All @@ -150,10 +150,9 @@ public async Task<IActionResult> ValidateData(

var dataAccessor = new CachedInstanceDataAccessor(instance, _dataClient, _appMetadata, _appModel);

// TODO: Consider filtering so that only relevant issues are reported.
messages.AddRange(
await _validationService.ValidateInstanceAtTask(instance, dataType.TaskId, dataAccessor, language)
);
// Run validations for all data elements, but only return the issues for the specific data element
var issues = await _validationService.ValidateInstanceAtTask(instance, dataType.TaskId, dataAccessor, language);
messages.AddRange(issues.Where(i => i.DataElementId == element.Id));

string taskId = instance.Process.CurrentTask.ElementId;

Expand Down
1 change: 0 additions & 1 deletion src/Altinn.App.Core/Features/ITaskValidator.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using Altinn.App.Core.Models.Validation;
using Altinn.Platform.Storage.Interface.Models;
using Microsoft.Extensions.DependencyInjection;

namespace Altinn.App.Core.Features;

Expand Down
2 changes: 1 addition & 1 deletion src/Altinn.App.Core/Features/IValidator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,5 @@ public interface IInstanceDataAccessor
/// </summary>
/// <param name="dataElement">The data element to retrieve. Must be from the instance that is currently active</param>
/// <returns>The deserialized data model for this data element or a stream for binary elements</returns>
Task<object> Get(DataElement dataElement);
Task<object> GetData(DataElement dataElement);
}
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ public async Task<List<ValidationIssue>> Validate(
var validationIssues = new List<ValidationIssue>();
foreach (var dataElement in formDataElementsForTask)
{
var data = instanceDataAccessor.Get(dataElement);
var issues = await ValidateFormData(instance, dataElement, instanceDataAccessor, taskId, language);
validationIssues.AddRange(issues);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public async Task<List<ValidationIssue>> Validate(
var dataTypes = appMetadata.DataTypes.Where(d => d.TaskId == taskId).Select(d => d.Id).ToList();
foreach (var dataElement in instance.Data.Where(d => dataTypes.Contains(d.DataType)))
{
var data = await instanceDataAccessor.Get(dataElement);
var data = await instanceDataAccessor.GetData(dataElement);
var modelState = new ModelStateDictionary();
await _instanceValidator.ValidateData(data, modelState);
issues.AddRange(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,11 @@ internal class FormDataValidatorWrapper : IValidator
{
private readonly IFormDataValidator _formDataValidator;
private readonly string _taskId;
private readonly List<DataType> _dataTypes;

public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string taskId, List<DataType> dataTypes)
public FormDataValidatorWrapper(IFormDataValidator formDataValidator, string taskId)
{
_formDataValidator = formDataValidator;
_taskId = taskId;
_dataTypes = dataTypes;
}

/// <inheritdoc />
Expand Down Expand Up @@ -44,7 +42,7 @@ public async Task<List<ValidationIssue>> Validate(
continue;
}

var data = await instanceDataAccessor.Get(dataElement);
var data = await instanceDataAccessor.GetData(dataElement);
var dataElementValidationResult = await _formDataValidator.ValidateFormData(
instance,
dataElement,
Expand Down
7 changes: 6 additions & 1 deletion src/Altinn.App.Core/Helpers/DataModel/DataModel.cs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,12 @@ public DataModel(IEnumerable<KeyValuePair<DataElement, object>> dataModels)
return null;
}

private object? GetModelDataRecursive(string[] keys, int index, object? currentModel, ReadOnlySpan<int> indicies)
private static object? GetModelDataRecursive(
string[] keys,
int index,
object? currentModel,
ReadOnlySpan<int> indicies
)
{
if (index == keys.Length || currentModel is null)
{
Expand Down
4 changes: 1 addition & 3 deletions src/Altinn.App.Core/Internal/Data/CachedFormDataAccessor.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ IAppModel appModel
public Instance Instance => _instance;

/// <inheritdoc />
public async Task<object> Get(DataElement dataElement)
public async Task<object> GetData(DataElement dataElement)
{
return await _cache.GetOrCreate(
dataElement.Id,
Expand Down Expand Up @@ -97,7 +97,6 @@ public async Task<TValue> GetOrCreate(TKey key, Func<TKey, Task<TValue>> valueFa
{
task = _cache.GetOrAdd(key, innerKey => new Lazy<Task<TValue>>(() => valueFactory(innerKey))).Value;
}
;
return await task;
}

Expand All @@ -116,7 +115,6 @@ public void Set(TKey key, TValue data)

private async Task<Stream> GetBinaryData(DataElement dataElement)
{
;
var data = await _dataClient.GetBinaryData(
_org,
_app,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@
using Altinn.App.Core.Features;
using Altinn.App.Core.Helpers.DataModel;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.Data;
using Altinn.Platform.Storage.Interface.Models;
using Microsoft.Extensions.Options;

Expand Down Expand Up @@ -76,7 +75,9 @@ public async Task<LayoutEvaluatorState> Init(
dataTasks.AddRange(
instance
.Data.Where(dataElement => dataElement.DataType == dataType)
.Select(async dataElement => KeyValuePair.Create(dataElement, await dataAccessor.Get(dataElement)))
.Select(async dataElement =>
KeyValuePair.Create(dataElement, await dataAccessor.GetData(dataElement))
)
);
}

Expand Down
3 changes: 1 addition & 2 deletions src/Altinn.App.Core/Internal/Patch/PatchService.cs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
using Altinn.App.Core.Models.Result;
using Altinn.Platform.Storage.Interface.Models;
using Json.Patch;
using static Altinn.App.Core.Features.Telemetry;

namespace Altinn.App.Core.Internal.Patch;

Expand Down Expand Up @@ -78,7 +77,7 @@ public async Task<ServiceResult<DataPatchResult, DataPatchError>> ApplyPatches(
};
}

var oldModel = await dataAccessor.Get(dataElement);
var oldModel = await dataAccessor.GetData(dataElement);
var oldModelNode = JsonSerializer.SerializeToNode(oldModel);
var patchResult = jsonPatch.Apply(oldModelNode);

Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,6 @@
using System.Globalization;
using System.Text;
using System.Text.Json;
using Altinn.App.Core.Features;
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.Process.Elements;
using Altinn.App.Core.Models.Expressions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ public async Task Finalize(string taskId, Instance instance)
await Task.WhenAll(
changedDataElements.Select(async dataElement =>
{
var data = await dataAccessor.Get(dataElement);
var data = await dataAccessor.GetData(dataElement);
return _dataClient.UpdateData(
data,
Guid.Parse(instance.Id.Split('/')[1]),
Expand All @@ -83,7 +83,7 @@ private async Task<IEnumerable<DataElement>> RunRemoveFieldsInModelOnTaskComplet
)
{
ArgumentNullException.ThrowIfNull(instance.Data);
HashSet<DataElement> modifiedDataElements = new();
HashSet<DataElement> modifiedDataElements = [];

var dataTypesWithLogic = dataTypesToLock.Where(d => !string.IsNullOrEmpty(d.AppLogic?.ClassRef)).ToList();
await Task.WhenAll(
Expand Down Expand Up @@ -128,11 +128,10 @@ private async Task<bool> RemoveFieldsOnTaskComplete(
)
{
bool isModified = false;
var data = await dataAccessor.Get(dataElement);
var data = await dataAccessor.GetData(dataElement);

// remove AltinnRowIds
ObjectUtils.RemoveAltinnRowId(data);
isModified = true;
isModified |= ObjectUtils.RemoveAltinnRowId(data);

// Remove hidden data before validation, ignore hidden rows.
if (_appSettings.Value?.RemoveHiddenData == true)
Expand All @@ -145,7 +144,7 @@ private async Task<bool> RemoveFieldsOnTaskComplete(
language
);
LayoutEvaluator.RemoveHiddenData(evaluationState, RowRemovalOption.Ignore);
// TODO:
// TODO: Make RemoveHiddenData return a bool indicating if data was removed
isModified = true;
}

Expand Down Expand Up @@ -185,11 +184,13 @@ await _dataClient.InsertFormData(
else
{
// Remove the shadow fields from the data
// TODO: This does not work!!!
data =
JsonSerializer.Deserialize(serializedData, data.GetType())
?? throw new JsonException(
"Could not deserialize back datamodel after removing shadow fields. Data was \"null\""
);

Check warning

Code scanning / CodeQL

Useless assignment to local variable Warning

This assignment to
data
is useless, since its value is never read.
isModified = true; // TODO: Detect if modifications were made
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ public IEnumerable<IValidator> GetValidators(string taskId)
.Select(dev => new DataElementValidatorWrapper(dev, taskId, dataTypes))
);
validators.AddRange(
GetFormDataValidators(taskId, dataTypes).Select(fdv => new FormDataValidatorWrapper(fdv, taskId, dataTypes))
GetFormDataValidators(taskId, dataTypes).Select(fdv => new FormDataValidatorWrapper(fdv, taskId))
);

// add legacy instance validators wrapped in IValidator wrappers
Expand Down
10 changes: 2 additions & 8 deletions src/Altinn.App.Core/Internal/Validation/ValidationService.cs
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
using Altinn.App.Core.Features;
using Altinn.App.Core.Internal.App;
using Altinn.App.Core.Internal.AppModel;
using Altinn.App.Core.Internal.Data;
using Altinn.App.Core.Models.Validation;
using Altinn.Platform.Storage.Interface.Models;
using Microsoft.Extensions.Logging;
Expand All @@ -14,7 +11,6 @@ namespace Altinn.App.Core.Internal.Validation;
public class ValidationService : IValidationService
{
private readonly IValidatorFactory _validatorFactory;
private readonly IAppMetadata _appMetadata;
private readonly ILogger<ValidationService> _logger;
private readonly Telemetry? _telemetry;

Expand All @@ -23,13 +19,11 @@ public class ValidationService : IValidationService
/// </summary>
public ValidationService(
IValidatorFactory validatorFactory,
IAppMetadata appMetadata,
ILogger<ValidationService> logger,
Telemetry? telemetry = null
)
{
_validatorFactory = validatorFactory;
_appMetadata = appMetadata;
_logger = logger;
_telemetry = telemetry;
}
Expand Down Expand Up @@ -64,7 +58,7 @@ public async Task<List<ValidationIssueWithSource>> ValidateInstanceAtTask(
{
_logger.LogError(
e,
"Error while running validator {validatorName} for task {taskId} on instance {instanceId}",
"Error while running validator {ValidatorName} for task {TaskId} on instance {InstanceId}",
v.ValidationSource,
taskId,

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
instance.Id

Check failure

Code scanning / CodeQL

Log entries created from user input High

This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
This log entry depends on a
user-provided value
.
Expand Down Expand Up @@ -144,7 +138,7 @@ public async Task<Dictionary<string, List<ValidationIssueWithSource>>> ValidateI
return lists.Where(k => k.Value is not null).ToDictionary(kv => kv.Key, kv => kv.Value!);
}

private void ThrowIfDuplicateValidators(IValidator[] validators, string taskId)
private static void ThrowIfDuplicateValidators(IValidator[] validators, string taskId)
{
var sourceNames = validators
.Select(v => v.ValidationSource)
Expand Down
1 change: 0 additions & 1 deletion src/Altinn.App.Core/Models/Validation/ValidationIssue.cs
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
using System.Text.Json;
using System.Text.Json.Serialization;
using Altinn.App.Core.Internal.Validation;
using Newtonsoft.Json;

namespace Altinn.App.Core.Models.Validation;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ public async Task ValidateFormData_WithErrors()
)
.Verifiable(Times.Once);

_instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data);
_instanceDataAccessor.Setup(ida => ida.GetData(_dataElement)).ReturnsAsync(data);

// Act
var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null);
Expand Down Expand Up @@ -151,7 +151,7 @@ public async Task ValidateErrorAndMappingWithCustomModel(string errorKey, string
}
)
.Verifiable(Times.Once);
_instanceDataAccessor.Setup(ida => ida.Get(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once);
_instanceDataAccessor.Setup(ida => ida.GetData(_dataElement)).ReturnsAsync(data).Verifiable(Times.Once);

// Act
var result = await _validator.Validate(_instance, _instanceDataAccessor.Object, "Task_1", null);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ public PatchServiceTests()
[],
_appMetadataMock.Object
);
var validationService = new ValidationService(validatorFactory, _appMetadataMock.Object, _vLoggerMock.Object);
var validationService = new ValidationService(validatorFactory, _vLoggerMock.Object);

_patchService = new PatchService(
_appMetadataMock.Object,
Expand Down

0 comments on commit e6cda48

Please sign in to comment.