Skip to content

Commit 7b71658

Browse files
olemartinorgOle Martin HandelandRonny Birkeli
authored
Proper diffing in JsonHelper for rows removed from array (#8)
Co-authored-by: Ole Martin Handeland <git@olemartin.org> Co-authored-by: Ronny Birkeli <ronny.birkeli@digdir.no>
1 parent 19d316c commit 7b71658

8 files changed

Lines changed: 370 additions & 32 deletions

File tree

AppLibDotnet.sln

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.App.PlatformServices
1111
EndProject
1212
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.App.PlatformServices.Tests", "test\Altinn.App.PlatformServices.Tests\Altinn.App.PlatformServices.Tests.csproj", "{17D7DCE9-7797-4BC1-B448-D0529FD6FB3D}"
1313
EndProject
14-
Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Altinn.App.Api.Tests", "test\Altinn.App.Api.Tests\Altinn.App.Api.Tests.csproj", "{2FD56505-1DB2-4AE1-8911-E076E535EAC6}"
14+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.App.Api.Tests", "test\Altinn.App.Api.Tests\Altinn.App.Api.Tests.csproj", "{2FD56505-1DB2-4AE1-8911-E076E535EAC6}"
1515
EndProject
1616
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "test", "test", "{6C8EB054-1747-4BAC-A637-754F304BCAFA}"
1717
EndProject
1818
Project("{2150E333-8FDC-42A3-9474-1A3956D46DE8}") = "src", "src", "{7AD5FADE-607F-4D5F-8511-6647D0C1AA1C}"
1919
EndProject
20+
Project("{9A19103F-16F7-4668-BE54-9A1E7A4F7556}") = "Altinn.App.Common.Tests", "test\Altinn.App.Common.Tests\Altinn.App.Common.Tests.csproj", "{CF47083E-B3E0-4247-87AA-F7EF6B3292D1}"
21+
EndProject
2022
Global
2123
GlobalSection(SolutionConfigurationPlatforms) = preSolution
2224
Debug|Any CPU = Debug|Any CPU
@@ -43,18 +45,23 @@ Global
4345
{2FD56505-1DB2-4AE1-8911-E076E535EAC6}.Debug|Any CPU.Build.0 = Debug|Any CPU
4446
{2FD56505-1DB2-4AE1-8911-E076E535EAC6}.Release|Any CPU.ActiveCfg = Release|Any CPU
4547
{2FD56505-1DB2-4AE1-8911-E076E535EAC6}.Release|Any CPU.Build.0 = Release|Any CPU
48+
{CF47083E-B3E0-4247-87AA-F7EF6B3292D1}.Debug|Any CPU.ActiveCfg = Debug|Any CPU
49+
{CF47083E-B3E0-4247-87AA-F7EF6B3292D1}.Debug|Any CPU.Build.0 = Debug|Any CPU
50+
{CF47083E-B3E0-4247-87AA-F7EF6B3292D1}.Release|Any CPU.ActiveCfg = Release|Any CPU
51+
{CF47083E-B3E0-4247-87AA-F7EF6B3292D1}.Release|Any CPU.Build.0 = Release|Any CPU
4652
EndGlobalSection
4753
GlobalSection(SolutionProperties) = preSolution
4854
HideSolutionNode = FALSE
4955
EndGlobalSection
50-
GlobalSection(ExtensibilityGlobals) = postSolution
51-
SolutionGuid = {4584C6E1-D5B4-40B1-A8C4-CF4620EB0896}
52-
EndGlobalSection
5356
GlobalSection(NestedProjects) = preSolution
5457
{E8F29FE8-6B62-41F1-A08C-2A318DD08BB4} = {7AD5FADE-607F-4D5F-8511-6647D0C1AA1C}
55-
{2FD56505-1DB2-4AE1-8911-E076E535EAC6} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
5658
{7D2FF2B1-7F21-4934-9D77-DBCDC15BD8CC} = {7AD5FADE-607F-4D5F-8511-6647D0C1AA1C}
5759
{98E6200A-ED99-418E-B30C-81BA564B509A} = {7AD5FADE-607F-4D5F-8511-6647D0C1AA1C}
5860
{17D7DCE9-7797-4BC1-B448-D0529FD6FB3D} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
61+
{2FD56505-1DB2-4AE1-8911-E076E535EAC6} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
62+
{CF47083E-B3E0-4247-87AA-F7EF6B3292D1} = {6C8EB054-1747-4BAC-A637-754F304BCAFA}
63+
EndGlobalSection
64+
GlobalSection(ExtensibilityGlobals) = postSolution
65+
SolutionGuid = {4584C6E1-D5B4-40B1-A8C4-CF4620EB0896}
5966
EndGlobalSection
6067
EndGlobal

src/Altinn.App.Common/Helpers/JsonHelper.cs

Lines changed: 96 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
#nullable enable
12
using System.Collections.Generic;
23
using System.Linq;
34

@@ -16,71 +17,139 @@ public static class JsonHelper
1617
/// <param name="oldJson">The old JSON object</param>
1718
/// <param name="currentJson">The new JSON object</param>
1819
/// <returns>Key-value pairs of the changed fields</returns>
19-
public static Dictionary<string, object> FindChangedFields(string oldJson, string currentJson)
20+
public static Dictionary<string, object?> FindChangedFields(string oldJson, string currentJson)
2021
{
2122
JToken old = JToken.Parse(oldJson);
2223
JToken current = JToken.Parse(currentJson);
23-
Dictionary<string, object> dict = new Dictionary<string, object>();
24+
Dictionary<string, object?> dict = new Dictionary<string, object?>();
2425
FindDiff(dict, old, current, string.Empty);
2526
return dict;
2627
}
2728

28-
private static void FindDiff(Dictionary<string, object> dict, JToken Old, JToken Current, string prefix)
29+
private static void FindDiff(Dictionary<string, object?> dict, JToken? old, JToken? current, string prefix)
2930
{
30-
if (JToken.DeepEquals(Old, Current))
31+
if (JToken.DeepEquals(old, current))
3132
{
3233
return;
3334
}
3435

35-
switch (Current.Type)
36+
int index = 0;
37+
JArray? oldArray = old as JArray;
38+
JObject? currentObj = current as JObject;
39+
JObject? oldObj = old as JObject;
40+
41+
switch (current?.Type)
3642
{
3743
case JTokenType.Object:
38-
JObject current = Current as JObject;
39-
JObject old = Old as JObject;
4044

41-
if (old == null)
45+
if (oldArray != null)
46+
{
47+
for (index = 0; index < oldArray.Count; index++)
48+
{
49+
dict.Add($"{prefix}[{index}]", null);
50+
}
51+
}
52+
else if (old?.Type != JTokenType.Object)
53+
{
54+
// Scalar values would use the plain prefix, but object create deeper prefixes. If a scalar
55+
// value is replaced by an object, we need to unset the scalar value as well.
56+
dict.Add(prefix, null);
57+
}
58+
59+
if (oldObj == null && currentObj != null)
4260
{
43-
foreach (string key in current.Properties().Select(c => c.Name))
61+
foreach (string key in currentObj.Properties().Select(c => c.Name))
4462
{
45-
FindDiff(dict, new JObject(), current[key], Join(prefix, key));
63+
FindDiff(dict, JValue.CreateNull(), currentObj[key], Join(prefix, key));
4664
}
4765

4866
break;
4967
}
5068

51-
IEnumerable<string> addedKeys = current.Properties().Select(c => c.Name).Except(old.Properties().Select(c => c.Name));
52-
IEnumerable<string> removedKeys = old.Properties().Select(c => c.Name).Except(current.Properties().Select(c => c.Name));
53-
IEnumerable<string> unchangedKeys = current.Properties().Where(c => JToken.DeepEquals(c.Value, old[c.Name])).Select(c => c.Name);
54-
foreach (string key in addedKeys)
69+
if (oldObj != null && currentObj != null)
5570
{
56-
FindDiff(dict, new JObject(), current[key], Join(prefix, key));
71+
IEnumerable<string> addedKeys = currentObj.Properties().Select(c => c.Name).Except(oldObj.Properties().Select(c => c.Name));
72+
IEnumerable<string> removedKeys = oldObj.Properties().Select(c => c.Name).Except(currentObj.Properties().Select(c => c.Name));
73+
IEnumerable<string> unchangedKeys = currentObj.Properties().Where(c => JToken.DeepEquals(c.Value, oldObj[c.Name])).Select(c => c.Name);
74+
foreach (string key in addedKeys)
75+
{
76+
FindDiff(dict, JValue.CreateNull(), currentObj[key], Join(prefix, key));
77+
}
78+
79+
foreach (string key in removedKeys)
80+
{
81+
FindDiff(dict, oldObj[key], JValue.CreateNull(), Join(prefix, key));
82+
}
83+
84+
var potentiallyModifiedKeys = currentObj.Properties().Select(c => c.Name).Except(addedKeys).Except(unchangedKeys);
85+
foreach (var key in potentiallyModifiedKeys)
86+
{
87+
FindDiff(dict, oldObj[key], currentObj[key], Join(prefix, key));
88+
}
5789
}
5890

59-
foreach (string key in removedKeys)
91+
break;
92+
93+
case JTokenType.Array:
94+
if (oldArray != null)
6095
{
61-
dict.Add(Join(prefix, key), null);
62-
}
96+
foreach (var value in current.Children())
97+
{
98+
FindDiff(
99+
dict,
100+
oldArray?.Count - 1 >= index ? oldArray?[index] : new JObject(),
101+
value,
102+
$"{prefix}[{index}]");
103+
104+
index++;
105+
}
63106

64-
var potentiallyModifiedKeys = current.Properties().Select(c => c.Name).Except(addedKeys).Except(unchangedKeys);
65-
foreach (var key in potentiallyModifiedKeys)
107+
while (index < oldArray?.Count)
108+
{
109+
FindDiff(dict, oldArray[index], JValue.CreateNull(), $"{prefix}[{index}]");
110+
index++;
111+
}
112+
}
113+
else
66114
{
67-
FindDiff(dict, old[key], current[key], Join(prefix, key));
115+
if (old?.Type == JTokenType.Object)
116+
{
117+
FindDiff(dict, old, JValue.CreateNull(), prefix);
118+
}
119+
120+
foreach (JToken value in current.Children())
121+
{
122+
FindDiff(dict, JValue.CreateNull(), value, $"{prefix}[{index}]");
123+
index++;
124+
}
68125
}
69126

70127
break;
71128

72-
case JTokenType.Array:
73-
int index = 0;
74-
foreach (JToken value in Current.Children())
129+
case JTokenType.Null:
130+
if (oldObj != null)
131+
{
132+
foreach (string key in oldObj.Properties().Select(c => c.Name))
133+
{
134+
FindDiff(dict, oldObj[key], JValue.CreateNull(), Join(prefix, key));
135+
}
136+
}
137+
else if (old?.Type == JTokenType.Array)
138+
{
139+
for (index = 0; index < oldArray?.Count; index++)
140+
{
141+
dict.Add($"{prefix}[{index}]", null);
142+
}
143+
}
144+
else
75145
{
76-
FindDiff(dict, new JObject(), value, $"{prefix}[{index}]");
77-
index++;
146+
dict.Add(prefix, ((JValue)current).Value);
78147
}
79148

80149
break;
81150

82151
default:
83-
dict.Add(prefix, ((JValue)Current).Value);
152+
dict.Add(prefix, current == null ? null : ((JValue)current).Value);
84153
break;
85154
}
86155
}
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
<Project Sdk="Microsoft.NET.Sdk">
2+
3+
<PropertyGroup>
4+
<TargetFramework>net6.0</TargetFramework>
5+
<Nullable>enable</Nullable>
6+
7+
<IsPackable>false</IsPackable>
8+
</PropertyGroup>
9+
10+
<ItemGroup>
11+
<None Remove="Helpers\JsonHelperTests\TestData\before.json" />
12+
<None Remove="Helpers\JsonHelperTests\TestData\after.json" />
13+
</ItemGroup>
14+
15+
<ItemGroup>
16+
<EmbeddedResource Include="Helpers\JsonHelperTests\TestData\before.json" />
17+
<EmbeddedResource Include="Helpers\JsonHelperTests\TestData\after.json" />
18+
</ItemGroup>
19+
20+
<ItemGroup>
21+
<PackageReference Include="FluentAssertions" Version="6.7.0" />
22+
<PackageReference Include="Microsoft.NET.Test.Sdk" Version="16.11.0" />
23+
<PackageReference Include="xunit" Version="2.4.1" />
24+
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.3">
25+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
26+
<PrivateAssets>all</PrivateAssets>
27+
</PackageReference>
28+
<PackageReference Include="coverlet.collector" Version="3.1.0">
29+
<IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>
30+
<PrivateAssets>all</PrivateAssets>
31+
</PackageReference>
32+
</ItemGroup>
33+
34+
<ItemGroup>
35+
<ProjectReference Include="..\..\src\Altinn.App.Common\Altinn.App.Common.csproj" />
36+
</ItemGroup>
37+
38+
</Project>
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
using System;
2+
using System.IO;
3+
using System.Reflection;
4+
using System.Threading.Tasks;
5+
6+
namespace Altinn.App.Common.Tests
7+
{
8+
public static class EmbeddedResource
9+
{
10+
public async static Task<string> LoadDataAsString(string resourceName)
11+
{
12+
var resourceStream = LoadDataAsStream(resourceName);
13+
14+
using var reader = new StreamReader(resourceStream);
15+
string text = await reader.ReadToEndAsync();
16+
17+
return text;
18+
}
19+
20+
public static Stream LoadDataAsStream(string resourceName)
21+
{
22+
var assembly = Assembly.GetExecutingAssembly();
23+
Stream? resourceStream = assembly.GetManifestResourceStream(resourceName);
24+
25+
if (resourceStream == null)
26+
{
27+
throw new InvalidOperationException($"Unable to find resource {resourceName} embedded in assembly {assembly.FullName}.");
28+
}
29+
30+
resourceStream.Seek(0, SeekOrigin.Begin);
31+
32+
return resourceStream;
33+
}
34+
}
35+
}
Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,82 @@
1+
using Altinn.App.Common.Helpers;
2+
using FluentAssertions;
3+
using System.Collections.Generic;
4+
using Xunit;
5+
6+
namespace Altinn.App.Common.Tests.Helpers.JsonHelperTests
7+
{
8+
public class JsonHelperTests
9+
{
10+
[Fact]
11+
public async void FindChangedFields_RepeatingGroups_ShouldFindRemovedEntry()
12+
{
13+
var before = await EmbeddedResource.LoadDataAsString(
14+
"Altinn.App.Common.Tests.Helpers.JsonHelperTests.TestData.before.json"
15+
);
16+
var after = await EmbeddedResource.LoadDataAsString(
17+
"Altinn.App.Common.Tests.Helpers.JsonHelperTests.TestData.after.json"
18+
);
19+
20+
Dictionary<string, object?> changedFields = JsonHelper.FindChangedFields(before, after);
21+
22+
Dictionary<string, object?> expected = new Dictionary<string, object?>
23+
{
24+
{"willBeRemoved", null},
25+
{"willChangeValue", false},
26+
{"object.willBeRemoved", null},
27+
{"object.listInObject[2]", null},
28+
29+
// One item has been removed, so the later values have been shifted up
30+
{"moreAdvanced.oneRemovedInList[2]", "kept3"},
31+
{"moreAdvanced.oneRemovedInList[3]", "kept4"},
32+
{"moreAdvanced.oneRemovedInList[4]", null},
33+
34+
{"moreAdvanced.objectWithRemovedProperty.removed1.hello", null},
35+
36+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.removed1.hello", null},
37+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.removed1.alsoRemovedProp", null},
38+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.removed2.hello", null},
39+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.removed2.alsoRemovedProp", null},
40+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.kept1.removedProp", null},
41+
{"moreAdvanced.objectWithRemovedPropertyAndInnerChanges.kept2.second", true},
42+
43+
{"moreAdvanced.mixedList[0].first", false},
44+
{"moreAdvanced.mixedList[1].first", true},
45+
{"moreAdvanced.mixedList[2]", "some string that changes"},
46+
47+
// This index used to be a number, but the number was removed, so what was [4] is now [3].
48+
// We need to tell the client that the scalar value in [3] is no more, while also putting
49+
// an object there to replace it.
50+
{"moreAdvanced.mixedList[3]", null},
51+
{"moreAdvanced.mixedList[3].first", "absolutely not"},
52+
53+
{"moreAdvanced.mixedList[4].first", null},
54+
{"moreAdvanced.mixedList[4].otherRemovedProp", null},
55+
56+
{"removedArray[0]", null},
57+
{"removedArray[1]", null},
58+
{"removedArray[2]", null},
59+
60+
{"arrayWithItemsAdded[3]", 7},
61+
{"arrayWithItemsAdded[4]", 8},
62+
{"arrayWithItemsAdded[5]", true},
63+
64+
{"newArray[0]", 1},
65+
{"newArray[1]", 2},
66+
67+
{"objectReplacedByArray.key", null},
68+
{"objectReplacedByArray[0]", 1},
69+
{"objectReplacedByArray[1]", 2},
70+
71+
{"arrayReplacedByObject[0]", null},
72+
{"arrayReplacedByObject[1]", null},
73+
{"arrayReplacedByObject.key", "value"},
74+
75+
{"scalarReplacedByObject", null},
76+
{"scalarReplacedByObject.key", "value"},
77+
};
78+
79+
changedFields.Should().Equal(expected);
80+
}
81+
}
82+
}

0 commit comments

Comments
 (0)