Fix PageComponentConverter and add unit tests

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

@@ -0,0 +1,84 @@
+using System.Text.Json;
+using System.Text.Json.Serialization;
+using Altinn.App.Core.Models.Expressions;
+
+namespace Altinn.App.Core.Models.Layout.Components;
+
+/// <summary>
+/// Component specialisation for repeating groups with maxCount > 1
+/// </summary>
+public class GridComponent : GroupComponent
+{
+    /// <summary>
+    /// Constructor for RepeatingGroupComponent
+    /// </summary>
+    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)
+    { }
+}
+
+
+/// <summary>
+/// Class for parsing a Grid component's rows and cells and extracting the child component IDs
+/// </summary>
+public class GridConfig
+{
+
+    /// <summary>
+    /// Reads the Grid component's rows and returns the child component IDs
+    /// </summary>
+    /// <param name="reader"></param>
+    /// <param name="options"></param>
+    /// <returns></returns>
+    public static List<string> ReadGridChildren(ref Utf8JsonReader reader, JsonSerializerOptions options)
+    {
+        var rows = JsonSerializer.Deserialize<GridRow[]>(ref reader, options);
+        var gridConfig = new GridConfig { Rows = rows };
+        return gridConfig.Children();
+    }
+
+    /// <summary>
+    /// Rows in the grid
+    /// </summary>
+    [JsonPropertyName("rows")]
+    public GridRow[]? Rows { get; set; }
+
+    /// <summary>
+    /// Defines a row in a grid
+    /// </summary>
+    public class GridRow
+    {
+        /// <summary>
+        /// Cells in the row
+        /// </summary>
+        [JsonPropertyName("cells")]
+        public GridCell[]? Cells { get; set; }
+
+        /// <summary>
+        /// Defines a cell in a grid
+        /// </summary>
+        public class GridCell
+        {
+            /// <summary>
+            /// The component ID of the cell
+            /// </summary>
+            [JsonPropertyName("component")]
+            public string? ComponentId { get; set; }
+        }
+    }
+
+    /// <summary>
+    /// Returns the child component IDs
+    /// </summary>
+    /// <returns></returns>
+    public List<String> Children()
+    {
+        return this.Rows?
+            .Where(r => r.Cells is not null)
+            .SelectMany(r => r.Cells!)
+            .Where(c => c.ComponentId is not null)
+            .Select(c => c.ComponentId!)
+            .ToList()
+            ?? new List<String>();
+    }
+}

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

@@ -12,18 +12,42 @@ 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; private 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($"Attempted to add child with id {child.Id} to group {this.Id}, but this child is not included in its list of child IDs");
+        }
+        if (this.Children.FirstOrDefault(c => c.Id == child.Id) != null)
+        {
+            throw new ArgumentException($"Attempted to add child with id {child.Id} to group {this.Id}, but a child with this id has already been added");
+        }
+        child.Parent = this;
+        this.Children = this.Children.Append(child);
+    }
+}

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, and the base implementation would not work as intended.
+    /// </summary>
+    public override void AddChild(BaseComponent child)
+    {
+
+    }
 }

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/PageComponentComverter.cs → src/Altinn.App.Core/Models/Layout/PageComponentConverter.cs

@@ -6,6 +6,8 @@
 using Altinn.App.Core.Models.Expressions;
 using System.Runtime.CompilerServices;
 using System.Diagnostics.CodeAnalysis;
+using System.Text.RegularExpressions;
+
 
 namespace Altinn.App.Core.Models.Layout;
 /// <summary>
@@ -87,8 +89,9 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS
             throw new JsonException();
         }
 
-        var components = new List<BaseComponent>();
-        var componentLookup = new Dictionary<string, BaseComponent>();
+        List<BaseComponent>? componentListFlat = null;
+        Dictionary<string, BaseComponent>? componentLookup = null;
+        Dictionary<string, GroupComponent>? childToGroupMapping = null;
 
         // Hidden is the only property that cascades.
         Expression? hidden = null;
@@ -111,7 +114,7 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS
             switch (propertyName.ToLowerInvariant())
             {
                 case "layout":
-                    ReadLayout(ref reader, components, componentLookup, options);
+                    (componentListFlat, componentLookup, childToGroupMapping) = ReadLayout(ref reader, options);
                     break;
                 case "hidden":
                     hidden = ExpressionConverter.ReadNotNull(ref reader, options);
@@ -129,38 +132,87 @@ private PageComponent ReadData(ref Utf8JsonReader reader, string pageName, JsonS
             }
         }
 
-        return new PageComponent(pageName, components, componentLookup, hidden, required, readOnly, additionalProperties);
+        if (componentListFlat is null || componentLookup is null || childToGroupMapping is null)
+        {
+            throw new JsonException("Missing property \"layout\" on layout page");
+        }
+
+        var layout = processLayout(componentListFlat, componentLookup, childToGroupMapping);
+
+        return new PageComponent(pageName, layout, componentLookup, hidden, required, readOnly, additionalProperties);
     }
 
-    private void ReadLayout(ref Utf8JsonReader reader, List<BaseComponent> components, Dictionary<string, BaseComponent> componentLookup, JsonSerializerOptions options)
+    private (List<BaseComponent>, Dictionary<string, BaseComponent>, Dictionary<string, GroupComponent>) ReadLayout(ref Utf8JsonReader reader, JsonSerializerOptions options)
     {
         if (reader.TokenType != JsonTokenType.StartArray)
         {
             throw new JsonException();
         }
 
+        var componentListFlat = new List<BaseComponent>();
+        var componentLookup = new Dictionary<string, BaseComponent>();
+        var childToGroupMapping = new Dictionary<string, GroupComponent>();
+
         while (reader.Read() && reader.TokenType != JsonTokenType.EndArray)
         {
             var component = ReadComponent(ref reader, options)!;
-            // Add new component to both collections
-            components.Add(component);
-            AddChildrenToLookup(component, componentLookup);
+
+            // Add component to the collections
+            componentListFlat.Add(component);
+            AddToComponentLookup(component, componentLookup);
+            if (component is GroupComponent groupComponent)
+            {
+                AddChildrenToMapping(groupComponent, groupComponent.ChildIDs, childToGroupMapping);
+            }
+        }
+
+        return (componentListFlat, componentLookup, childToGroupMapping);
+    }
+
+    private static List<BaseComponent> processLayout(List<BaseComponent> componentListFlat, Dictionary<string, BaseComponent> componentLookup, Dictionary<string, GroupComponent> childToGroupMapping)
+    {
+        var layout = new List<BaseComponent>();
+        foreach (var component in componentListFlat)
+        {
+            if (component is GroupComponent groupComponent)
+            {
+                var children = groupComponent.ChildIDs.Select(id => componentLookup[id]).ToList();
+                children.ForEach(c => groupComponent.AddChild(c));
+            }
+
+            if (!childToGroupMapping.ContainsKey(component.Id))
+            {
+                layout.Add(component);
+            }
         }
+        return layout;
     }
 
-    private static void AddChildrenToLookup(BaseComponent component, Dictionary<string, BaseComponent> componentLookup)
+    private static void AddToComponentLookup(BaseComponent component, Dictionary<string, BaseComponent> componentLookup)
     {
         if (componentLookup.ContainsKey(component.Id))
         {
             throw new JsonException($"Duplicate key \"{component.Id}\" detected on page \"{component.PageId}\"");
         }
         componentLookup[component.Id] = component;
-        if (component is GroupComponent groupComponent)
+    }
+
+    private static readonly Regex MultiPageIndexRegex = new Regex(@"^(\d+:)?([^\s:]+)$", RegexOptions.None, TimeSpan.FromSeconds(1));
+    private static string GetIdWithoutMultiPageIndex(string id)
+    {
+        var match = MultiPageIndexRegex.Match(id);
+        return match.Groups[2].Value;
+    }
+
+    private static void AddChildrenToMapping(GroupComponent component, IEnumerable<string> children, Dictionary<string, GroupComponent> childToGroupMapping)
+    {
+        foreach (var childId in children)
         {
-            foreach (var child in groupComponent.Children)
+            if (childToGroupMapping.TryGetValue(childId, out var existingMapping))
             {
-                AddChildrenToLookup(child, componentLookup);
+                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;
         }
     }
 
@@ -217,7 +269,10 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, JsonSerializerOpt
                 // 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);
                     break;
                 case "maxcount":
                     maxCount = reader.GetInt32();
@@ -265,28 +320,31 @@ private BaseComponent ReadComponent(ref Utf8JsonReader reader, JsonSerializerOpt
             case "group":
                 ThrowJsonExceptionIfNull(children, "Component with \"type\": \"Group\" requires a \"children\" property");
 
-                var childComponents = ReadChildren(ref reader, id, children, options);
                 if (maxCount > 1)
                 {
                     if (!(dataModelBindings?.ContainsKey("group") ?? false))
                     {
                         throw new JsonException($"A group id:\"{id}\" with maxCount: {maxCount} does not have a \"group\" dataModelBinding");
                     }
 
-                    return new RepeatingGroupComponent(id, type, dataModelBindings, childComponents, maxCount, hidden, required, readOnly, additionalProperties);
+                    var repComponent = new RepeatingGroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, maxCount, hidden, required, readOnly, additionalProperties);
+                    return repComponent;
                 }
                 else
                 {
-                    return new GroupComponent(id, type, dataModelBindings, childComponents, hidden, required, readOnly, additionalProperties);
+                    var groupComponent = new GroupComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, hidden, required, readOnly, additionalProperties);
+                    return groupComponent;
                 }
+            case "grid":
+                var gridComponent = new GridComponent(id, type, dataModelBindings, new List<BaseComponent>(), children, hidden, required, readOnly, additionalProperties);
+                return gridComponent;
             case "summary":
                 ValidateSummary(componentRef, pageRef);
                 return new SummaryComponent(id, type, hidden, componentRef, pageRef, additionalProperties);
             case "checkboxes":
             case "radiobuttons":
             case "dropdown":
                 ValidateOptions(optionId, literalOptions, optionsSource, secure);
-
                 return new OptionsComponent(id, type, dataModelBindings, hidden, required, readOnly, optionId, literalOptions, optionsSource, secure, additionalProperties);
         }
 
@@ -341,30 +399,9 @@ private static void ThrowJsonExceptionIfNull([NotNull] object? obj, string? mess
         }
     }
 
-
-    private List<BaseComponent> ReadChildren(ref Utf8JsonReader reader, string parentId, List<string> childIds, JsonSerializerOptions options)
-    {
-        var ret = new List<BaseComponent>();
-        foreach (var childId in childIds)
-        {
-            reader.Read();
-            if (reader.TokenType != JsonTokenType.StartObject)
-            {
-                throw new JsonException($"Invalid Group component \"{parentId}\". No components found after group component");
-            }
-            var component = ReadComponent(ref reader, options)!;
-            if (component.Id != childId)
-            {
-                throw new JsonException($"Invalid Group component \"{parentId}\". The next component has id \"{component.Id}\" instead of \"{childId}\"");
-            }
-            ret.Add(component);
-        }
-        return ret;
-    }
-
     /// <inheritdoc />
     public override void Write(Utf8JsonWriter writer, PageComponent value, JsonSerializerOptions options)
     {
         throw new NotImplementedException();
     }
-}
+}