From 57287b3f698bb28fc1863e235869325819436937 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Sat, 22 Aug 2020 02:00:35 +0530 Subject: [PATCH 1/4] Refactored custom serialization implementation for MessageExtras, Added tests --- src/IO.Ably.Shared/Types/MessageExtras.cs | 16 +-- .../MessageExtrasConverterTests.cs | 130 +++++++++++++++--- 2 files changed, 115 insertions(+), 31 deletions(-) diff --git a/src/IO.Ably.Shared/Types/MessageExtras.cs b/src/IO.Ably.Shared/Types/MessageExtras.cs index 2aba65989..ccf26b8a4 100644 --- a/src/IO.Ably.Shared/Types/MessageExtras.cs +++ b/src/IO.Ably.Shared/Types/MessageExtras.cs @@ -1,3 +1,4 @@ +using IO.Ably.CustomSerialisers; using Newtonsoft.Json; using Newtonsoft.Json.Linq; @@ -6,6 +7,7 @@ namespace IO.Ably.Types /// /// Extra properties on the Message. /// + [JsonConverter(typeof(MessageExtrasConverter))] public class MessageExtras { private const string DeltaProperty = "delta"; @@ -16,6 +18,7 @@ public class MessageExtras /// /// Delta extras is part of the Ably delta's functionality. /// + [JsonIgnore] public DeltaExtras Delta { get; } /// @@ -41,18 +44,7 @@ public MessageExtras(JToken data = null) /// returns the inner json. public JToken ToJson() { - if (Data == null && Delta == null) - { - return null; - } - - var result = Data?.DeepClone() ?? new JObject(); - if (Delta != null) - { - result[DeltaProperty] = JObject.FromObject(Delta); - } - - return result; + return Data?.DeepClone(); } } diff --git a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs index bc87968f5..33aebfeca 100644 --- a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs +++ b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs @@ -1,4 +1,5 @@ using System.Collections.Generic; +using System.Linq; using FluentAssertions; using IO.Ably.CustomSerialisers; using IO.Ably.Types; @@ -11,38 +12,129 @@ namespace IO.Ably.Tests.DotNetCore20.CustomSerializers { public class MessageExtrasConverterTests { - public JsonSerializerSettings JsonSettings + public JsonSerializerSettings JsonSettings = JsonHelper.Settings; + + [Fact] + [Trait("spec ", "tm2i")] + public void ShouldParse_MessageExtrasJson() { - get - { - JsonSerializerSettings res = new JsonSerializerSettings(); - res.Converters = new List() - { - new MessageExtrasConverter(), - }; - res.DateTimeZoneHandling = DateTimeZoneHandling.RoundtripKind; - res.NullValueHandling = NullValueHandling.Ignore; - res.ContractResolver = new CamelCasePropertyNamesContractResolver(); - return res; - } + var json = @" + { + 'random':'boo', + 'delta': { + 'From': '1', + 'Format':'best' + } + }"; + var originalJToken = JToken.Parse(json); + var messageExtras = JsonConvert.DeserializeObject(json, JsonSettings); + + messageExtras.Delta.Should().NotBeNull(); + messageExtras.Delta.From.Should().Be("1"); + messageExtras.Delta.Format.Should().Be("best"); + + ((string)messageExtras.ToJson()["random"]).Should().Be("boo"); + + var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); + var serializedJToken = JToken.Parse(serialized); + + Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); + + // todo: upgrade testing library - https://github.com/fluentassertions/fluentassertions.json/issues/7 + // https://stackoverflow.com/questions/52645603/how-to-compare-two-json-objects-using-c-sharp + + // serializedJToken.Should().BeEquivalentTo(originalJToken); } [Fact] [Trait("spec ", "tm2i")] - public void Should_parse_MessageExtras_json_correctly() + public void ShouldParse_MessageExtrasJson_WithEmptyDelta() { - var json = "{ \"random\":\"boo\", \"delta\":{ \"From\": \"1\", \"Format\":\"best\" } }"; - var originalJObject = JObject.Parse(json); + var json = @"{ + 'random':'boo', + 'foo':'fooValue', + 'bar':'barValue', + 'object' : { + 'key1': 'value1', + 'key2': 'value2' + } + }"; + var originalJToken = JToken.Parse(json); + var messageExtras = JsonConvert.DeserializeObject(json, JsonSettings); + ((string)messageExtras.ToJson()["random"]).Should().Be("boo"); + ((string)messageExtras.ToJson()["foo"]).Should().Be("fooValue"); + ((string)messageExtras.ToJson()["bar"]).Should().Be("barValue"); + ((string)messageExtras.ToJson()["object"]["key1"]).Should().Be("value1"); + ((string)messageExtras.ToJson()["object"]["key2"]).Should().Be("value2"); + + var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); + var serializedJToken = JToken.Parse(serialized); + Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); + } + + [Fact] + [Trait("spec ", "tm2i")] + public void ShouldParse_MessageExtrasJson_WithDelta() + { + var json = @"{ + 'delta': { + 'From': '1', + 'Format':'best' + } + }"; + var originalJToken = JToken.Parse(json); var messageExtras = JsonConvert.DeserializeObject(json, JsonSettings); messageExtras.Delta.Should().NotBeNull(); messageExtras.Delta.From.Should().Be("1"); messageExtras.Delta.Format.Should().Be("best"); - ((string)messageExtras.ToJson()["random"]).Should().Be("boo"); var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); - var serializedJObject = JObject.Parse(serialized); - JToken.DeepEquals(serializedJObject, originalJObject).Should().BeTrue(); + var serializedJToken = JToken.Parse(serialized); + Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); + } + + [Fact] + [Trait("spec ", "tm2i")] + public void ShouldParse_Message_WithNullMessageExtras() + { + var json = @"{ + 'id':'UniqueId', + 'clientId':'clientId', + 'connectionId':'connectionId', + 'name':'connectionName', + 'data':'data', + 'encoding':'encoding', + 'extras': null + }"; + + var messageObject = JsonConvert.DeserializeObject(json, JsonSettings); + messageObject.Extras.Should().BeNull(); + var serialized = JsonConvert.SerializeObject(messageObject, JsonSettings); + var serializedJToken = JToken.Parse(serialized); + serializedJToken.Contains("extras").Should().Be(false); + } + + [Fact] + [Trait("spec ", "tm2i")] + public void ShouldSerialize_Message_WithArbitraryMessageExtras() + { + var json = @"{ + 'id':'UniqueId', + 'clientId':'clientId', + 'connectionId':'connectionId', + 'name':'connectionName', + 'data':'data', + 'extras': 'extraData', + 'encoding':'encoding' + }"; + + var messageObject = JsonConvert.DeserializeObject(json, JsonSettings); + messageObject.Extras.Delta.Should().BeNull(); + messageObject.Extras.ToJson().ToString().Should().Be("extraData"); + var serialized = JsonConvert.SerializeObject(messageObject, JsonSettings); + var serializedJToken = JToken.Parse(serialized); + serializedJToken.Contains("extras").Should().Be(false); } } } From d214e3ccee7a0a15adb60252b9a673e393197305 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Sat, 22 Aug 2020 02:16:05 +0530 Subject: [PATCH 2/4] Updated MessageExtras factname to more consistant name --- .../CustomSerializers/MessageExtrasConverterTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs index 33aebfeca..e1ce5ef43 100644 --- a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs +++ b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs @@ -117,7 +117,7 @@ public void ShouldParse_Message_WithNullMessageExtras() [Fact] [Trait("spec ", "tm2i")] - public void ShouldSerialize_Message_WithArbitraryMessageExtras() + public void ShouldParse_Message_WithArbitraryMessageExtras() { var json = @"{ 'id':'UniqueId', From 30bcd91ce705703f3266c29e171f25dda64c0484 Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Mon, 24 Aug 2020 20:23:12 +0530 Subject: [PATCH 3/4] Marked delta property to be private, Modified constructor accessor Moved deserialization to internal static method --- .../MessageExtrasConverter.cs | 2 +- src/IO.Ably.Shared/Types/MessageExtras.cs | 20 +++++++++++++++++-- 2 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/IO.Ably.Shared/CustomSerialisers/MessageExtrasConverter.cs b/src/IO.Ably.Shared/CustomSerialisers/MessageExtrasConverter.cs index 29dfccc55..e834501eb 100644 --- a/src/IO.Ably.Shared/CustomSerialisers/MessageExtrasConverter.cs +++ b/src/IO.Ably.Shared/CustomSerialisers/MessageExtrasConverter.cs @@ -20,7 +20,7 @@ public override void WriteJson(JsonWriter writer, object value, JsonSerializer s public override object ReadJson(JsonReader reader, Type objectType, object existingValue, JsonSerializer serializer) { var extrasToken = JToken.Load(reader); - return new MessageExtras(extrasToken); + return MessageExtras.From(extrasToken); } public override bool CanConvert(Type objectType) diff --git a/src/IO.Ably.Shared/Types/MessageExtras.cs b/src/IO.Ably.Shared/Types/MessageExtras.cs index ccf26b8a4..c0aed5510 100644 --- a/src/IO.Ably.Shared/Types/MessageExtras.cs +++ b/src/IO.Ably.Shared/Types/MessageExtras.cs @@ -12,8 +12,11 @@ public class MessageExtras { private const string DeltaProperty = "delta"; + /// + /// Data holds actual extra information associated with message. + /// [JsonIgnore] - private JToken Data { get; } + public JToken Data { get; } /// /// Delta extras is part of the Ably delta's functionality. @@ -26,16 +29,29 @@ public class MessageExtras /// /// the json object passed to Message extras. public MessageExtras(JToken data = null) + : this(data, null) + { + } + + private MessageExtras(JToken data, DeltaExtras delta) { Data = data; + Delta = delta; + } + + internal static MessageExtras From(JToken data = null) + { + DeltaExtras delta = null; if (data != null && data is JObject dataObject) { var deltaProp = dataObject[DeltaProperty]; if (deltaProp != null && deltaProp is JObject deltaObject) { - Delta = deltaObject.ToObject(); + delta = deltaObject.ToObject(); } } + + return new MessageExtras(data, delta); } /// From 28e8fe4d580e4b5248c0e2ed2dd2be23f9f594dc Mon Sep 17 00:00:00 2001 From: sacOO7 Date: Wed, 26 Aug 2020 00:31:03 +0530 Subject: [PATCH 4/4] Reverted Data property back to private, added Helper method for Json DeepEqual --- src/IO.Ably.Shared/Types/MessageExtras.cs | 2 +- .../MessageExtrasConverterTests.cs | 20 ++- src/IO.Ably.Tests.Shared/Helpers/JAssert.cs | 24 +++ src/IO.Ably.Tests.Shared/Helpers/JDiff.cs | 147 ++++++++++++++++++ .../IO.Ably.Tests.Shared.projitems | 2 + 5 files changed, 186 insertions(+), 9 deletions(-) create mode 100644 src/IO.Ably.Tests.Shared/Helpers/JAssert.cs create mode 100644 src/IO.Ably.Tests.Shared/Helpers/JDiff.cs diff --git a/src/IO.Ably.Shared/Types/MessageExtras.cs b/src/IO.Ably.Shared/Types/MessageExtras.cs index c0aed5510..25fbcb457 100644 --- a/src/IO.Ably.Shared/Types/MessageExtras.cs +++ b/src/IO.Ably.Shared/Types/MessageExtras.cs @@ -16,7 +16,7 @@ public class MessageExtras /// Data holds actual extra information associated with message. /// [JsonIgnore] - public JToken Data { get; } + private JToken Data { get; } /// /// Delta extras is part of the Ably delta's functionality. diff --git a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs index e1ce5ef43..4a9fb010d 100644 --- a/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs +++ b/src/IO.Ably.Tests.Shared/CustomSerializers/MessageExtrasConverterTests.cs @@ -1,19 +1,28 @@ +using System; using System.Collections.Generic; using System.Linq; using FluentAssertions; using IO.Ably.CustomSerialisers; +using IO.Ably.Tests.Shared.Helpers; using IO.Ably.Types; using Newtonsoft.Json; using Newtonsoft.Json.Linq; using Newtonsoft.Json.Serialization; using Xunit; +using Xunit.Abstractions; namespace IO.Ably.Tests.DotNetCore20.CustomSerializers { public class MessageExtrasConverterTests { + private readonly ITestOutputHelper _testOutputHelper; public JsonSerializerSettings JsonSettings = JsonHelper.Settings; + public MessageExtrasConverterTests(ITestOutputHelper testOutputHelper) + { + _testOutputHelper = testOutputHelper; + } + [Fact] [Trait("spec ", "tm2i")] public void ShouldParse_MessageExtrasJson() @@ -38,12 +47,7 @@ public void ShouldParse_MessageExtrasJson() var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); var serializedJToken = JToken.Parse(serialized); - Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); - - // todo: upgrade testing library - https://github.com/fluentassertions/fluentassertions.json/issues/7 - // https://stackoverflow.com/questions/52645603/how-to-compare-two-json-objects-using-c-sharp - - // serializedJToken.Should().BeEquivalentTo(originalJToken); + JAssert.DeepEquals(serializedJToken, originalJToken, _testOutputHelper).Should().Be(true); } [Fact] @@ -69,7 +73,7 @@ public void ShouldParse_MessageExtrasJson_WithEmptyDelta() var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); var serializedJToken = JToken.Parse(serialized); - Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); + JAssert.DeepEquals(serializedJToken, originalJToken, _testOutputHelper).Should().Be(true); } [Fact] @@ -91,7 +95,7 @@ public void ShouldParse_MessageExtrasJson_WithDelta() var serialized = JsonConvert.SerializeObject(messageExtras, JsonSettings); var serializedJToken = JToken.Parse(serialized); - Assert.True(JToken.DeepEquals(serializedJToken, originalJToken)); + JAssert.DeepEquals(serializedJToken, originalJToken, _testOutputHelper).Should().Be(true); } [Fact] diff --git a/src/IO.Ably.Tests.Shared/Helpers/JAssert.cs b/src/IO.Ably.Tests.Shared/Helpers/JAssert.cs new file mode 100644 index 000000000..1c0091daf --- /dev/null +++ b/src/IO.Ably.Tests.Shared/Helpers/JAssert.cs @@ -0,0 +1,24 @@ +using System; +using Newtonsoft.Json.Linq; +using Xunit.Abstractions; + +namespace IO.Ably.Tests.Shared.Helpers +{ + internal class JAssert + { + // todo: upgrade testing library - https://github.com/fluentassertions/fluentassertions.json/issues/7 + // https://stackoverflow.com/questions/52645603/how-to-compare-two-json-objects-using-c-sharp + + public static bool DeepEquals(JToken token1, JToken token2, ITestOutputHelper testOutputHelper) + { + var areEqual = JToken.DeepEquals(token1, token2); + if (!areEqual) + { + var diff = JDiff.Differentiate(token1, token2); + testOutputHelper.WriteLine($"Json Difference {diff}"); + } + + return areEqual; + } + } +} diff --git a/src/IO.Ably.Tests.Shared/Helpers/JDiff.cs b/src/IO.Ably.Tests.Shared/Helpers/JDiff.cs new file mode 100644 index 000000000..80e1e4e50 --- /dev/null +++ b/src/IO.Ably.Tests.Shared/Helpers/JDiff.cs @@ -0,0 +1,147 @@ +using System; +using System.Linq; +using Newtonsoft.Json.Linq; + +namespace IO.Ably.Tests.Shared +{ + public static class JDiff + { + public static JToken Differentiate(JToken first, JToken second) + { + if (JToken.DeepEquals(first, second)) + { + return null; + } + + if (first != null && second != null && first?.GetType() != second?.GetType()) + { + throw new InvalidOperationException($"Operands' types must match; '{first.GetType().Name}' <> '{second.GetType().Name}'"); + } + + var propertyNames = (first?.Children() ?? default).Union(second?.Children() ?? default)?.Select(_ => (_ as JProperty)?.Name)?.Distinct(); + + if (!propertyNames.Any() && (first is JValue || second is JValue)) + { + return (first == null) ? second : first; + } + + var difference = JToken.Parse("{}"); + + foreach (var property in propertyNames) + { + if (property == null) + { + if (first == null) + { + difference = second; + } + + // array of object? + else if (first is JArray && first.Children().All(c => !(c is JValue))) + { + var difrences = new JArray(); + // var mode = second == null ? '-' : '*'; + var maximum = Math.Max(first?.Count() ?? 0, second?.Count() ?? 0); + + for (int i = 0; i < maximum; i++) + { + var firstsItem = first?.ElementAtOrDefault(i); + var secondsItem = second?.ElementAtOrDefault(i); + + var diff = Differentiate(firstsItem, secondsItem); + + if (diff != null) + { + difrences.Add(diff); + } + } + + if (difrences.HasValues) + { + difference/*[$"{mode}{property}"] */ = difrences; + } + } + else + { + difference = first; + } + + continue; + } + + if (first?[property] == null) + { + var secondVal = second?[property]?.Parent as JProperty; + + difference[$"+{property}"] = secondVal.Value; + + continue; + } + + if (second?[property] == null) + { + var firstVal = first?[property]?.Parent as JProperty; + + difference[$"-{property}"] = firstVal.Value; + + continue; + } + + if (first?[property] is JValue value) + { + if (!JToken.DeepEquals(first?[property], second?[property])) + { + difference[$"*{property}"] = value; + } + + continue; + } + + if (first?[property] is JObject) + { + var mode = second?[property] == null ? '-' : '*'; + var firstsItem = first[property]; + var secondsItem = second[property]; + + var diffrence = Differentiate(firstsItem, secondsItem); + + if (diffrence != null /*&& diffrence.Count() > 0*/) + { + difference[$"{mode}{property}"] = diffrence; + } + + continue; + } + + if (first?[property] is JArray) + { + var difrences = new JArray(); + var mode = second?[property] == null ? '-' : '*'; + var maximum = Math.Max(first?[property]?.Count() ?? 0, second?[property]?.Count() ?? 0); + + for (int i = 0; i < maximum; i++) + { + var firstsItem = first[property]?.ElementAtOrDefault(i); + var secondsItem = second[property]?.ElementAtOrDefault(i); + + var diff = Differentiate(firstsItem, secondsItem); + + if (diff != null) + { + difrences.Add(diff); + } + } + + if (difrences.HasValues) + { + difference[$"{mode}{property}"] = difrences; + } + + continue; + } + } + + return difference; + } + } +} diff --git a/src/IO.Ably.Tests.Shared/IO.Ably.Tests.Shared.projitems b/src/IO.Ably.Tests.Shared/IO.Ably.Tests.Shared.projitems index 04dd59216..94022ff75 100644 --- a/src/IO.Ably.Tests.Shared/IO.Ably.Tests.Shared.projitems +++ b/src/IO.Ably.Tests.Shared/IO.Ably.Tests.Shared.projitems @@ -26,6 +26,7 @@ + @@ -52,6 +53,7 @@ +