refactor to avoid hacky list manipulation

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

@@ -12,8 +12,8 @@ public class GridComponent : GroupComponent
     /// <summary>
     /// Constructor for RepeatingGroupComponent
     /// </summary>
-    public GridComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
-        base(id, type, dataModelBindings, children, hidden, required, readOnly, additionalProperties)
+    public GridComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, IEnumerable<string>? childIDs, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
+        base(id, type, dataModelBindings, children, childIDs, hidden, required, readOnly, additionalProperties)
     { }
 }
 

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

@@ -12,18 +12,76 @@ public class GroupComponent : BaseComponent
     /// <summary>
     /// Constructor for GroupComponent
     /// </summary>
-    public GroupComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
+    public GroupComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, IEnumerable<string>? childIDs, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
         base(id, type, dataModelBindings, hidden, required, readOnly, additionalProperties)
     {
+
         Children = children;
+        ChildIDs = childIDs ?? children.Select(c => c.Id);
         foreach (var child in Children)
         {
             child.Parent = this;
         }
     }
-    
+
     /// <summary>
     /// The children in this group/page
     /// </summary>
-    public IEnumerable<BaseComponent> Children { get; internal set; }
-}
+    public IEnumerable<BaseComponent> Children { get; private set; }
+
+    /// <summary>
+    /// The child IDs in this group/page
+    /// </summary>
+    public IEnumerable<string> ChildIDs { get; internal set; }
+
+    /// <summary>
+    /// Adds a child component which is already defined in its child IDs
+    /// </summary>
+    public virtual void AddChild(BaseComponent child)
+    {
+        if (!this.ChildIDs.Contains(child.Id))
+        {
+            throw new ArgumentException($"Child with id {child.Id} is not defined in the child IDs of this group");
+        }
+        if (this.Children.FirstOrDefault(c => c.Id == child.Id) != null)
+        {
+            throw new ArgumentException($"Child with id {child.Id} is already added to this group");
+        }
+        child.Parent = this;
+        this.Children = this.Children.Append(child);
+    }
+
+    /// <summary>
+    /// Validates that the children in this group matches the child IDs and orders the children according to the child IDs
+    /// </summary>
+    public void ValidateChildren()
+    {
+        var childIDs = this.ChildIDs.ToList();
+
+        foreach (var childID in childIDs)
+        {
+            if (!this.Children.Select(c => c.Id).Contains(childID))
+            {
+                throw new ArgumentException($"Child with id {childID} could not be found for the group {this.Id}");
+            }
+        }
+
+        var childIDCount = childIDs.Count;
+        var childCount = this.Children.Count();
+
+        if (childCount != childIDCount)
+        {
+            throw new ArgumentException($"The number of children ({childCount}) in group {this.Id} does not match the number of child IDs provided ({childIDCount})");
+        }
+
+        this.Children = this.Children.OrderBy(c => childIDs.IndexOf(c.Id));
+
+        foreach (var child in this.Children)
+        {
+            if (child is GroupComponent group)
+            {
+                group.ValidateChildren();
+            }
+        }
+    }
+}

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

@@ -1,5 +1,3 @@
-using System.Collections.Immutable;
-using System.Text.Json;
 using System.Text.Json.Serialization;
 
 using Altinn.App.Core.Models.Expressions;
@@ -16,7 +14,7 @@ public class PageComponent : GroupComponent
     /// Constructor for PageComponent
     /// </summary>
     public PageComponent(string id, List<BaseComponent> children, Dictionary<string, BaseComponent> componentLookup, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? extra) :
-        base(id, "page", null, children, hidden, required, readOnly, extra)
+        base(id, "page", null, children, null, hidden, required, readOnly, extra)
     {
         ComponentLookup = componentLookup;
     }
@@ -25,4 +23,12 @@ public PageComponent(string id, List<BaseComponent> children, Dictionary<string,
     /// Helper dictionary to find components without traversing childern.
     /// </summary>
     public Dictionary<string, BaseComponent> ComponentLookup { get; }
+
+    /// <summary>
+    /// AddChild is not needed for PageComponent
+    /// </summary>
+    public override void AddChild(BaseComponent child)
+    {
+        throw new NotImplementedException();
+    }
 }

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

@@ -1,7 +1,3 @@
-using System.Collections.Immutable;
-using System.Text.Json;
-using System.Text.Json.Serialization;
-
 using Altinn.App.Core.Models.Expressions;
 
 namespace Altinn.App.Core.Models.Layout.Components;
@@ -14,8 +10,8 @@ public class RepeatingGroupComponent : GroupComponent
     /// <summary>
     /// Constructor for RepeatingGroupComponent
     /// </summary>
-    public RepeatingGroupComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, int maxCount, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
-        base(id, type, dataModelBindings, children, hidden, required, readOnly, additionalProperties)
+    public RepeatingGroupComponent(string id, string type, IReadOnlyDictionary<string, string>? dataModelBindings, IEnumerable<BaseComponent> children, IEnumerable<string>? childIDs, int maxCount, Expression? hidden, Expression? required, Expression? readOnly, IReadOnlyDictionary<string, string>? additionalProperties) :
+        base(id, type, dataModelBindings, children, childIDs, hidden, required, readOnly, additionalProperties)
     {
         MaxCount = maxCount;
     }
@@ -24,4 +20,4 @@ public RepeatingGroupComponent(string id, string type, IReadOnlyDictionary<strin
     /// Maximum number of repeatitions of this repating group
     /// </summary>
     public int MaxCount { get; }
-}
+}

src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs

@@ -91,7 +91,7 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS
 
         var components = new List<BaseComponent>();
         var componentLookup = new Dictionary<string, BaseComponent>();
-        var childToGroupMapping = new Dictionary<string, (GroupComponent, int)>();
+        var childToGroupMapping = new Dictionary<string, GroupComponent>();
 
         // Hidden is the only property that cascades.
         Expression? hidden = null;
@@ -133,11 +133,11 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS
         }
 
         var page = new PageComponent(pageName, components, componentLookup, hidden, required, readOnly, additionalProperties);
-        ValidateGroupChildren(page);
+        page.ValidateChildren();
         return page;
     }
 
-    private void ReadLayout(ref Utf8JsonReader reader, List<BaseComponent> components, Dictionary<string, BaseComponent> componentLookup, Dictionary<string, (GroupComponent, int)> childToGroupMapping, JsonSerializerOptions options)
+    private void ReadLayout(ref Utf8JsonReader reader, List<BaseComponent> components, Dictionary<string, BaseComponent> componentLookup, Dictionary<string, GroupComponent> childToGroupMapping, JsonSerializerOptions options)
     {
         if (reader.TokenType != JsonTokenType.StartArray)
         {
@@ -152,16 +152,9 @@ private void ReadLayout(ref Utf8JsonReader reader, List<BaseComponent> component
             AddChildrenToLookup(component, componentLookup);
 
             // Check if component should be added to group children or to page
-            if (childToGroupMapping.ContainsKey(component.Id))
+            if (childToGroupMapping.TryGetValue(component.Id, out var groupComponent))
             {
-                var (parent, index) = childToGroupMapping[component.Id];
-                var children = (List<BaseComponent>)parent.Children;
-                while (children.Count <= index)
-                {
-                    children.Add(null!);
-                }
-                children[index] = component;
-                component.Parent = parent;
+                groupComponent.AddChild(component);
             }
             else
             {
@@ -179,23 +172,26 @@ private static void AddChildrenToLookup(BaseComponent component, Dictionary<stri
         componentLookup[component.Id] = component;
     }
 
-    private static readonly Regex MultiPageIndexRegex = new Regex(@"^(\d+:)?([^\s:]+)$");
-    private static void AddChildrenToMapping(GroupComponent component, List<string> children, Dictionary<string, (GroupComponent, int)> childToGroupMapping)
+    private static readonly Regex MultiPageIndexRegex = new Regex(@"^(\d+:)?([^\s:]+)$", RegexOptions.None, TimeSpan.FromSeconds(1));
+    private static string GetIdWithoutMultiPageIndex(string id)
     {
-        for (var index = 0; index < children.Count; index++)
+        var match = MultiPageIndexRegex.Match(id);
+        return match.Groups[2].Value;
+    }
+
+    private static void AddChildrenToMapping(GroupComponent component, List<string> children, Dictionary<string, GroupComponent> childToGroupMapping)
+    {
+        foreach (var childId in children)
         {
-            // Remove MultiPageIndex if present
-            var match = MultiPageIndexRegex.Match(children[index]);
-            var childId = match.Groups[2].Value;
-            if (childToGroupMapping.ContainsKey(childId))
+            if (childToGroupMapping.TryGetValue(childId, out var existingMapping))
             {
-                throw new JsonException($"Component \"{component.Id}\" tried to claim \"{childId}\" as a child, but that child is already claimed by \"{childToGroupMapping[childId].Item1.Id}\"");
+                throw new JsonException($"Component \"{component.Id}\" tried to claim \"{childId}\" as a child, but that child is already claimed by \"{existingMapping.Id}\"");
             }
-            childToGroupMapping[childId] = (component, index);
+            childToGroupMapping[childId] = component;
         }
     }
 
-    private BaseComponent ReadComponent(ref Utf8JsonReader reader, Dictionary<string, (GroupComponent, int)> childToGroupMapping, JsonSerializerOptions options)
+    private BaseComponent ReadComponent(ref Utf8JsonReader reader, Dictionary<string, GroupComponent> childToGroupMapping, JsonSerializerOptions options)
     {
         if (reader.TokenType != JsonTokenType.StartObject)
         {
@@ -248,7 +244,7 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, Dictionary<string
                 // case "textresourcebindings":
                 //     break;
                 case "children":
-                    children = JsonSerializer.Deserialize<List<string>>(ref reader, options);
+                    children = JsonSerializer.Deserialize<List<string>>(ref reader, options)?.Select(GetIdWithoutMultiPageIndex).ToList();
                     break;
                 case "rows":
                     children = GridConfig.ReadGridChildren(ref reader, options);
@@ -306,18 +302,18 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, Dictionary<string
                         throw new JsonException($"A group id:\"{id}\" with maxCount: {maxCount} does not have a \"group\" dataModelBinding");
                     }
 
-                    var repComponent = new RepeatingGroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), maxCount, hidden, required, readOnly, additionalProperties);
+                    var repComponent = new RepeatingGroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, maxCount, hidden, required, readOnly, additionalProperties);
                     AddChildrenToMapping(repComponent, children, childToGroupMapping);
                     return repComponent;
                 }
                 else
                 {
-                    var groupComponent = new GroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), hidden, required, readOnly, additionalProperties);
+                    var groupComponent = new GroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, hidden, required, readOnly, additionalProperties);
                     AddChildrenToMapping(groupComponent, children, childToGroupMapping);
                     return groupComponent;
                 }
             case "grid":
-                var gridComponent = new GridComponent(id, type, dataModelBindings, new List<BaseComponent>(), hidden, required, readOnly, additionalProperties);
+                var gridComponent = new GridComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, hidden, required, readOnly, additionalProperties);
                 AddChildrenToMapping(gridComponent, children!, childToGroupMapping);
                 return gridComponent;
             case "summary":
@@ -370,22 +366,6 @@ private static void ValidateSummary([NotNull] string? componentRef, [NotNull] st
         }
     }
 
-    private static void ValidateGroupChildren(GroupComponent groupComponent)
-    {
-        var children = (List<BaseComponent>)groupComponent.Children;
-        for (var i = 0; i < children.Count; i++)
-        {
-            if (children[i] is null)
-            {
-                throw new JsonException($"Group \"{groupComponent.Id}\" has a null child at index {i}");
-            }
-            if (children[i] is GroupComponent childGroup)
-            {
-                ValidateGroupChildren(childGroup);
-            }
-        }
-    }
-
     /// <summary>
     /// Utility method to recduce so called Coginitve Complexity by writing if in the meth
     /// </summary>