Fix serialization not accepting JsonObject/Dictionary with null values (

#107)

* add null checks to compat apis

* update serialization.net version

* add unit tests

---------

Co-authored-by: Wenxi Zeng <[email protected]>

Analytics-CSharp/Analytics-CSharp.csproj

@@ -39,7 +39,7 @@
   </ItemGroup>
   <ItemGroup>
     <PackageReference Include="Coroutine.NET" Version="1.4.0" />
-    <PackageReference Include="Serialization.NET" Version="1.4.0" />
+    <PackageReference Include="Serialization.NET" Version="1.4.1" />
     <PackageReference Include="Sovran.NET" Version="1.4.0" />
   </ItemGroup>
   <ItemGroup>

Analytics-CSharp/Segment/Analytics/Compat/Migration.cs

@@ -1,9 +1,5 @@
 using System;
 using System.Collections.Generic;
-using System.Linq;
-using System.Net.Http.Headers;
-using System.Reflection;
-using Segment.Analytics;
 using Segment.Serialization;
 
 namespace Segment.Analytics.Compat
@@ -31,9 +27,13 @@ public static void Track(this Analytics analytics, string userId, string eventNa
         public static void Track(this Analytics analytics, string userId, string eventName,
             Dictionary<string, object> properties)
         {
+            if (properties == null)
+            {
+                properties = new Dictionary<string, object>();
+            }
             properties.Add("userId", userId);
             analytics.Track(eventName,
-                JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
+                JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
@@ -43,36 +43,48 @@ public static void Screen(this Analytics analytics, string userId, string eventN
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
-        public static void Screen(this Analytics analytics, string userId, string eventName,
+        public static void Screen(this Analytics analytics, string userId, string title,
             Dictionary<string, object> properties)
         {
+            if (properties == null)
+            {
+                properties = new Dictionary<string, object>();
+            }
             properties.Add("userId", userId);
-            analytics.Screen(eventName,
-                JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
+            analytics.Screen(title,
+                JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
-        public static void Page(this Analytics analytics, string userId, string eventName)
+        public static void Page(this Analytics analytics, string userId, string title)
         {
-            analytics.Page(eventName, new JsonObject() {{"userId", userId}});
+            analytics.Page(title, new JsonObject() {{"userId", userId}});
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
-        public static void Page(this Analytics analytics, string userId, string eventName,
+        public static void Page(this Analytics analytics, string userId, string title,
             Dictionary<string, object> properties)
         {
+            if (properties == null)
+            {
+                properties = new Dictionary<string, object>();
+            }
             properties.Add("userId", userId);
-            analytics.Page(eventName,
-                JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
+            analytics.Page(title,
+                JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
         public static void Group(this Analytics analytics, string userId, string groupId,
             Dictionary<string, object> traits)
         {
+            if (traits == null)
+            {
+                traits = new Dictionary<string, object>();
+            }
             traits.Add("userId", userId);
             analytics.Group(groupId,
-                JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(traits)));
+                JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(traits)));
         }
 
         [Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]

Tests/Compat/MigrationTest.cs

@@ -0,0 +1,182 @@
+using System.Collections.Generic;
+using Moq;
+using Segment.Analytics;
+using Segment.Analytics.Utilities;
+using Segment.Analytics.Compat;
+using Segment.Serialization;
+using Tests.Utils;
+using Xunit;
+
+namespace Tests.Compat
+{
+    public class MigrationTest
+    {
+        private readonly Analytics _analytics;
+
+        private Settings? _settings;
+
+        private readonly Mock<StubAfterEventPlugin> _plugin;
+
+        public MigrationTest()
+        {
+            _settings = JsonUtility.FromJson<Settings?>(
+                "{\"integrations\":{\"Segment.io\":{\"apiKey\":\"1vNgUqwJeCHmqgI9S1sOm9UHCyfYqbaQ\"}},\"plan\":{},\"edgeFunction\":{}}");
+
+            var mockHttpClient = new Mock<HTTPClient>(null, null, null);
+            mockHttpClient
+                .Setup(httpClient => httpClient.Settings())
+                .ReturnsAsync(_settings);
+
+            _plugin = new Mock<StubAfterEventPlugin>
+            {
+                CallBase = true
+            };
+
+            var config = new Configuration(
+                writeKey: "123",
+                storageProvider: new DefaultStorageProvider("tests"),
+                autoAddSegmentDestination: false,
+                useSynchronizeDispatcher: true,
+                httpClientProvider: new MockHttpClientProvider(mockHttpClient)
+            );
+            _analytics = new Analytics(config);
+            _analytics.Add(new UserIdPlugin());
+        }
+
+        [Fact]
+        public void TestCompatTrackAcceptNullDict()
+        {
+            var actual = new List<TrackEvent>();
+            _plugin.Setup(o => o.Track(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Track("user123", "foo", null);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Event);
+        }
+
+        [Fact]
+        public void TestCompatTrackAcceptDictWithNulls()
+        {
+            var properties = new Dictionary<string, object>
+            {
+                ["nullValue"] = null
+            };
+            var actual = new List<TrackEvent>();
+            _plugin.Setup(o => o.Track(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Track("user123", "foo", properties);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Event);
+            Assert.True(actual[0].Properties.ContainsKey("nullValue"));
+            Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
+        }
+
+        [Fact]
+        public void TestCompatScreenAcceptNullDict()
+        {
+            var actual = new List<ScreenEvent>();
+            _plugin.Setup(o => o.Screen(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Screen("user123", "foo", null);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Name);
+        }
+
+        [Fact]
+        public void TestCompatScreenAcceptDictWithNulls()
+        {
+            var properties = new Dictionary<string, object>
+            {
+                ["nullValue"] = null
+            };
+            var actual = new List<ScreenEvent>();
+            _plugin.Setup(o => o.Screen(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Screen("user123", "foo", properties);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Name);
+            Assert.True(actual[0].Properties.ContainsKey("nullValue"));
+            Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
+        }
+
+        [Fact]
+        public void TestCompatPageAcceptNullDict()
+        {
+            var actual = new List<PageEvent>();
+            _plugin.Setup(o => o.Page(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Page("user123", "foo", null);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Name);
+        }
+
+        [Fact]
+        public void TestCompatPageAcceptDictWithNulls()
+        {
+            var properties = new Dictionary<string, object>
+            {
+                ["nullValue"] = null
+            };
+            var actual = new List<PageEvent>();
+            _plugin.Setup(o => o.Page(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Page("user123", "foo", properties);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Properties.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].Name);
+            Assert.True(actual[0].Properties.ContainsKey("nullValue"));
+            Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
+        }
+
+        [Fact]
+        public void TestCompatGroupAcceptNullDict()
+        {
+            var actual = new List<GroupEvent>();
+            _plugin.Setup(o => o.Group(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Group("user123", "foo", null);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Traits.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].GroupId);
+        }
+
+        [Fact]
+        public void TestCompatGroupAcceptDictWithNulls()
+        {
+            var properties = new Dictionary<string, object>
+            {
+                ["nullValue"] = null
+            };
+            var actual = new List<GroupEvent>();
+            _plugin.Setup(o => o.Group(Capture.In(actual)));
+            _analytics.Add(_plugin.Object);
+            _analytics.Group("user123", "foo", properties);
+
+            Assert.NotEmpty(actual);
+            Assert.False(actual[0].Traits.ContainsKey("userId"));
+            Assert.Equal("user123", actual[0].UserId);
+            Assert.Equal("foo", actual[0].GroupId);
+            Assert.True(actual[0].Traits.ContainsKey("nullValue"));
+            Assert.Equal(JsonNull.Instance, actual[0].Traits["nullValue"]);
+        }
+    }
+}

Tests/Utils/Stubs.cs

@@ -45,6 +45,11 @@ public class StubEventPlugin : EventPlugin
         public override PluginType Type => PluginType.Before;
     }
 
+    public class StubAfterEventPlugin : EventPlugin
+    {
+        public override PluginType Type => PluginType.After;
+    }
+
     public class StubDestinationPlugin : DestinationPlugin
     {
         public override string Key { get; }