Skip to content

Commit d8aa727

Browse files
Kroach/oasis 1806 null (#19)
* Fix CS1902: Invalid option 'portable' and MSB3245: Could not resolve System.Web.Entity for Mac VS Community edition * Add FilterNullValues * Add #region's to OptimizelyTest.cs * Add TestActivateWithNullAttributes * Moved TestInvalidDispatchImpressionEvent into different #region * OASIS-1806 [C#] Prune null/nil/None value attributes in track activate Summary: [C#] Prune null/nil/None value attributes in track activate Remove null attribute and eventTag values inputted to activate and track. JSON's sent to OPTIMIZELY.COM should not contain null's. Fix CS1902: Invalid option 'portable' and MSB3245: Could not resolve System.Web.Entity for Mac VS Community edition Add FilterNullValues Add #region's to OptimizelyTest.cs Add TestActivateWithNullAttributes Moved TestInvalidDispatchImpressionEvent into different #region Add TestTrackWithNullAttributesWithNullEventValue Test Plan: "Unit Tests" / "Run All" Passed: 92 Failed: 0 incluing our 2 new tests Add'ed above. Reviewers: mike.ng, matthew.auerbach JIRA Issues: OASIS-1806 Differential Revision: https://phabricator.optimizely.com/D17725
1 parent d8aa047 commit d8aa727

File tree

6 files changed

+148
-9
lines changed

6 files changed

+148
-9
lines changed

OptimizelySDK.DemoApp/OptimizelySDK.DemoApp.csproj

+8-4
Original file line numberDiff line numberDiff line change
@@ -28,8 +28,8 @@
2828
<TargetFrameworkProfile />
2929
</PropertyGroup>
3030
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">
31-
<DebugSymbols>true</DebugSymbols>
32-
<DebugType>full</DebugType>
31+
<DebugSymbols>false</DebugSymbols>
32+
<DebugType></DebugType>
3333
<Optimize>false</Optimize>
3434
<OutputPath>bin\</OutputPath>
3535
<DefineConstants>DEBUG;TRACE</DefineConstants>
@@ -71,7 +71,6 @@
7171
<Reference Include="System.Data.DataSetExtensions" />
7272
<Reference Include="System.Drawing" />
7373
<Reference Include="System.Web.DynamicData" />
74-
<Reference Include="System.Web.Entity" />
7574
<Reference Include="System.Web.ApplicationServices" />
7675
<Reference Include="System.ComponentModel.DataAnnotations" />
7776
<Reference Include="System.Web.Extensions" />
@@ -222,6 +221,11 @@
222221
</WebProjectProperties>
223222
</FlavorProperties>
224223
</VisualStudio>
224+
<MonoDevelop>
225+
<Properties>
226+
<XspParameters Port="8080" Address="127.0.0.1" SslMode="None" SslProtocol="Default" KeyType="None" CertFile="" KeyFile="" PasswordOptions="None" Password="" Verbose="True" />
227+
</Properties>
228+
</MonoDevelop>
225229
</ProjectExtensions>
226230
<Target Name="EnsureNuGetPackageBuildImports" BeforeTargets="PrepareForBuild">
227231
<PropertyGroup>
@@ -236,4 +240,4 @@
236240
</Target>
237241
<Target Name="AfterBuild">
238242
</Target> -->
239-
</Project>
243+
</Project>

OptimizelySDK.DemoApp/Web.config

+5-1
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,11 @@
2323
</system.Web>
2424
-->
2525
<system.web>
26-
<compilation debug="true" targetFramework="4.5" />
26+
<compilation debug="true" targetFramework="4.5">
27+
<assemblies>
28+
<add assembly="System.Net.Http.WebRequest, Version=4.0.0.0, Culture=neutral, PublicKeyToken=b03f5f7f11d50a3a" />
29+
</assemblies>
30+
</compilation>
2731
<httpRuntime targetFramework="4.5" />
2832
</system.web>
2933
<runtime>

OptimizelySDK.Tests/OptimizelyTest.cs

+94-3
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ public class OptimizelyTest
4141
private const string TestUserId = "testUserId";
4242
private OptimizelyHelper Helper;
4343

44+
#region Test Life Cycle
4445
[SetUp]
4546
public void Initialize()
4647
{
@@ -83,7 +84,9 @@ public void Cleanup()
8384
Config = null;
8485
EventBuilderMock = null;
8586
}
86-
87+
#endregion
88+
89+
#region OptimizelyHelper
8790
private class OptimizelyHelper
8891
{
8992
static Type[] ParameterTypes = new[]
@@ -107,6 +110,18 @@ private class OptimizelyHelper
107110
{ "location", "San Francisco" }
108111
};
109112

113+
// NullUserAttributes extends copy of UserAttributes with key-value
114+
// pairs containing null values which should not be sent to OPTIMIZELY.COM .
115+
public static UserAttributes NullUserAttributes = new UserAttributes
116+
{
117+
{ "device_type", "iPhone" },
118+
{ "company", "Optimizely" },
119+
{ "location", "San Francisco" },
120+
{ "null_value", null},
121+
{ "wont_be_sent", null},
122+
{ "bad_food", null}
123+
};
124+
110125
public string Datafile { get; set; }
111126
public IEventDispatcher EventDispatcher { get; set; }
112127
public ILogger Logger { get; set; }
@@ -128,7 +143,9 @@ public PrivateObject CreatePrivateOptimizely()
128143
});
129144
}
130145
}
146+
#endregion
131147

148+
#region Test Validate
132149
[Test]
133150
public void TestValidateInputsInvalidFileJsonValidationNotSkipped()
134151
{
@@ -293,7 +310,9 @@ public void TestActivateNoAudienceNoAttributes()
293310

294311
Assert.AreEqual("group_exp_1_var_2", variationkey);
295312
}
313+
#endregion
296314

315+
#region Test Activate
297316
[Test]
298317
public void TestActivateAudienceNoAttributes()
299318
{
@@ -336,6 +355,33 @@ public void TestActivateWithAttributes()
336355
Assert.AreEqual("control", variationkey);
337356
}
338357

358+
[Test]
359+
public void TestActivateWithNullAttributes()
360+
{
361+
EventBuilderMock.Setup(b => b.CreateImpressionEvent(It.IsAny<ProjectConfig>(), It.IsAny<Experiment>(),
362+
It.IsAny<string>(), It.IsAny<string>(), It.IsAny<UserAttributes>()))
363+
.Returns(new LogEvent("logx.optimizely.com/decision", OptimizelyHelper.SingleParameter, "POST", new Dictionary<string, string> { }));
364+
365+
var optly = Helper.CreatePrivateOptimizely();
366+
optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object);
367+
368+
var variationkey = optly.Invoke("Activate", "test_experiment", "test_user", OptimizelyHelper.NullUserAttributes);
369+
370+
EventBuilderMock.Verify(b => b.CreateImpressionEvent(It.IsAny<ProjectConfig>(), Config.GetExperimentFromKey("test_experiment"),
371+
"7722370027", "test_user", OptimizelyHelper.UserAttributes), Times.Once);
372+
373+
LoggerMock.Verify(l => l.Log(It.IsAny<LogLevel>(), It.IsAny<string>()), Times.Exactly(8));
374+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [3037] to user [test_user]"), Times.Once);
375+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is in variation [control] of experiment [test_experiment]."), Times.Once);
376+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key null_value removed and will not be sent to results."), Times.Once);
377+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key wont_be_sent removed and will not be sent to results."), Times.Once);
378+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key bad_food removed and will not be sent to results."), Times.Once);
379+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Activating user test_user in experiment test_experiment."), Times.Once);
380+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Dispatching impression event to URL OptimizelySDK.Event.LogEvent with params {\"param1\":\"val1\"}."), Times.Once);
381+
382+
Assert.AreEqual("control", variationkey);
383+
}
384+
339385
[Test]
340386
public void TestActivateExperimentNotRunning()
341387
{
@@ -353,7 +399,9 @@ public void TestActivateExperimentNotRunning()
353399

354400
Assert.IsNull(variationkey);
355401
}
402+
#endregion
356403

404+
#region Test GetVariation
357405
[Test]
358406
public void TestGetVariationInvalidOptimizelyObject()
359407
{
@@ -379,7 +427,6 @@ public void TestGetVariationInvalidAttributes()
379427
380428
Assert.IsNull(result);
381429
} */
382-
383430
[Test]
384431
public void TestGetVariationAudienceMatch()
385432
{
@@ -423,7 +470,9 @@ public void TestTrackInvalidOptimizelyObject()
423470
LoggerMock.Verify(l => l.Log(It.IsAny<LogLevel>(), It.IsAny<string>()), Times.Exactly(2));
424471
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Datafile has invalid format. Failing 'track'."), Times.Once);
425472
}
473+
#endregion
426474

475+
#region Test Track
427476
[Test]
428477
public void TestTrackInvalidAttributes()
429478
{
@@ -528,6 +577,45 @@ public void TestTrackWithAttributesWithEventValue()
528577
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Dispatching conversion event to URL logx.optimizely.com/track with params {\"param1\":\"val1\"}."), Times.Once);
529578
}
530579

580+
[Test]
581+
public void TestTrackWithNullAttributesWithNullEventValue()
582+
{
583+
EventBuilderMock.Setup(b => b.CreateConversionEvent(It.IsAny<ProjectConfig>(), It.IsAny<string>(),
584+
It.IsAny<IEnumerable<Experiment>>(), It.IsAny<string>(), It.IsAny<UserAttributes>(), It.IsAny<EventTags>()))
585+
.Returns(new LogEvent("logx.optimizely.com/track", OptimizelyHelper.SingleParameter,
586+
"POST", new Dictionary<string, string> { }));
587+
588+
var optly = Helper.CreatePrivateOptimizely();
589+
optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object);
590+
optly.Invoke("Track", "purchase", "test_user", OptimizelyHelper.NullUserAttributes, new EventTags
591+
{
592+
{ "revenue", 42 },
593+
{ "wont_send_null", null}
594+
});
595+
596+
LoggerMock.Verify(l => l.Log(It.IsAny<LogLevel>(), It.IsAny<string>()), Times.Exactly(18));
597+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [3037] to user [test_user]"));
598+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is in variation [control] of experiment [test_experiment]."));
599+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "This decision will not be saved since the UserProfileService is null."));
600+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [4517] to user [test_user]"));
601+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is not in experiment [group_experiment_1] of group [7722400015]."));
602+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [4517] to user [test_user]"));
603+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is in experiment [group_experiment_2] of group [7722400015]."));
604+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [9871] to user [test_user]"));
605+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is in variation [group_exp_2_var_2] of experiment [group_experiment_2]."));
606+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "This decision will not be saved since the UserProfileService is null."));
607+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Experiment \"paused_experiment\" is not running."));
608+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Not tracking user \"test_user\" for experiment \"paused_experiment\""));
609+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key null_value removed and will not be sent to results."));
610+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key wont_be_sent removed and will not be sent to results."));
611+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[UserAttributes] Null value for key bad_food removed and will not be sent to results."));
612+
LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "[EventTags] Null value for key wont_send_null removed and will not be sent to results."));
613+
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Tracking event purchase for user test_user."));
614+
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Dispatching conversion event to URL logx.optimizely.com/track with params {\"param1\":\"val1\"}."));
615+
}
616+
#endregion
617+
618+
#region Test Invalid Dispatch
531619
[Test]
532620
public void TestInvalidDispatchImpressionEvent()
533621
{
@@ -571,7 +659,9 @@ public void TestInvalidDispatchConversionEvent()
571659
LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Tracking event purchase for user test_user."), Times.Once);
572660
LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Dispatching conversion event to URL logx.optimizely.com/track with params {\"param1\":\"val1\"}."), Times.Once);
573661
}
662+
#endregion
574663

664+
#region Test Misc
575665
/* Start 1 */
576666
public void TestTrackNoAttributesWithInvalidEventValue()
577667
{
@@ -714,5 +804,6 @@ public void TestTrackNoAttributesWithDeprecatedEventValue()
714804
}
715805

716806
/* End */
807+
#endregion
808+
}
717809
}
718-
}

OptimizelySDK/Entity/EventAttributes.cs

+12
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,22 @@
1414
* limitations under the License.
1515
*/
1616
using System.Collections.Generic;
17+
using OptimizelySDK.Logger;
1718

1819
namespace OptimizelySDK.Entity
1920
{
2021
public class EventTags : Dictionary<string, object>
2122
{
23+
public EventTags FilterNullValues(ILogger logger) {
24+
EventTags answer = new EventTags();
25+
foreach (KeyValuePair<string, object> pair in this) {
26+
if (pair.Value != null) {
27+
answer[pair.Key] = pair.Value;
28+
} else {
29+
logger.Log(LogLevel.ERROR, string.Format("[EventTags] Null value for key {0} removed and will not be sent to results.", pair.Key));
30+
}
31+
}
32+
return answer;
33+
}
2234
}
2335
}

OptimizelySDK/Entity/UserAttributes.cs

+13
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,23 @@
1414
* limitations under the License.
1515
*/
1616
using System.Collections.Generic;
17+
using OptimizelySDK.Logger;
1718

1819
namespace OptimizelySDK.Entity
1920
{
2021
public class UserAttributes : Dictionary<string, string>
2122
{
23+
public UserAttributes FilterNullValues(ILogger logger)
24+
{
25+
UserAttributes answer = new UserAttributes();
26+
foreach (KeyValuePair<string, string> pair in this) {
27+
if (pair.Value != null) {
28+
answer[pair.Key] = pair.Value;
29+
} else {
30+
logger.Log(LogLevel.ERROR, string.Format("[UserAttributes] Null value for key {0} removed and will not be sent to results.", pair.Key));
31+
}
32+
}
33+
return answer;
34+
}
2235
}
2336
}

OptimizelySDK/Optimizely.cs

+16-1
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,10 @@ public string Activate(string experimentKey, string userId, UserAttributes userA
149149
return null;
150150
}
151151

152+
if (userAttributes != null) {
153+
userAttributes = userAttributes.FilterNullValues(Logger);
154+
}
155+
152156
var impressionEvent = EventBuilder.CreateImpressionEvent(Config, experiment, variation.Id, userId, userAttributes);
153157
Logger.Log(LogLevel.INFO, string.Format("Activating user {0} in experiment {1}.", userId, experimentKey));
154158
Logger.Log(LogLevel.DEBUG, string.Format("Dispatching impression event to URL {0} with params {1}.",
@@ -221,6 +225,17 @@ public void Track(string eventKey, string userId, UserAttributes userAttributes
221225

222226
if (validExperiments.Count > 0)
223227
{
228+
229+
if (userAttributes != null)
230+
{
231+
userAttributes = userAttributes.FilterNullValues(Logger);
232+
}
233+
234+
if (eventTags != null)
235+
{
236+
eventTags = eventTags.FilterNullValues(Logger);
237+
}
238+
224239
var conversionEvent = EventBuilder.CreateConversionEvent(Config, eventKey, validExperiments,
225240
userId, userAttributes, eventTags);
226241
Logger.Log(LogLevel.INFO, string.Format("Tracking event {0} for user {1}.", eventKey, userId));
@@ -267,4 +282,4 @@ public string GetVariation(string experimentKey, string userId, UserAttributes u
267282
return variation == null ? null : variation.Key;
268283
}
269284
}
270-
}
285+
}

0 commit comments

Comments
 (0)