Remove hidden data before validation and make clean data available in DataProcessor #1279

ivarne · 2025-04-29T13:29:50Z

Now all data is cleaned for all (backend) validation if AppSettings.RemoveHiddenData == true and IInstanceDataAccessor has a new method GetCleanDataAccessor so that a DataProcessor (or UserAction) can easily get a copy of the data with hidden data removed for calculations or similar.

One key factor here is that we add a source generator to be able to work with data models more efficiently

During self review I tried to split the PR into smaller commits that stands on its own

63018b3 Move Altinn.App.Analyzer from .Core to .Api
ecf795d Make changes for more efficient removal of hidden data
- Introduce IFormDataWrapper
- Deprecate DataModelWrapper
- New helpers in IInstanceDataAccessor
1f015ef Add soure generator for IFormDataWrapper implemenations in .Analyzers
This includes two test projects.
- SourceGenerator.Tests: Use the generator directly and verify text output
- SourceGenerator.Integration.Tests: Reference the generator in build process and test the generated code
This has to be two projects, because it is really hard to debug a source generator when it runs as part of the build process.
67d2df5 Add Clean and Previous InstanceDataAccessors
This enables DataProcessors to get the clean versions of data models for calculations.
df07e13 Clean data before validation if AppSettings.RemoveHiddenData = true
6589b33 Minor fixes

Related Issue(s)

Form data: Remove hidden values #1157

Verification

Your code builds clean without any errors or warnings
Manual testing done (required)
Relevant automated test added (if you find this hard, leave it and we'll help out)
All tests run green

Documentation

User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

ivarne · 2025-04-29T13:33:21Z

/publish

github-actions · 2025-04-29T13:33:40Z

PR release:

⚙️ Building...
✅ Done!

src/Altinn.App.Analyzers/SourceTextGenerator/AddIndexToPathGenerator.cs

src/Altinn.App.Analyzers/SourceTextGenerator/SourceReaderUtils.cs

src/Altinn.App.Analyzers/Utils/EquatableArray.cs

src/Altinn.App.Analyzers/Utils/NanoJsonReader.cs

src/Altinn.App.Analyzers/Utils/RoslynExtensions.cs

src/Altinn.App.Core/Features/IFormDataWrapper.cs

ivarne · 2025-04-29T20:43:04Z

/publish

github-actions · 2025-04-29T20:43:26Z

PR release:

⚙️ Building...
✅ Done!

src/Altinn.App.Analyzers/SourceTextGenerator/AltinnRowIdsGenerator.cs

ivarne · 2025-05-07T07:15:02Z

/publish

github-actions · 2025-05-07T07:15:23Z

PR release:

⚙️ Building...
✅ Done!

* Introduce IFormDataWrapper * Deprecate DataModelWrapper * New helpers in IInstanceDataAccessor

This includes two test projects. * SourceGenerator.Tests: Use the generator directly and verify text output * SourceGenerator.Integration.Tests: Reference the generator in build process and test the generated code This has to be two projects, because it is really hard to debug a source generator when it runs as part of the build process.

This enables DataProcessors to get the clean versions of data models for calculations.

ivarne · 2025-05-13T07:04:57Z

/publish

github-actions · 2025-05-13T07:05:18Z

PR release:

⚙️ Building...
✅ Done!

src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs

martinothamar · 2025-05-13T09:02:15Z

src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs

+            {
+                if (dataType.Type != JsonType.Object)
+                {
+                    continue;


This branch could emit a diagnostic, all the values in dataTypes should be objects

Yes, I definitely could add more diagnostics. Not sure I want to do that, because diagnostics from incremental source generators is pretty buggy. I wanted to have a diagnostic for not finding "ClassRef" in compilation, and added some extra just because they were easy.

Reading up on the whole "diagnostics from incremental generators" topic, it seems like there is no intention to have a create story there. Maybe we should go in the opposite direction then? Drop all diagnostics here and rather implement analyzers if we feel it is necessary?

src/Altinn.App.Analyzers/IncrementalGenerator/FormDataWrapperGenerator.cs

martinothamar · 2025-05-13T09:18:47Z

src/Altinn.App.Analyzers/ModelPathNode.cs

+    /// <summary>
+    /// Initializes a new instance of the <see cref="ModelPathNode"/> class.
+    /// </summary>
+    public ModelPathNode(


It wasn't obvious to me why cSharpName and jsonName could be empty in some cases. That is for the root node right? Maybe we could have static named constructors like CreateRoot and CreateProperty or something. Type hierarchy/union pattern is also an option

That is for the root node right?

Yes, it is only for root.

I think the proper solution would be to create two different types. Node(List<Property>) and Property(cSharpName, jsonName, node).

(Yes, I checked and that is what PolyType does have two levels)

Do you think clarity is worth the rewrite?

I think it is worth the rewrite at some point. If we do that now as part of this PR or in the future I'll leave for you to decide 😄

src/Altinn.App.Analyzers/SourceTextGenerator/RemoveGenerator.cs

martinothamar · 2025-05-13T09:39:19Z

src/Altinn.App.Analyzers/SourceTextGenerator/SourceReaderUtils.cs

+                // Arrays have never been supported in data models.
+                // Just ignore them for now.
+                // return (UnwrapNullable(arrayTypeSymbol.ElementType), arrayTypeSymbol);
+                throw new NotSupportedException(


Should this function create diagnostics instead of throwing?

Yes, probably, but since 8.0.0 we crash at runtime if anyone has an array in their data model, and nobody complains (probably because they generate models in studio.)

martinothamar · 2025-05-13T09:41:21Z

src/Altinn.App.Analyzers/SourceTextGenerator/SourceReaderUtils.cs

+                    return (UnwrapNullable(collectionTypeSymbol.TypeArguments[0]), namedTypeSymbol);
+                }
+
+                break;


Is it always ICollection<>? Feels a little brittle. We could build a set of collection symbols in an earlier stage and do set lookups instead

The proper thing to do would probably be to use whatever System.Text.Json / PolyType does.

I think we should either try to do the simple thing or rely on a library.

src/Altinn.App.Analyzers/SourceTextGenerator/SourceTextGenerator.cs

src/Altinn.App.Analyzers/Utils/EquitableDiagnostic.cs

Backend currently have 3 failiure conditions where a data model for a reference is not found. * It does not exist in applicationmetadata * No dataElement has yet been created * The dataType has maxCount != 1 Currently the backend message is "DataType non-existing not found in applicationmetadata.json", which is more specific so I modified the shared tests.

ivarne · 2025-05-15T09:39:06Z

/publish

github-actions · 2025-05-15T09:39:29Z

PR release:

⚙️ Building...
✅ Done!

…th CleanDataAccessor when RemoveHiddenData=true

…rmDataValidator along with NoIncrementalValidation

The generated code could not add indexes to the end

If a List backing a repeating group contains a null, that is highly likely caused by cleaning the rows

src/Altinn.App.Core/Internal/Expressions/LayoutEvaluatorState.cs

+            Field =
+                formDataWrapper.AddIndexToPath(binding.Field, indexes)
+                ?? throw new InvalidOperationException(
+                    $"Failed to add indexes to path {binding.Field} with indexes {indexes} on {dataElementId}"


To fix the issue, we need to replace the default ToString() call on the indexes array with a more meaningful string representation of its contents. The best approach is to use string.Join(", ", indexes) to concatenate the array elements into a comma-separated string. This will ensure that the error message provides a clear and useful representation of the indexes array.

The change will be made in the exception message on line 308. No additional imports or method definitions are required, as string.Join is part of the System namespace, which is already available.

sonarqubecloud · 2025-05-27T09:34:00Z

Quality Gate failed

Failed conditions
63.81% Condition Coverage on New Code (required ≥ 65%)

See analysis details on SonarQube Cloud

ivarne · 2025-05-27T12:46:11Z

/publish

github-actions · 2025-05-27T12:46:29Z

PR release:

⚙️ Building...
✅ Done!

ivarne added backport-ignore This PR is a new feature and should not be cherry-picked onto release branches feature Label Pull requests with new features. Used when generation releasenotes labels Apr 29, 2025

github-advanced-security bot found potential problems Apr 29, 2025

View reviewed changes

ivarne force-pushed the feat/cleanDataAccessor branch from 11165d0 to d6c3aff Compare April 30, 2025 13:12

github-advanced-security bot found potential problems Apr 30, 2025

View reviewed changes

src/Altinn.App.Analyzers/SourceTextGenerator/AltinnRowIdsGenerator.cs Fixed Show fixed Hide fixed

ivarne force-pushed the feat/cleanDataAccessor branch from b878697 to 92c03b0 Compare May 7, 2025 06:24

Move Altinn.App.Analyzer from .Core to .Api

63018b3

ivarne force-pushed the feat/cleanDataAccessor branch 5 times, most recently from dc750e9 to 3987785 Compare May 10, 2025 19:53

Make changes for more efficient removal of hidden data

ecf795d

* Introduce IFormDataWrapper * Deprecate DataModelWrapper * New helpers in IInstanceDataAccessor

ivarne force-pushed the feat/cleanDataAccessor branch 3 times, most recently from 3fdef2c to f0f764f Compare May 12, 2025 12:39

ivarne force-pushed the feat/cleanDataAccessor branch from f0f764f to 1f015ef Compare May 12, 2025 12:54

ivarne added 3 commits May 12, 2025 14:57

Add Clean and Previous InstanceDataAccessors

67d2df5

This enables DataProcessors to get the clean versions of data models for calculations.

Clean data before validation if AppSettings.RemoveHiddenData = true

df07e13

Minor fixes

6589b33

ivarne changed the title ~~Feat/clean data accessor~~ Remove hidden data before validation and make clean data available in DataProcessor May 12, 2025

ivarne marked this pull request as ready for review May 12, 2025 13:22

ivarne requested a review from martinothamar May 12, 2025 13:31

ivarne added 2 commits May 12, 2025 22:54

Improve test coverage

d636c69

sonar issues

984d64e

martinothamar reviewed May 13, 2025

View reviewed changes

ivarne added 2 commits May 13, 2025 14:01

Fix code review feedback

20fb349

Merge remote-tracking branch 'origin/main' into feat/cleanDataAccessor

f9083e7

ivarne mentioned this pull request May 14, 2025

Use strict mocking in signing tests to ensure that we do proper Setup() #1314

Merged

ivarne added 2 commits May 15, 2025 08:28

Use strict mocking in signing tests to ensure that we do proper Setup()

8412245

ivarne added 11 commits May 16, 2025 08:40

Use v4 of json schema for $schema annotations in layouts

b1e86e7

Merge remote-tracking branch 'origin/main' into feat/cleanDataAccessor

ccb4702

Cleanup

1d2eb3d

Add more tests for the RequiredLayoutValidator to see how it works wi…

760a0f9

…th CleanDataAccessor when RemoveHiddenData=true

Add ShouldRunAfterRemovingHiddenData to IValidator and proxy into IFo…

fc2dfe4

…rmDataValidator along with NoIncrementalValidation

Add more telemetry to iinstanceDataAccessor

3b675ed

Fix bug in AddIndexToPathGenerator

688a716

The generated code could not add indexes to the end

Minor telemetry improvements

c501173

Store reference to State in Component context and Obsolete DataModel

243a33e

Check rows for null when evaluating hidden expressions

9524933

If a List backing a repeating group contains a null, that is highly likely caused by cleaning the rows

Merge remote-tracking branch 'origin/main' into feat/cleanDataAccessor

22b4eea

github-advanced-security bot found potential problems May 27, 2025

View reviewed changes

Remove hidden data before validation and make clean data available in DataProcessor #1279

Are you sure you want to change the base?

Remove hidden data before validation and make clean data available in DataProcessor #1279

Uh oh!

Conversation

ivarne commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Related Issue(s)

Verification

Documentation

Uh oh!

ivarne commented Apr 29, 2025

Uh oh!

github-actions bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR release:

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ivarne commented Apr 29, 2025

Uh oh!

github-actions bot commented Apr 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR release:

Uh oh!

Uh oh!

ivarne commented May 7, 2025

Uh oh!

github-actions bot commented May 7, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR release:

Uh oh!

ivarne commented May 13, 2025

Uh oh!

github-actions bot commented May 13, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR release:

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ivarne commented May 15, 2025

Uh oh!

github-actions bot commented May 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR release:

Uh oh!

Check warning

Uh oh!

ivarne commented Apr 29, 2025 •

edited

Loading

github-actions bot commented Apr 29, 2025 •

edited

Loading

github-actions bot commented Apr 29, 2025 •

edited

Loading

github-actions bot commented May 7, 2025 •

edited

Loading

github-actions bot commented May 13, 2025 •

edited

Loading

github-actions bot commented May 15, 2025 •

edited

Loading

github-actions bot commented May 27, 2025 •

edited

Loading