Skip to content

Commit 3b85e70

Browse files
tjololotjololo
authored
Repeating group fix for PDF broke options without mappings (static codelists) in PDF (#30)
* Fixes two bugs: - Repeating group fix broke options without mappings (static codelists) - Tests revealed unintended side effects when making recursive call * Tests updated to test for both errors * Fix some compile warnings * another rookie mistake fixed
1 parent 20efe28 commit 3b85e70

6 files changed

Lines changed: 212 additions & 122 deletions

File tree

‎src/Altinn.App.Core/Internal/Pdf/PdfOptionsMapping.cs‎

Lines changed: 120 additions & 116 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,14 @@
11
using Altinn.App.Core.Features.Options;
22
using Altinn.App.Core.Models;
3+
using Microsoft.IdentityModel.Tokens;
34
using Newtonsoft.Json.Linq;
45

56
namespace Altinn.App.Core.Internal.Pdf;
67

78
/// <summary>
89
/// Default implementation of IPdfOptionsMapping
910
/// </summary>
10-
public class PdfOptionsMapping: IPdfOptionsMapping
11+
public class PdfOptionsMapping : IPdfOptionsMapping
1112
{
1213
private readonly IAppOptionsService _appOptionsService;
1314

@@ -19,160 +20,163 @@ public PdfOptionsMapping(IAppOptionsService appOptionsService)
1920
{
2021
_appOptionsService = appOptionsService;
2122
}
22-
23-
23+
24+
2425
/// <inheritdoc />
2526
public async Task<Dictionary<string, Dictionary<string, string>>> GetOptionsDictionary(string formLayout, string language, object data, string instanceId)
26-
{
27-
IEnumerable<JToken> componentsWithOptionsDefined = GetFormComponentsWithOptionsDefined(formLayout);
28-
29-
Dictionary<string, Dictionary<string, string>> dictionary = new Dictionary<string, Dictionary<string, string>>();
27+
{
28+
IEnumerable<JToken> componentsWithOptionsDefined = GetFormComponentsWithOptionsDefined(formLayout);
3029

31-
foreach (JToken component in componentsWithOptionsDefined)
32-
{
33-
string optionsId = component.SelectToken("optionsId").Value<string>();
34-
bool hasMappings = component.SelectToken("mapping") != null;
35-
var secureToken = component.SelectToken("secure");
36-
bool isSecureOptions = secureToken != null && secureToken.Value<bool>();
37-
Dictionary<string, List<string>> keyValues = new Dictionary<string, List<string>>();
38-
keyValues = hasMappings
39-
? GetComponentKeyValuePairs(component, data)
40-
: new Dictionary<string, List<string>>();
41-
42-
await GetMappingsForComponent(language, instanceId, keyValues, isSecureOptions, optionsId, dictionary);
43-
}
30+
Dictionary<string, Dictionary<string, string>> dictionary = new Dictionary<string, Dictionary<string, string>>();
4431

45-
return dictionary;
32+
foreach (JToken component in componentsWithOptionsDefined)
33+
{
34+
string? optionsId = component.SelectToken("optionsId")?.Value<string>();
35+
if (optionsId == null)
36+
continue;
37+
bool hasMappings = component.SelectToken("mapping") != null;
38+
var secureToken = component.SelectToken("secure");
39+
bool isSecureOptions = secureToken != null && secureToken.Value<bool>();
40+
Dictionary<string, List<string>> keyValues = hasMappings ? GetComponentKeyValuePairs(component, data) : new Dictionary<string, List<string>>();
41+
42+
await GetMappingsForComponent(language, instanceId, keyValues, isSecureOptions, optionsId, dictionary);
4643
}
4744

48-
private async Task GetMappingsForComponent(string language, string instanceId, Dictionary<string, List<string>> keyValues,
49-
bool isSecureOptions, string? optionsId, Dictionary<string, Dictionary<string, string>> dictionary)
45+
return dictionary;
46+
}
47+
48+
private async Task GetMappingsForComponent(string language, string instanceId, Dictionary<string, List<string>> mappings, bool isSecureOptions, string optionsId, Dictionary<string, Dictionary<string, string>> dictionary)
5049
{
50+
if (!dictionary.ContainsKey(optionsId))
51+
{
52+
dictionary.Add(optionsId, new Dictionary<string, string>());
53+
}
54+
5155
var instanceIdentifier = new InstanceIdentifier(instanceId);
52-
foreach (var pair in keyValues)
56+
if (mappings.IsNullOrEmpty())
57+
{
58+
AppOptions appOptions = await GetOptions(isSecureOptions, instanceIdentifier, optionsId, language, new Dictionary<string, string>());
59+
AppendOptionsToDictionary(dictionary[optionsId], appOptions.Options);
60+
return;
61+
}
62+
63+
foreach (var pair in mappings)
5364
{
5465
foreach (var value in pair.Value)
5566
{
56-
AppOptions appOptions;
57-
if (isSecureOptions)
58-
{
59-
appOptions = await _appOptionsService.GetOptionsAsync(instanceIdentifier, optionsId,
60-
language,
61-
new Dictionary<string, string>() { { pair.Key, value } });
62-
}
63-
else
64-
{
65-
appOptions = await _appOptionsService.GetOptionsAsync(optionsId,
66-
language,
67-
new Dictionary<string, string>() { { pair.Key, value } });
68-
}
69-
70-
if (!dictionary.ContainsKey(optionsId))
71-
{
72-
dictionary.Add(optionsId, new Dictionary<string, string>());
73-
}
67+
AppOptions appOptions = await GetOptions(isSecureOptions, instanceIdentifier, optionsId, language, new Dictionary<string, string>() { { pair.Key, value } });
7468

7569
AppendOptionsToDictionary(dictionary[optionsId], appOptions.Options);
7670
}
7771
}
7872
}
7973

80-
private static IEnumerable<JToken> GetFormComponentsWithOptionsDefined(string formLayout)
74+
private async Task<AppOptions> GetOptions(bool isSecure, InstanceIdentifier instanceIdentifier, string optionsId, string language, Dictionary<string, string> mappings)
75+
{
76+
if (isSecure)
8177
{
82-
JObject formLayoutObject = JObject.Parse(formLayout);
83-
84-
// @ = Current object, ?(expression) = Filter, the rest is just dot notation ref. https://goessner.net/articles/JsonPath/
85-
return formLayoutObject.SelectTokens("*.data.layout[?(@.optionsId)]");
78+
return await _appOptionsService.GetOptionsAsync(instanceIdentifier, optionsId, language, mappings);
8679
}
87-
88-
private static Dictionary<string, List<string>> GetComponentKeyValuePairs(JToken component, object data)
80+
else
8981
{
90-
var componentKeyValuePairs = new Dictionary<string, List<string>>();
91-
JObject jsonData = JObject.FromObject(data);
82+
return await _appOptionsService.GetOptionsAsync(optionsId, language, mappings);
83+
}
84+
}
9285

93-
Dictionary<string, string> mappings = GetMappingsForComponent(component);
94-
foreach (var map in mappings)
95-
{
96-
var selectedDatas = GetMappingValues(jsonData, map);
86+
private static IEnumerable<JToken> GetFormComponentsWithOptionsDefined(string formLayout)
87+
{
88+
JObject formLayoutObject = JObject.Parse(formLayout);
9789

98-
componentKeyValuePairs.Add(map.Value, selectedDatas);
99-
}
90+
// @ = Current object, ?(expression) = Filter, the rest is just dot notation ref. https://goessner.net/articles/JsonPath/
91+
return formLayoutObject.SelectTokens("*.data.layout[?(@.optionsId)]");
92+
}
10093

101-
return componentKeyValuePairs;
102-
}
94+
private static Dictionary<string, List<string>> GetComponentKeyValuePairs(JToken component, object data)
95+
{
96+
var componentKeyValuePairs = new Dictionary<string, List<string>>();
97+
JObject jsonData = JObject.FromObject(data);
10398

104-
private static Dictionary<string, string> GetMappingsForComponent(JToken component)
99+
Dictionary<string, string> mappings = GetMappingsForComponent(component);
100+
foreach (var map in mappings)
105101
{
106-
var maps = new Dictionary<string, string>();
107-
foreach (var jToken in component.SelectToken("mapping")?.Children()!)
108-
{
109-
var map = (JProperty)jToken;
110-
maps.Add(map.Name, map.Value.ToString());
111-
}
102+
var selectedDatas = GetMappingValues(jsonData, map);
112103

113-
return maps;
104+
componentKeyValuePairs.Add(map.Value, selectedDatas);
114105
}
115106

116-
private static void AppendOptionsToDictionary(Dictionary<string, string> dictionary, List<AppOption> options)
107+
return componentKeyValuePairs;
108+
}
109+
110+
private static Dictionary<string, string> GetMappingsForComponent(JToken component)
111+
{
112+
var maps = new Dictionary<string, string>();
113+
foreach (var jToken in component.SelectToken("mapping")?.Children()!)
117114
{
118-
foreach (AppOption item in options)
119-
{
120-
if (!dictionary.ContainsKey(item.Label))
121-
{
122-
dictionary.Add(item.Label, item.Value);
123-
}
124-
}
115+
var map = (JProperty)jToken;
116+
maps.Add(map.Name, map.Value.ToString());
125117
}
126118

119+
return maps;
120+
}
127121

128-
129-
private static List<string> GetMappingValues(JObject jsonData, KeyValuePair<string, string> map, int depth = 0)
122+
private static void AppendOptionsToDictionary(Dictionary<string, string> dictionary, List<AppOption> options)
123+
{
124+
foreach (AppOption item in options)
130125
{
131-
int count = 1;
132-
if (MappingHasRepeatingGroup(map.Key))
133-
{
134-
var mappingUntilFirstGroup = GetMappingUntilFirstGroup(map.Key);
135-
JToken repeatingGroup = jsonData.SelectToken(mappingUntilFirstGroup);
136-
count = repeatingGroup.Children().Count();
137-
}
138-
139-
List<string> selectedDatas = new List<string>();
140-
for (var i = 0; i < count; i++)
126+
if (!dictionary.ContainsKey(item.Label))
141127
{
142-
string replaceText = "{" + depth + "}";
143-
144-
string select = map.Key.Replace(replaceText, i.ToString());
145-
if (MappingHasRepeatingGroup(select))
146-
{
147-
selectedDatas.AddRange(GetMappingValues(jsonData,
148-
new KeyValuePair<string, string>(select, map.Value), ++depth));
149-
}
150-
else
151-
{
152-
selectedDatas.Add(jsonData.SelectToken(select).ToString());
153-
}
128+
dictionary.Add(item.Label, item.Value);
154129
}
155-
156-
return selectedDatas;
157130
}
131+
}
158132

159-
/// <summary>
160-
/// Return true if mapping contains a array replacement start "[{"
161-
/// </summary>
162-
/// <param name="mapping">Field mapping</param>
163-
/// <returns></returns>
164-
private static bool MappingHasRepeatingGroup(string mapping)
133+
134+
private static List<string> GetMappingValues(JObject jsonData, KeyValuePair<string, string> map, int depth = 0)
135+
{
136+
int count = 1;
137+
if (MappingHasRepeatingGroup(map.Key))
165138
{
166-
return mapping.Contains("[{");
139+
var mappingUntilFirstGroup = GetMappingUntilFirstGroup(map.Key);
140+
JToken repeatingGroup = jsonData.SelectToken(mappingUntilFirstGroup);
141+
count = repeatingGroup.Children().Count();
167142
}
168143

169-
/// <summary>
170-
/// Returns mapping of first element in mapping that is a list with replacement string.
171-
/// </summary>
172-
/// <param name="mapping"></param>
173-
/// <returns></returns>
174-
private static string GetMappingUntilFirstGroup(string mapping)
144+
List<string> selectedDatas = new List<string>();
145+
for (var i = 0; i < count; i++)
175146
{
176-
return mapping.Substring(0, mapping.IndexOf("[{"));
147+
string replaceText = "{" + depth + "}";
148+
149+
string select = map.Key.Replace(replaceText, i.ToString());
150+
if (MappingHasRepeatingGroup(select))
151+
{
152+
selectedDatas.AddRange(GetMappingValues(jsonData, new KeyValuePair<string, string>(select, map.Value), depth + 1));
153+
}
154+
else
155+
{
156+
selectedDatas.Add(jsonData.SelectToken(select).ToString());
157+
}
177158
}
178-
}
159+
160+
return selectedDatas;
161+
}
162+
163+
/// <summary>
164+
/// Return true if mapping contains a array replacement start "[{"
165+
/// </summary>
166+
/// <param name="mapping">Field mapping</param>
167+
/// <returns></returns>
168+
private static bool MappingHasRepeatingGroup(string mapping)
169+
{
170+
return mapping.Contains("[{");
171+
}
172+
173+
/// <summary>
174+
/// Returns mapping of first element in mapping that is a list with replacement string.
175+
/// </summary>
176+
/// <param name="mapping"></param>
177+
/// <returns></returns>
178+
private static string GetMappingUntilFirstGroup(string mapping)
179+
{
180+
return mapping.Substring(0, mapping.IndexOf("[{"));
181+
}
182+
}

‎test/Altinn.App.Core.Tests/Internal/Pdf/PdfOptionsMappingTests.cs‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ namespace Altinn.App.PlatformServices.Tests.Internal.Pdf;
99
public class PdfOptionsMappingTests
1010
{
1111
[Fact]
12-
public async void GetOptionsDictionary_returns_dictionary_for_options()
12+
public async void GetOptionsDictionary_returns_dictionary_for_options_with_mapping()
1313
{
1414
IPdfOptionsMapping pdfOptionsMapping = new PdfOptionsMapping(new AppOptionsDouble());
1515
string formlayout = File.ReadAllText(Path.Combine("Internal", "Pdf", "TestData", "formlayout.json"));
16-
object o = ReadTestData();
16+
object o = ReadTestData("data.xml");
1717

1818
var res = await pdfOptionsMapping.GetOptionsDictionary(formlayout, "nb", o, "512345/cab39d95-7439-4ecb-b908-2fb3726d9295");
1919
res.Count.Should().Be(2);
@@ -25,11 +25,26 @@ public async void GetOptionsDictionary_returns_dictionary_for_options()
2525
res["repdropdown"].Should().ContainKey("val");
2626
res["repdropdown"]["val"].Should().Be("1-repdropdown-nb");
2727
}
28+
29+
[Fact]
30+
public async void GetOptionsDictionary_returns_dictionary_for_options_without_mapping()
31+
{
32+
IPdfOptionsMapping pdfOptionsMapping = new PdfOptionsMapping(new AppOptionsDouble());
33+
string formlayout = File.ReadAllText(Path.Combine("Internal", "Pdf", "TestData", "formlayout-nomapping.json"));
34+
object o = ReadTestData("data-nomapping.xml");
35+
36+
var res = await pdfOptionsMapping.GetOptionsDictionary(formlayout, "nb", o, "512345/cab39d95-7439-4ecb-b908-2fb3726d9295");
37+
res.Count.Should().Be(1);
38+
res.Should().ContainKeys("fylker");
39+
res["fylker"].Count.Should().Be(1);
40+
res["fylker"].Should().ContainKey("no-mapping");
41+
res["fylker"]["no-mapping"].Should().Be("fylker-nb");
42+
}
2843

29-
private static object ReadTestData()
44+
private static object ReadTestData(string datafile)
3045
{
3146
System.Xml.Serialization.XmlSerializer reader = new System.Xml.Serialization.XmlSerializer(typeof(Skjema));
32-
using (StreamReader file = new System.IO.StreamReader(Path.Combine("Internal", "Pdf", "TestData", "data.xml")))
47+
using (StreamReader file = new System.IO.StreamReader(Path.Combine("Internal", "Pdf", "TestData", datafile)))
3348
{
3449
Skjema s = (Skjema)reader.Deserialize(file);
3550
file.Close();

‎test/Altinn.App.Core.Tests/Internal/Pdf/TestData/data-nomapping.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
<?xml version="1.0" encoding="utf-8"?>
2+
<Skjema xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xsd="http://www.w3.org/2001/XMLSchema">
3+
<melding>
4+
<name>Default</name>
5+
<tags>designer</tags>
6+
<nested_list>
7+
<key>18</key>
8+
</nested_list>
9+
<nested_list>
10+
<key>19</key>
11+
</nested_list>
12+
<toggle>false</toggle>
13+
</melding>
14+
</Skjema>

‎test/Altinn.App.Core.Tests/Internal/Pdf/TestData/data.xml‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,20 @@
1616
</simple_keyvalues>
1717
</simple_list>
1818
<nested_list>
19+
<key>1</key>
20+
<values>
21+
<key>cowboy</key>
22+
<doubleValue>0</doubleValue>
23+
<intValue>1</intValue>
24+
</values>
25+
<values>
26+
<key>operator</key>
27+
<doubleValue>0</doubleValue>
28+
<intValue>4</intValue>
29+
</values>
30+
</nested_list>
31+
<nested_list>
32+
<key>2</key>
1933
<values>
2034
<key>cowboy</key>
2135
<doubleValue>0</doubleValue>

0 commit comments

Comments
 (0)