Add separate context for rows in expression evaluation (#979)

* Improve debugging and fix typos

* Remove logic for handeling "hiddenRows" without separate row context (cleanup)

* Generate row contexts in order to support "hiddenRow" expressions on repeating groups

Previously this was implemented as a special case, but this makes the code simpler and fixes bugs I did not understand.

src/Altinn.App.Core/Features/Validation/Default/ExpressionValidator.cs

@@ -139,7 +139,6 @@ internal async Task<List<ValidationIssue>> ValidateFormData(
                 var context = new ComponentContext(
                     component: null,
                     rowIndices: DataModel.GetRowIndices(resolvedField.Field),
-                    rowLength: null,
                     dataElementIdentifier: resolvedField.DataElementIdentifier
                 );
                 var positionalArguments = new object[] { resolvedField };

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

@@ -29,8 +29,6 @@ bool defaultReturn
             var expr = property switch
             {
                 "hidden" => context.Component.Hidden,
-                "hiddenRow" when context.Component is RepeatingGroupComponent repeatingGroup =>
-                    repeatingGroup.HiddenRow,
                 "required" => context.Component.Required,
                 _ => throw new ExpressionEvaluatorTypeErrorException($"unknown boolean expression property {property}"),
             };

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

@@ -22,14 +22,15 @@ public static async Task<List<DataReference>> GetHiddenFieldsForRemoval(
         var hiddenModelBindings = new HashSet<DataReference>();
         var nonHiddenModelBindings = new HashSet<DataReference>();
 
-        foreach (var context in await state.GetComponentContexts())
+        var pageContexts = await state.GetComponentContexts();
+        foreach (var pageContext in pageContexts)
         {
             await HiddenFieldsForRemovalRecurs(
                 state,
                 includeHiddenRowChildren,
                 hiddenModelBindings,
                 nonHiddenModelBindings,
-                context
+                pageContext
             );
         }
 
@@ -45,21 +46,31 @@ private static async Task HiddenFieldsForRemovalRecurs(
         ComponentContext context
     )
     {
-        // Recurse children
-        foreach (var childContext in context.ChildContexts)
+        if (context.Component is null)
         {
-            // Ignore children of hidden rows if includeHiddenRowChildren is false
-            if (!includeHiddenRowChildren && childContext.Component is RepeatingGroupComponent)
+            throw new ArgumentNullException(
+                nameof(context),
+                "Context must have a component when removing hidden fields"
+            );
+        }
+
+        var isHidden = await context.IsHidden(state);
+        // Ignore children of hidden rows if includeHiddenRowChildren is false
+        if (!includeHiddenRowChildren && isHidden)
+        {
+            if (context.Component is RepeatingGroupRowComponent or RepeatingGroupComponent)
             {
-                var hiddenRows = await childContext.GetHiddenRows(state);
-                var currentRow = childContext.RowIndices?[^1];
-                var rowIsHidden = currentRow is not null && hiddenRows[currentRow.Value];
-                if (rowIsHidden)
+                if (context.Component.DataModelBindings.TryGetValue("group", out var groupBinding))
                 {
-                    continue;
+                    var indexedBinding = await state.AddInidicies(groupBinding, context);
+                    hiddenModelBindings.Add(indexedBinding);
                 }
+                return;
             }
-
+        }
+        // Recurse children
+        foreach (var childContext in context.ChildContexts)
+        {
             await HiddenFieldsForRemovalRecurs(
                 state,
                 includeHiddenRowChildren,
@@ -69,52 +80,23 @@ await HiddenFieldsForRemovalRecurs(
             );
         }
 
-        // Get dataReference for hidden rows
-        if (context is { Component: RepeatingGroupComponent repGroup })
-        {
-            var hiddenRows = await context.GetHiddenRows(state);
-            // Reverse order to get the last hidden row first so that the index is correct when removing from the data object
-            foreach (var index in Enumerable.Range(0, hiddenRows.Length).Reverse())
-            {
-                var rowIndices = context.RowIndices?.Append(index).ToArray() ?? [index];
-                var indexedBinding = await state.AddInidicies(
-                    repGroup.DataModelBindings["group"],
-                    context.DataElementIdentifier,
-                    rowIndices
-                );
-
-                if (hiddenRows[index])
-                {
-                    hiddenModelBindings.Add(indexedBinding);
-                }
-                else
-                {
-                    nonHiddenModelBindings.Add(indexedBinding);
-                }
-            }
-        }
-
         // Remove data if hidden
-        if (context.Component is not null)
+        foreach (var (bindingName, binding) in context.Component.DataModelBindings)
         {
-            foreach (var (bindingName, binding) in context.Component.DataModelBindings)
+            if (bindingName == "group")
             {
-                if (bindingName == "group")
-                {
-                    continue;
-                }
+                continue;
+            }
 
-                var indexedBinding = await state.AddInidicies(binding, context);
-                var isHidden = await context.IsHidden(state);
+            var indexedBinding = await state.AddInidicies(binding, context);
 
-                if (isHidden)
-                {
-                    hiddenModelBindings.Add(indexedBinding);
-                }
-                else
-                {
-                    nonHiddenModelBindings.Add(indexedBinding);
-                }
+            if (isHidden)
+            {
+                hiddenModelBindings.Add(indexedBinding);
+            }
+            else
+            {
+                nonHiddenModelBindings.Add(indexedBinding);
             }
         }
     }

src/Altinn.App.Core/Internal/Process/ExpressionsExclusiveGateway.cs

@@ -91,12 +91,7 @@ ProcessGatewayInformation processGatewayInformation
             DataElementIdentifier dataElement =
                 instance.Data.Find(d => d.DataType == dataTypeId) ?? new DataElementIdentifier();
 
-            var componentContext = new ComponentContext(
-                component: null,
-                rowIndices: null,
-                rowLength: null,
-                dataElement
-            );
+            var componentContext = new ComponentContext(component: null, rowIndices: null, dataElement);
             var result = await ExpressionEvaluator.EvaluateExpression(state, expression, componentContext);
             if (result is true)
             {

src/Altinn.App.Core/Models/Expressions/ComponentContext.cs

@@ -1,4 +1,4 @@
-using System.Collections;
+using System.Diagnostics;
 using Altinn.App.Core.Internal.Expressions;
 using Altinn.App.Core.Models.Layout;
 using Altinn.App.Core.Models.Layout.Components;
@@ -8,25 +8,23 @@ namespace Altinn.App.Core.Models.Expressions;
 /// <summary>
 /// Simple class for holding the context for <see cref="ExpressionEvaluator"/>
 /// </summary>
+[DebuggerDisplay("{_debuggerDisplay}", Name = "{_debuggerName}")]
+[DebuggerTypeProxy(typeof(DebuggerProxy))]
 public sealed class ComponentContext
 {
-    private readonly int? _rowLength;
-
     /// <summary>
     /// Constructor for ComponentContext
     /// </summary>
     public ComponentContext(
         BaseComponent? component,
         int[]? rowIndices,
-        int? rowLength,
         DataElementIdentifier dataElementIdentifier,
-        IEnumerable<ComponentContext>? childContexts = null
+        List<ComponentContext>? childContexts = null
     )
     {
         DataElementIdentifier = dataElementIdentifier;
         Component = component;
         RowIndices = rowIndices;
-        _rowLength = rowLength;
         ChildContexts = childContexts ?? [];
         foreach (var child in ChildContexts)
         {
@@ -65,53 +63,10 @@ public async Task<bool> IsHidden(LayoutEvaluatorState state)
         return _isHidden.Value;
     }
 
-    private BitArray? _hiddenRows;
-
-    /// <summary>
-    /// Hidden rows for repeating group
-    /// </summary>
-    public async Task<BitArray> GetHiddenRows(LayoutEvaluatorState state)
-    {
-        if (_hiddenRows is not null)
-        {
-            return _hiddenRows;
-        }
-        if (Component is not RepeatingGroupComponent)
-        {
-            throw new InvalidOperationException("HiddenRows can only be called on a repeating group");
-        }
-        if (_rowLength is null)
-        {
-            throw new InvalidOperationException("RowLength must be set to call HiddenRows on repeating group");
-        }
-
-        var hiddenRows = new BitArray(_rowLength.Value);
-        foreach (var index in Enumerable.Range(0, hiddenRows.Length))
-        {
-            var rowIndices = RowIndices?.Append(index).ToArray() ?? [index];
-            var childContexts = ChildContexts.Where(c => c.RowIndices?[RowIndices?.Length ?? 0] == index);
-            // Row contexts are not in the tree, so we need to create them here
-            var rowContext = new ComponentContext(
-                Component,
-                rowIndices,
-                rowLength: hiddenRows.Length,
-                dataElementIdentifier: DataElementIdentifier,
-                childContexts: childContexts
-            );
-            var rowHidden = await ExpressionEvaluator.EvaluateBooleanExpression(state, rowContext, "hiddenRow", false);
-
-            hiddenRows[index] = rowHidden;
-        }
-
-        // Set the hidden rows so that it does not need to be recomputed
-        _hiddenRows = hiddenRows;
-        return _hiddenRows;
-    }
-
     /// <summary>
     /// Contexts that logically belongs under this context (eg cell => row => group=> page)
     /// </summary>
-    public IEnumerable<ComponentContext> ChildContexts { get; }
+    public List<ComponentContext> ChildContexts { get; }
 
     /// <summary>
     /// Parent context or null, if this is a root context, or a context created without setting parent
@@ -140,4 +95,26 @@ public IEnumerable<ComponentContext> Descendants
             }
         }
     }
+
+    private string _debuggerName =>
+        $"{Component?.Type}" + (RowIndices is not null ? $"[{string.Join(',', RowIndices)}]" : "");
+    private string _debuggerDisplay =>
+        $"id:\"{Component?.Id}\"" + (ChildContexts.Count > 0 ? $" ({ChildContexts.Count} children)" : "");
+
+    private class DebuggerProxy
+    {
+        private readonly ComponentContext _context;
+
+        [DebuggerBrowsable(DebuggerBrowsableState.RootHidden)]
+        public List<ComponentContext> ChildContexts => _context.ChildContexts;
+        public BaseComponent? Component => _context.Component;
+        public ComponentContext? Parent => _context.Parent;
+        public bool? IsHidden => _context._isHidden;
+        public Guid DataElementId => _context.DataElementIdentifier.Guid;
+
+        public DebuggerProxy(ComponentContext context)
+        {
+            _context = context;
+        }
+    }
 }

src/Altinn.App.Core/Models/Layout/Components/BaseComponent.cs

@@ -4,7 +4,7 @@
 namespace Altinn.App.Core.Models.Layout.Components;
 
 /// <summary>
-/// Inteface to be able to handle all most components same way.
+/// Interface to be able to handle all most components same way.
 /// </summary>
 /// <remarks>
 /// See <see cref="GroupComponent" /> for any components that handle children.
@@ -36,7 +36,7 @@ public BaseComponent(
     }
 
     /// <summary>
-    /// ID of the component (or pagename for pages)
+    /// ID of the component (or pageName for pages)
     /// </summary>
     public string Id { get; }
 

src/Altinn.App.Core/Models/Layout/Components/RepeatingGroupComponent.cs

@@ -30,7 +30,7 @@ public RepeatingGroupComponent(
     }
 
     /// <summary>
-    /// Maximum number of repeatitions of this repating group
+    /// Maximum number of repetitions of this repeating group
     /// </summary>
     public int MaxCount { get; }
 
@@ -39,3 +39,31 @@ public RepeatingGroupComponent(
     /// </summary>
     public Expression HiddenRow { get; }
 }
+
+/// <summary>
+/// Component (currently only used for contexts to have something to point to) for a row in a repeating group
+/// </summary>
+public record RepeatingGroupRowComponent : BaseComponent
+{
+    /// <summary>
+    /// Constructor for RepeatingGroupRowComponent
+    /// </summary>
+    public RepeatingGroupRowComponent(
+        string id,
+        IReadOnlyDictionary<string, ModelBinding> dataModelBindings,
+        Expression hiddenRow,
+        BaseComponent parent
+    )
+        : base(
+            id,
+            "groupRow",
+            dataModelBindings,
+            hiddenRow,
+            required: Expression.False,
+            readOnly: Expression.False,
+            null
+        )
+    {
+        Parent = parent;
+    }
+}