-
Notifications
You must be signed in to change notification settings - Fork 25
Set data field by expression #1683
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
43f09b3
03339cd
dd637ed
53f29af
a11d093
ce5f067
c589a4c
1ea1ece
1f961e4
e9712ef
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,231 @@ | ||||||||||
| using System.Text.Json; | ||||||||||
| using Altinn.App.Core.Internal.App; | ||||||||||
| using Altinn.App.Core.Internal.Data; | ||||||||||
| using Altinn.App.Core.Internal.Expressions; | ||||||||||
| using Altinn.App.Core.Models; | ||||||||||
| using Altinn.App.Core.Models.Layout; | ||||||||||
| using Altinn.Platform.Storage.Interface.Models; | ||||||||||
| using Microsoft.Extensions.Logging; | ||||||||||
| using ComponentContext = Altinn.App.Core.Models.Expressions.ComponentContext; | ||||||||||
|
|
||||||||||
| namespace Altinn.App.Core.Features.DataProcessing; | ||||||||||
|
|
||||||||||
| internal sealed class DataFieldValueCalculator | ||||||||||
| { | ||||||||||
| private static readonly JsonSerializerOptions _jsonSerializerOptions = new() | ||||||||||
| { | ||||||||||
| ReadCommentHandling = JsonCommentHandling.Skip, | ||||||||||
| PropertyNamingPolicy = JsonNamingPolicy.CamelCase, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| private readonly ILogger<DataFieldValueCalculator> _logger; | ||||||||||
| private readonly IAppResources _appResourceService; | ||||||||||
| private readonly ILayoutEvaluatorStateInitializer _layoutEvaluatorStateInitializer; | ||||||||||
| private readonly IDataElementAccessChecker _dataElementAccessChecker; | ||||||||||
| private readonly Telemetry? _telemetry; | ||||||||||
|
|
||||||||||
| public DataFieldValueCalculator( | ||||||||||
| ILogger<DataFieldValueCalculator> logger, | ||||||||||
| ILayoutEvaluatorStateInitializer layoutEvaluatorStateInitializer, | ||||||||||
| IAppResources appResourceService, | ||||||||||
| IDataElementAccessChecker dataElementAccessChecker, | ||||||||||
| Telemetry? telemetry | ||||||||||
| ) | ||||||||||
| { | ||||||||||
| _logger = logger; | ||||||||||
| _appResourceService = appResourceService; | ||||||||||
| _layoutEvaluatorStateInitializer = layoutEvaluatorStateInitializer; | ||||||||||
| _dataElementAccessChecker = dataElementAccessChecker; | ||||||||||
| _telemetry = telemetry; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| public async Task Calculate(IInstanceDataAccessor dataAccessor, string taskId) | ||||||||||
| { | ||||||||||
| using var activity = _telemetry?.StartCalculateActivity(dataAccessor.Instance.Id, taskId); | ||||||||||
| foreach (var (dataType, dataElement) in dataAccessor.GetDataElementsWithFormDataForTask(taskId)) | ||||||||||
| { | ||||||||||
| if (await _dataElementAccessChecker.CanRead(dataAccessor.Instance, dataType) is false) | ||||||||||
|
Check warning on line 47 in src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.cs
|
||||||||||
| { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| var calculationConfig = _appResourceService.GetCalculationConfiguration(dataType.Id); | ||||||||||
| if (!string.IsNullOrEmpty(calculationConfig)) | ||||||||||
| { | ||||||||||
| await CalculateFormData(dataAccessor, dataElement, taskId, calculationConfig); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| internal async Task CalculateFormData( | ||||||||||
| IInstanceDataAccessor dataAccessor, | ||||||||||
| DataElement dataElement, | ||||||||||
| string taskId, | ||||||||||
| string rawCalculationConfig | ||||||||||
| ) | ||||||||||
| { | ||||||||||
| var evaluatorState = await _layoutEvaluatorStateInitializer.Init(dataAccessor, taskId); | ||||||||||
| var hiddenFields = await LayoutEvaluator.GetHiddenFieldsForRemoval( | ||||||||||
| evaluatorState, | ||||||||||
| evaluateRemoveWhenHidden: false | ||||||||||
| ); | ||||||||||
| DataElementIdentifier dataElementIdentifier = dataElement; | ||||||||||
| var dataFieldCalculations = ParseDataFieldCalculationConfig(rawCalculationConfig, _logger); | ||||||||||
| var formDataWrapper = await dataAccessor.GetFormDataWrapper(dataElement); | ||||||||||
|
|
||||||||||
| foreach (var (baseField, calculations) in dataFieldCalculations) | ||||||||||
| { | ||||||||||
| var resolvedFields = await evaluatorState.GetResolvedKeys( | ||||||||||
| new DataReference() { Field = baseField, DataElementIdentifier = dataElementIdentifier }, | ||||||||||
| true | ||||||||||
| ); | ||||||||||
| foreach (var resolvedField in resolvedFields) | ||||||||||
| { | ||||||||||
| if ( | ||||||||||
| hiddenFields.Exists(d => | ||||||||||
| d.DataElementIdentifier == resolvedField.DataElementIdentifier | ||||||||||
| && resolvedField.Field.StartsWith(d.Field, StringComparison.InvariantCulture) | ||||||||||
| ) | ||||||||||
| ) | ||||||||||
| { | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| var context = new ComponentContext( | ||||||||||
| evaluatorState, | ||||||||||
| component: null, | ||||||||||
| rowIndices: ExpressionHelper.GetRowIndices(resolvedField.Field), | ||||||||||
| dataElementIdentifier: resolvedField.DataElementIdentifier | ||||||||||
| ); | ||||||||||
| var positionalArguments = new object[] { resolvedField.Field }; | ||||||||||
| foreach (var calculation in calculations) | ||||||||||
| { | ||||||||||
| await RunCalculation( | ||||||||||
| formDataWrapper, | ||||||||||
| evaluatorState, | ||||||||||
| resolvedField, | ||||||||||
| context, | ||||||||||
| positionalArguments, | ||||||||||
| calculation | ||||||||||
| ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private async Task RunCalculation( | ||||||||||
| IFormDataWrapper formDataWrapper, | ||||||||||
| LayoutEvaluatorState evaluatorState, | ||||||||||
| DataReference resolvedField, | ||||||||||
| ComponentContext context, | ||||||||||
| object[] positionalArguments, | ||||||||||
| DataFieldCalculation calculation | ||||||||||
| ) | ||||||||||
| { | ||||||||||
| try | ||||||||||
| { | ||||||||||
| var calculationResult = await ExpressionEvaluator.EvaluateExpressionToExpressionValue( | ||||||||||
| evaluatorState, | ||||||||||
| calculation.Condition, | ||||||||||
| context, | ||||||||||
| positionalArguments | ||||||||||
| ); | ||||||||||
| if (!formDataWrapper.Set(resolvedField.Field, calculationResult)) | ||||||||||
| { | ||||||||||
| _logger.LogWarning( | ||||||||||
| "Could not set calculated value for field {Field} in data element {DataElementId}. " | ||||||||||
| + "This is because the type conversion failed.", | ||||||||||
| resolvedField.Field, | ||||||||||
| resolvedField.DataElementIdentifier.Id | ||||||||||
| ); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| catch (Exception e) | ||||||||||
|
Check warning on line 143 in src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.cs
|
||||||||||
| { | ||||||||||
| _logger.LogError(e, "Error while evaluating calculation for field {Field}", resolvedField.Field); | ||||||||||
| throw; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private Dictionary<string, List<DataFieldCalculation>> ParseDataFieldCalculationConfig( | ||||||||||
| string rawCalculationConfig, | ||||||||||
| ILogger<DataFieldValueCalculator> logger | ||||||||||
| ) | ||||||||||
| { | ||||||||||
| using var calculationConfigDocument = JsonDocument.Parse(rawCalculationConfig); | ||||||||||
|
|
||||||||||
| var dataFieldCalculations = new Dictionary<string, List<DataFieldCalculation>>(); | ||||||||||
| var hasCalculations = calculationConfigDocument.RootElement.TryGetProperty( | ||||||||||
| "calculations", | ||||||||||
| out JsonElement calculationsObject | ||||||||||
| ); | ||||||||||
| if (hasCalculations) | ||||||||||
| { | ||||||||||
| foreach (var calculationArray in calculationsObject.EnumerateObject()) | ||||||||||
| { | ||||||||||
| var field = calculationArray.Name; | ||||||||||
| var calculations = calculationArray.Value; | ||||||||||
| foreach (var calculation in calculations.EnumerateArray()) | ||||||||||
| { | ||||||||||
| if (!dataFieldCalculations.TryGetValue(field, out var dataFieldCalculation)) | ||||||||||
| { | ||||||||||
| dataFieldCalculation = new List<DataFieldCalculation>(); | ||||||||||
| dataFieldCalculations[field] = dataFieldCalculation; | ||||||||||
| } | ||||||||||
| var resolvedDataFieldCalculation = ResolveDataFieldCalculation(field, calculation, logger); | ||||||||||
| if (resolvedDataFieldCalculation == null) | ||||||||||
| { | ||||||||||
| logger.LogError("Calculation for field {Field} could not be resolved", field); | ||||||||||
| continue; | ||||||||||
| } | ||||||||||
| dataFieldCalculation.Add(resolvedDataFieldCalculation); | ||||||||||
| } | ||||||||||
| } | ||||||||||
| } | ||||||||||
| return dataFieldCalculations; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| private DataFieldCalculation? ResolveDataFieldCalculation(string field, JsonElement definition, ILogger logger) | ||||||||||
|
Check failure on line 188 in src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.cs
|
||||||||||
| { | ||||||||||
|
Comment on lines
+188
to
+189
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: # First, let's read the file to see the method implementation
head -n 250 src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.cs | tail -n +180Repository: Altinn/app-lib-dotnet Length of output: 1807 🏁 Script executed: # Check if _jsonSerializerOptions is static or instance field
rg "_jsonSerializerOptions" src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.cs -A 2 -B 2Repository: Altinn/app-lib-dotnet Length of output: 491 🏁 Script executed: # Search for all calls to ResolveDataFieldCalculation
rg "ResolveDataFieldCalculation" src/Altinn.App.Core/Features/DataProcessing/DataFieldValueCalculator.csRepository: Altinn/app-lib-dotnet Length of output: 293 Make The method only uses its parameters and the static 🛠️ Proposed fix- private DataFieldCalculation? ResolveDataFieldCalculation(string field, JsonElement definition, ILogger logger)
+ private static DataFieldCalculation? ResolveDataFieldCalculation(string field, JsonElement definition, ILogger logger)📝 Committable suggestion
Suggested change
🧰 Tools🪛 GitHub Actions: Build and Test on windows, macos and ubuntu[error] 188-188: CA1822: Member 'ResolveDataFieldCalculation' does not access instance data and can be marked as static. 🪛 GitHub Actions: CodeQL[error] 188-188: CA1822: Member 'ResolveDataFieldCalculation' does not access instance data and can be marked as static (https://learn.microsoft.com/dotnet/fundamentals/code-analysis/quality-rules/ca1822) 🪛 GitHub Check: Analyze (csharp)[failure] 188-188: [failure] 188-188: [failure] 188-188: 🪛 GitHub Check: Run dotnet build and test (macos-latest)[failure] 188-188: [failure] 188-188: 🤖 Prompt for AI Agents |
||||||||||
| var rawDataFieldValueCalculation = new RawDataFieldValueCalculation(); | ||||||||||
|
|
||||||||||
| if (definition.ValueKind == JsonValueKind.String) | ||||||||||
| { | ||||||||||
| var stringReference = definition.GetString(); | ||||||||||
| if (stringReference == null) | ||||||||||
| { | ||||||||||
| logger.LogError("Could not resolve null reference for calculation for field {Field}", field); | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| else | ||||||||||
| { | ||||||||||
| var dataFieldCalculationDefinition = definition.Deserialize<RawDataFieldValueCalculation>( | ||||||||||
| _jsonSerializerOptions | ||||||||||
| ); | ||||||||||
| if (dataFieldCalculationDefinition == null) | ||||||||||
| { | ||||||||||
| logger.LogError("Calculation for field {Field} could not be parsed", field); | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (dataFieldCalculationDefinition.Condition != null) | ||||||||||
| { | ||||||||||
| rawDataFieldValueCalculation.Condition = dataFieldCalculationDefinition.Condition; | ||||||||||
| } | ||||||||||
| } | ||||||||||
|
|
||||||||||
| if (rawDataFieldValueCalculation.Condition == null) | ||||||||||
| { | ||||||||||
| logger.LogError("Calculation for field {Field} is missing condition", field); | ||||||||||
| return null; | ||||||||||
| } | ||||||||||
|
|
||||||||||
| var dataFieldCalculation = new DataFieldCalculation | ||||||||||
| { | ||||||||||
| Condition = rawDataFieldValueCalculation.Condition.Value, | ||||||||||
| }; | ||||||||||
|
|
||||||||||
| return dataFieldCalculation; | ||||||||||
| } | ||||||||||
| } | ||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,37 @@ | ||
| using Altinn.App.Core.Models; | ||
|
|
||
| namespace Altinn.App.Core.Features.DataProcessing; | ||
|
|
||
| /// <summary> | ||
| /// Processing data fields values that is calculated by expressions provided in [modelName].calculation.json. | ||
| /// </summary> | ||
| internal sealed class DataFieldValueCalculatorProcessor : IDataWriteProcessor | ||
| { | ||
| private readonly DataFieldValueCalculator _dataFieldValueCalculator; | ||
|
|
||
| /// <summary> | ||
| /// Initializes a new instance of the <see cref="DataFieldValueCalculatorProcessor"/> class. | ||
| /// </summary> | ||
| /// <param name="dataFieldValueCalculator"></param> | ||
| public DataFieldValueCalculatorProcessor(DataFieldValueCalculator dataFieldValueCalculator) | ||
| { | ||
| _dataFieldValueCalculator = dataFieldValueCalculator; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Processes data write operations on properties in the data model. | ||
| /// </summary> | ||
| /// <param name="instanceDataMutator">Object to fetch data elements not included in changes</param> | ||
| /// <param name="taskId">The current task ID</param> | ||
| /// <param name="changes">Not used in this context</param> | ||
| /// <param name="language">Not used in this context</param> | ||
| public async Task ProcessDataWrite( | ||
| IInstanceDataMutator instanceDataMutator, | ||
| string taskId, | ||
| DataElementChanges changes, | ||
| string? language | ||
| ) | ||
| { | ||
| await _dataFieldValueCalculator.Calculate(instanceDataMutator, taskId); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| using System.Diagnostics; | ||
| using static Altinn.App.Core.Features.Telemetry.DataFieldValueCalculator; | ||
|
|
||
| namespace Altinn.App.Core.Features; | ||
|
|
||
| partial class Telemetry | ||
| { | ||
| internal Activity? StartCalculateActivity(string instanceId, string taskId) | ||
| { | ||
| var activity = ActivitySource.StartActivity($"{Prefix}.Calculate"); | ||
| activity?.SetInstanceId(instanceId); | ||
| activity?.SetTaskId(taskId); | ||
| return activity; | ||
| } | ||
|
|
||
| internal static class DataFieldValueCalculator | ||
| { | ||
| internal const string Prefix = "DataFieldValueCalculator"; | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StartsWithfiltering can skip unrelated fields.resolvedField.Field.StartsWith(d.Field, ...)is too broad and can match sibling names. Use an exact-or-descendant path check with segment boundaries.🐛 Proposed fix
🤖 Prompt for AI Agents