diff --git a/src/Altinn.App.Api/Controllers/DataController.cs b/src/Altinn.App.Api/Controllers/DataController.cs index a6f788d72..fa4c2c323 100644 --- a/src/Altinn.App.Api/Controllers/DataController.cs +++ b/src/Altinn.App.Api/Controllers/DataController.cs @@ -550,8 +550,7 @@ private async Task PutFormData(string org, string app, Instance in return BadRequest("No data found in content"); } - string serviceModelJsonString = JsonSerializer.Serialize(serviceModel); - bool changedByCalculation = await _dataProcessor.ProcessDataWrite(instance, dataGuid, serviceModel); + Dictionary changedFields = await JsonHelper.ProcessDataWriteWithDiff(instance, dataGuid, serviceModel, _dataProcessor, _logger); await UpdatePresentationTextsOnInstance(instance, dataType, serviceModel); await UpdateDataValuesOnInstance(instance, dataType, serviceModel); @@ -569,21 +568,10 @@ private async Task PutFormData(string org, string app, Instance in SelfLinkHelper.SetDataAppSelfLinks(instanceOwnerPartyId, instanceGuid, updatedDataElement, Request); string dataUrl = updatedDataElement.SelfLinks.Apps; - - if (changedByCalculation) + if (changedFields is not null) { - string updatedServiceModelString = JsonSerializer.Serialize(serviceModel); CalculationResult calculationResult = new CalculationResult(updatedDataElement); - try - { - Dictionary changedFields = JsonHelper.FindChangedFields(serviceModelJsonString, updatedServiceModelString); - calculationResult.ChangedFields = changedFields; - } - catch (Exception e) - { - _logger.LogError(e, "Unable to determine changed fields"); - } - + calculationResult.ChangedFields = changedFields; return StatusCode((int)HttpStatusCode.SeeOther, calculationResult); } diff --git a/src/Altinn.App.Core/Helpers/JsonHelper.cs b/src/Altinn.App.Core/Helpers/JsonHelper.cs index baafab0cd..6b79d7e16 100644 --- a/src/Altinn.App.Core/Helpers/JsonHelper.cs +++ b/src/Altinn.App.Core/Helpers/JsonHelper.cs @@ -1,5 +1,9 @@ #nullable enable +using System.Numerics; +using Altinn.App.Core.Features; +using Altinn.Platform.Storage.Interface.Models; +using Microsoft.Extensions.Logging; using Newtonsoft.Json.Linq; namespace Altinn.App.Core.Helpers @@ -9,6 +13,38 @@ namespace Altinn.App.Core.Helpers /// public static class JsonHelper { + /// + /// Run DataProcessWrite returning the dictionary of the changed fields. + /// + public static async Task?> ProcessDataWriteWithDiff(Instance instance, Guid dataGuid, object serviceModel, IDataProcessor dataProcessor, ILogger logger) + { + string serviceModelJsonString = System.Text.Json.JsonSerializer.Serialize(serviceModel); + + bool changedByCalculation = await dataProcessor.ProcessDataWrite(instance, dataGuid, serviceModel); + + Dictionary? changedFields = null; + if (changedByCalculation) + { + string updatedServiceModelString = System.Text.Json.JsonSerializer.Serialize(serviceModel); + try + { + changedFields = FindChangedFields(serviceModelJsonString, updatedServiceModelString); + } + catch (Exception e) + { + logger.LogError(e, "Unable to determine changed fields"); + } + } + + // TODO: Consider not bothering frontend with an empty changes list + // if(changedFields?.Count == 0) + // { + // return null; + // } + + return changedFields; + } + /// /// Find changed fields between old and new json objects /// @@ -147,7 +183,13 @@ private static void FindDiff(Dictionary dict, JToken? old, JTok break; default: - dict.Add(prefix, current == null ? null : ((JValue)current).Value); + var convertedValue = (current as JValue)?.Value switch + { + // BigInteger is not supported in json, so try to reduce to decimal, if possible, or string if too big + BigInteger bigInt => bigInt <= new BigInteger(decimal.MaxValue) ? (decimal)bigInt : bigInt.ToString(System.Globalization.NumberFormatInfo.InvariantInfo), + _ => (current as JValue)?.Value + }; + dict.Add(prefix, convertedValue); break; } } diff --git a/test/Altinn.App.Core.Tests/Helpers/JsonHelperTests.cs b/test/Altinn.App.Core.Tests/Helpers/JsonHelperTests.cs new file mode 100644 index 000000000..a2ebf1bcf --- /dev/null +++ b/test/Altinn.App.Core.Tests/Helpers/JsonHelperTests.cs @@ -0,0 +1,273 @@ +#nullable enable + +using System.Text.Json.Serialization; +using Altinn.App.Core.Features; +using Altinn.App.Core.Helpers; +using Altinn.Platform.Storage.Interface.Models; +using FluentAssertions; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Altinn.App.Core.Tests.Helpers; + +public class JsonHelperTests +{ + /// + /// Helper method to setup and get the dictionary of the diffs + /// + public async Task?> DoTest(TModel model, Func processDataWriteImpl) + where TModel : class + { + var instance = new Instance(); + var logger = new Mock().Object; + var guid = Guid.Empty; + var dataProcessorMock = new Mock(); + Func> dataProcessWrite = (instance, guid, model) => Task.FromResult(processDataWriteImpl((TModel)model)); + dataProcessorMock + .Setup((d) => d.ProcessDataWrite(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(dataProcessWrite); + + return await JsonHelper.ProcessDataWriteWithDiff(instance, guid, model, dataProcessorMock.Object, logger); + } + + public class TestModel + { + public int IntegerTest { get; set; } + + public int? NullableIntTest { get; set; } + + public decimal DecimalTest { get; set; } + + public decimal? NullableDecimalTest { get; set; } + + public string StringTest { get; set; } = null!; + + public string? NullableStringTest { get; set; } + + // [Newtonsoft.Json.JsonProperty("jsonPropertyName")] + [JsonPropertyName("jsonPropertyName")] + public string? NotJsonPropertyNameTest { get; set; } + + // [JsonPropertyName("newtonsoftString")] + [Newtonsoft.Json.JsonProperty("newtonsoftString")] + public string? NewtonsoftAttributeWorks { get; set; } + + public TestModel? RecursiveTest { get; set; } + + public TestModel NonNullableRecursiveTest { get; set; } = default!; + + public List PrimitiveList { get; set; } = default!; + + public List ListTest { get; set; } = default!; + + public List? NullableListTest { get; set; } + } + + [Fact] + public async Task SimpleNoChangeTest() + { + var data = new TestModel(); + var diff = await DoTest(data, (model) => false); + diff.Should().BeNull(); + } + + [Fact(Skip = "Curently empty diffs are sendt to frontend, so the result here is an empty dictionary")] + public async Task SimpleNoChangeTestTrueReturn() + { + var data = new TestModel(); + var diff = await DoTest(data, (model) => true); + diff.Should().BeNull(); + } + + [Fact] + public async Task InitializingPropertiesLeadsToNoDiff() + { + var data = new TestModel(); + var diff = await DoTest(data, (model) => + { + model.ListTest = new(); + model.PrimitiveList = new(); + model.NullableListTest = new(); + return true; + }); + + // Might be null in the future + diff.Should().BeEmpty(); + } + + [Fact] + public async Task InitializingNonNullablePropertiesCreatesDiff() + { + var data = new TestModel(); + var diff = await DoTest(data, (model) => + { + model.RecursiveTest = new(); + return true; + }); + + // Not sure if RecursiveTest should be null here, but apparently it does not hurt + diff.Should().Equal(new Dictionary() + { + {"RecursiveTest.IntegerTest", 0}, + {"RecursiveTest.DecimalTest", 0M}, + {"RecursiveTest", null}, + }); + } + + [Fact] + public async Task NullIsNotZero() + { + var data = new TestModel() + { + NullableIntTest = null, + }; + var diff = await DoTest(data, (model) => + { + model.NullableIntTest = 0; + return true; + }); + + diff.Should().Contain("NullableIntTest", 0); + diff.Should().HaveCount(1); + } + + [Fact] + public async Task ZeroIsNotNull() + { + var data = new TestModel() + { + NullableIntTest = 0, + }; + var diff = await DoTest(data, (model) => + { + model.NullableIntTest = null; + return true; + }); + + diff.Should().Contain("NullableIntTest", null); + diff.Should().HaveCount(1); + } + + [Fact] + public async Task TestSystemTextJsonAnnotation() + { + var data = new TestModel() + { + NotJsonPropertyNameTest = "Original Value", + }; + var diff = await DoTest(data, (model) => + { + model.NotJsonPropertyNameTest = "New Value"; + return true; + }); + + diff.Should().Equal(new Dictionary + { + {"jsonPropertyName", "New Value"}, + }); + } + + [Fact(Skip = "System.Text.Json annotation is required")] + public async Task NewtonsoftJsonAnnotation() + { + var data = new TestModel() + { + NewtonsoftAttributeWorks = "Original Value", + }; + var diff = await DoTest(data, (model) => + { + model.NewtonsoftAttributeWorks = "New Value"; + return true; + }); + + diff.Should().Equal(new Dictionary + { + {"newtonsoftString", "New Value"}, + }); + } + + [Theory] + [InlineData(-1)] + [InlineData(10)] + [InlineData(100)] + [InlineData(int.MinValue)] + [InlineData(int.MaxValue)] + public async Task ChangeInteger(int value) + { + var data = new TestModel() + { + RecursiveTest = new(), + PrimitiveList = new(), + }; + + var diff = await DoTest(data, (model) => + { + model.IntegerTest = value; + model.NullableIntTest = value; + model.RecursiveTest ??= new(); + model.RecursiveTest.IntegerTest = value; + model.PrimitiveList ??= new(); + model.PrimitiveList.Add(value); + return true; + }); + + diff.Should().BeEquivalentTo(new Dictionary() + { + { "IntegerTest", value }, + { "NullableIntTest", value }, + { "RecursiveTest.IntegerTest", value }, + { "PrimitiveList[0]", value }, + }); + } + + [Theory] + [InlineData("-1")] + [InlineData("10")] + [InlineData("100")] + [InlineData(int.MinValue)] + [InlineData(int.MaxValue)] + [InlineData(long.MinValue)] + [InlineData(long.MaxValue)] + [InlineData("9223372036854775808")] + [InlineData("79228162514264337593543950335")] + [InlineData("-79228162514264337593543950334")] + public async Task ChangeDecimal(object rawValue) + { + var value = rawValue switch + { + string stringValue => decimal.Parse(stringValue), + int intValue => (decimal)intValue, + long longValue => (decimal)longValue, + _ => throw new NotImplementedException(), + }; + var data = new TestModel() + { + RecursiveTest = new(), + ListTest = new() { new() }, + NullableListTest = new() { new() }, + }; + var diff = await DoTest(data, (model) => + { + model.DecimalTest = value; + model.NullableDecimalTest = value; + model.RecursiveTest ??= new(); + model.RecursiveTest.DecimalTest = value; + model.NullableListTest ??= new(); + model.NullableListTest[0].DecimalTest = value; + model.ListTest ??= new(); + model.ListTest[0].DecimalTest = value; + return true; + }); + + // casting is weird (the current implementation of diff returns System.Numerics.BigInteger for large numbers) + Func isMatch = x => (decimal?)(dynamic?)x == value; + + diff.Should().ContainKey("DecimalTest").WhoseValue.Should().Match(x => isMatch(x)); + diff.Should().ContainKey("NullableDecimalTest").WhoseValue.Should().Match(x => isMatch(x)); + diff.Should().ContainKey("RecursiveTest.DecimalTest").WhoseValue.Should().Match(x => isMatch(x)); + diff.Should().ContainKey("NullableListTest[0].DecimalTest").WhoseValue.Should().Match(x => isMatch(x)); + diff.Should().ContainKey("ListTest[0].DecimalTest").WhoseValue.Should().Match(x => isMatch(x)); + diff.Should().HaveCount(5); + } +} \ No newline at end of file