Skip to content

Commit 4aa241b

Browse files
authored
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]>
1 parent d153aff commit 4aa241b

File tree

4 files changed

+214
-15
lines changed

4 files changed

+214
-15
lines changed

Analytics-CSharp/Analytics-CSharp.csproj

+1-1
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@
3939
</ItemGroup>
4040
<ItemGroup>
4141
<PackageReference Include="Coroutine.NET" Version="1.4.0" />
42-
<PackageReference Include="Serialization.NET" Version="1.4.0" />
42+
<PackageReference Include="Serialization.NET" Version="1.4.1" />
4343
<PackageReference Include="Sovran.NET" Version="1.4.0" />
4444
</ItemGroup>
4545
<ItemGroup>

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

+26-14
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,5 @@
11
using System;
22
using System.Collections.Generic;
3-
using System.Linq;
4-
using System.Net.Http.Headers;
5-
using System.Reflection;
6-
using Segment.Analytics;
73
using Segment.Serialization;
84

95
namespace Segment.Analytics.Compat
@@ -31,9 +27,13 @@ public static void Track(this Analytics analytics, string userId, string eventNa
3127
public static void Track(this Analytics analytics, string userId, string eventName,
3228
Dictionary<string, object> properties)
3329
{
30+
if (properties == null)
31+
{
32+
properties = new Dictionary<string, object>();
33+
}
3434
properties.Add("userId", userId);
3535
analytics.Track(eventName,
36-
JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
36+
JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
3737
}
3838

3939
[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
4343
}
4444

4545
[Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
46-
public static void Screen(this Analytics analytics, string userId, string eventName,
46+
public static void Screen(this Analytics analytics, string userId, string title,
4747
Dictionary<string, object> properties)
4848
{
49+
if (properties == null)
50+
{
51+
properties = new Dictionary<string, object>();
52+
}
4953
properties.Add("userId", userId);
50-
analytics.Screen(eventName,
51-
JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
54+
analytics.Screen(title,
55+
JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
5256
}
5357

5458
[Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
55-
public static void Page(this Analytics analytics, string userId, string eventName)
59+
public static void Page(this Analytics analytics, string userId, string title)
5660
{
57-
analytics.Page(eventName, new JsonObject() {{"userId", userId}});
61+
analytics.Page(title, new JsonObject() {{"userId", userId}});
5862
}
5963

6064
[Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
61-
public static void Page(this Analytics analytics, string userId, string eventName,
65+
public static void Page(this Analytics analytics, string userId, string title,
6266
Dictionary<string, object> properties)
6367
{
68+
if (properties == null)
69+
{
70+
properties = new Dictionary<string, object>();
71+
}
6472
properties.Add("userId", userId);
65-
analytics.Page(eventName,
66-
JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(properties)));
73+
analytics.Page(title,
74+
JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(properties)));
6775
}
6876

6977
[Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]
7078
public static void Group(this Analytics analytics, string userId, string groupId,
7179
Dictionary<string, object> traits)
7280
{
81+
if (traits == null)
82+
{
83+
traits = new Dictionary<string, object>();
84+
}
7385
traits.Add("userId", userId);
7486
analytics.Group(groupId,
75-
JsonUtility.FromJson<Segment.Serialization.JsonObject>(JsonUtility.ToJson(traits)));
87+
JsonUtility.FromJson<JsonObject>(JsonUtility.ToJson(traits)));
7688
}
7789

7890
[Obsolete("This should only be used if migrating from Analytics.NET or Analytics.Xamarin")]

Tests/Compat/MigrationTest.cs

+182
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,182 @@
1+
using System.Collections.Generic;
2+
using Moq;
3+
using Segment.Analytics;
4+
using Segment.Analytics.Utilities;
5+
using Segment.Analytics.Compat;
6+
using Segment.Serialization;
7+
using Tests.Utils;
8+
using Xunit;
9+
10+
namespace Tests.Compat
11+
{
12+
public class MigrationTest
13+
{
14+
private readonly Analytics _analytics;
15+
16+
private Settings? _settings;
17+
18+
private readonly Mock<StubAfterEventPlugin> _plugin;
19+
20+
public MigrationTest()
21+
{
22+
_settings = JsonUtility.FromJson<Settings?>(
23+
"{\"integrations\":{\"Segment.io\":{\"apiKey\":\"1vNgUqwJeCHmqgI9S1sOm9UHCyfYqbaQ\"}},\"plan\":{},\"edgeFunction\":{}}");
24+
25+
var mockHttpClient = new Mock<HTTPClient>(null, null, null);
26+
mockHttpClient
27+
.Setup(httpClient => httpClient.Settings())
28+
.ReturnsAsync(_settings);
29+
30+
_plugin = new Mock<StubAfterEventPlugin>
31+
{
32+
CallBase = true
33+
};
34+
35+
var config = new Configuration(
36+
writeKey: "123",
37+
storageProvider: new DefaultStorageProvider("tests"),
38+
autoAddSegmentDestination: false,
39+
useSynchronizeDispatcher: true,
40+
httpClientProvider: new MockHttpClientProvider(mockHttpClient)
41+
);
42+
_analytics = new Analytics(config);
43+
_analytics.Add(new UserIdPlugin());
44+
}
45+
46+
[Fact]
47+
public void TestCompatTrackAcceptNullDict()
48+
{
49+
var actual = new List<TrackEvent>();
50+
_plugin.Setup(o => o.Track(Capture.In(actual)));
51+
_analytics.Add(_plugin.Object);
52+
_analytics.Track("user123", "foo", null);
53+
54+
Assert.NotEmpty(actual);
55+
Assert.False(actual[0].Properties.ContainsKey("userId"));
56+
Assert.Equal("user123", actual[0].UserId);
57+
Assert.Equal("foo", actual[0].Event);
58+
}
59+
60+
[Fact]
61+
public void TestCompatTrackAcceptDictWithNulls()
62+
{
63+
var properties = new Dictionary<string, object>
64+
{
65+
["nullValue"] = null
66+
};
67+
var actual = new List<TrackEvent>();
68+
_plugin.Setup(o => o.Track(Capture.In(actual)));
69+
_analytics.Add(_plugin.Object);
70+
_analytics.Track("user123", "foo", properties);
71+
72+
Assert.NotEmpty(actual);
73+
Assert.False(actual[0].Properties.ContainsKey("userId"));
74+
Assert.Equal("user123", actual[0].UserId);
75+
Assert.Equal("foo", actual[0].Event);
76+
Assert.True(actual[0].Properties.ContainsKey("nullValue"));
77+
Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
78+
}
79+
80+
[Fact]
81+
public void TestCompatScreenAcceptNullDict()
82+
{
83+
var actual = new List<ScreenEvent>();
84+
_plugin.Setup(o => o.Screen(Capture.In(actual)));
85+
_analytics.Add(_plugin.Object);
86+
_analytics.Screen("user123", "foo", null);
87+
88+
Assert.NotEmpty(actual);
89+
Assert.False(actual[0].Properties.ContainsKey("userId"));
90+
Assert.Equal("user123", actual[0].UserId);
91+
Assert.Equal("foo", actual[0].Name);
92+
}
93+
94+
[Fact]
95+
public void TestCompatScreenAcceptDictWithNulls()
96+
{
97+
var properties = new Dictionary<string, object>
98+
{
99+
["nullValue"] = null
100+
};
101+
var actual = new List<ScreenEvent>();
102+
_plugin.Setup(o => o.Screen(Capture.In(actual)));
103+
_analytics.Add(_plugin.Object);
104+
_analytics.Screen("user123", "foo", properties);
105+
106+
Assert.NotEmpty(actual);
107+
Assert.False(actual[0].Properties.ContainsKey("userId"));
108+
Assert.Equal("user123", actual[0].UserId);
109+
Assert.Equal("foo", actual[0].Name);
110+
Assert.True(actual[0].Properties.ContainsKey("nullValue"));
111+
Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
112+
}
113+
114+
[Fact]
115+
public void TestCompatPageAcceptNullDict()
116+
{
117+
var actual = new List<PageEvent>();
118+
_plugin.Setup(o => o.Page(Capture.In(actual)));
119+
_analytics.Add(_plugin.Object);
120+
_analytics.Page("user123", "foo", null);
121+
122+
Assert.NotEmpty(actual);
123+
Assert.False(actual[0].Properties.ContainsKey("userId"));
124+
Assert.Equal("user123", actual[0].UserId);
125+
Assert.Equal("foo", actual[0].Name);
126+
}
127+
128+
[Fact]
129+
public void TestCompatPageAcceptDictWithNulls()
130+
{
131+
var properties = new Dictionary<string, object>
132+
{
133+
["nullValue"] = null
134+
};
135+
var actual = new List<PageEvent>();
136+
_plugin.Setup(o => o.Page(Capture.In(actual)));
137+
_analytics.Add(_plugin.Object);
138+
_analytics.Page("user123", "foo", properties);
139+
140+
Assert.NotEmpty(actual);
141+
Assert.False(actual[0].Properties.ContainsKey("userId"));
142+
Assert.Equal("user123", actual[0].UserId);
143+
Assert.Equal("foo", actual[0].Name);
144+
Assert.True(actual[0].Properties.ContainsKey("nullValue"));
145+
Assert.Equal(JsonNull.Instance, actual[0].Properties["nullValue"]);
146+
}
147+
148+
[Fact]
149+
public void TestCompatGroupAcceptNullDict()
150+
{
151+
var actual = new List<GroupEvent>();
152+
_plugin.Setup(o => o.Group(Capture.In(actual)));
153+
_analytics.Add(_plugin.Object);
154+
_analytics.Group("user123", "foo", null);
155+
156+
Assert.NotEmpty(actual);
157+
Assert.False(actual[0].Traits.ContainsKey("userId"));
158+
Assert.Equal("user123", actual[0].UserId);
159+
Assert.Equal("foo", actual[0].GroupId);
160+
}
161+
162+
[Fact]
163+
public void TestCompatGroupAcceptDictWithNulls()
164+
{
165+
var properties = new Dictionary<string, object>
166+
{
167+
["nullValue"] = null
168+
};
169+
var actual = new List<GroupEvent>();
170+
_plugin.Setup(o => o.Group(Capture.In(actual)));
171+
_analytics.Add(_plugin.Object);
172+
_analytics.Group("user123", "foo", properties);
173+
174+
Assert.NotEmpty(actual);
175+
Assert.False(actual[0].Traits.ContainsKey("userId"));
176+
Assert.Equal("user123", actual[0].UserId);
177+
Assert.Equal("foo", actual[0].GroupId);
178+
Assert.True(actual[0].Traits.ContainsKey("nullValue"));
179+
Assert.Equal(JsonNull.Instance, actual[0].Traits["nullValue"]);
180+
}
181+
}
182+
}

Tests/Utils/Stubs.cs

+5
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,11 @@ public class StubEventPlugin : EventPlugin
4545
public override PluginType Type => PluginType.Before;
4646
}
4747

48+
public class StubAfterEventPlugin : EventPlugin
49+
{
50+
public override PluginType Type => PluginType.After;
51+
}
52+
4853
public class StubDestinationPlugin : DestinationPlugin
4954
{
5055
public override string Key { get; }

0 commit comments

Comments
 (0)