From 6e9f82860a39fc04a4fb14d75d1d1ed6e90873b8 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Fri, 7 Sep 2018 22:15:47 +0500 Subject: [PATCH 1/4] Added attribute types support. --- OptimizelySDK.Tests/ProjectConfigTest.cs | 5 ++++- OptimizelySDK.Tests/TestData.json | 12 ++++++++++++ OptimizelySDK/Bucketing/DecisionService.cs | 2 +- OptimizelySDK/Entity/UserAttributes.cs | 4 ++-- OptimizelySDK/Utils/ConditionEvaluator.cs | 2 +- 5 files changed, 20 insertions(+), 5 deletions(-) diff --git a/OptimizelySDK.Tests/ProjectConfigTest.cs b/OptimizelySDK.Tests/ProjectConfigTest.cs index f2b4381e..3b697869 100644 --- a/OptimizelySDK.Tests/ProjectConfigTest.cs +++ b/OptimizelySDK.Tests/ProjectConfigTest.cs @@ -117,7 +117,10 @@ public void TestInit() { { "device_type", Config.GetAttribute("device_type") }, { "location", Config.GetAttribute("location")}, - { "browser_type", Config.GetAttribute("browser_type")} + { "browser_type", Config.GetAttribute("browser_type")}, + { "boolean_key", Config.GetAttribute("boolean_key")}, + { "integer_key", Config.GetAttribute("integer_key")}, + { "double_key", Config.GetAttribute("double_key")} }; Assert.IsTrue(TestData.CompareObjects(attributeKeyMap, Config.AttributeKeyMap)); diff --git a/OptimizelySDK.Tests/TestData.json b/OptimizelySDK.Tests/TestData.json index 04c86ffd..f7615fbf 100644 --- a/OptimizelySDK.Tests/TestData.json +++ b/OptimizelySDK.Tests/TestData.json @@ -785,6 +785,18 @@ { "id": "134", "key": "browser_type" + }, + { + "id": "323434545", + "key": "boolean_key" + }, + { + "id": "616727838", + "key": "integer_key" + }, + { + "id": "808797686", + "key": "double_key" } ], "projectId": "7720880029", diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 29ee69e1..0bfef54a 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -401,7 +401,7 @@ private string GetBucketingId(string userId, UserAttributes filteredAttributes) // If the bucketing ID key is defined in attributes, then use that in place of the userID for the murmur hash key if (filteredAttributes != null && filteredAttributes.ContainsKey(ControlAttributes.BUCKETING_ID_ATTRIBUTE)) { - bucketingId = filteredAttributes[ControlAttributes.BUCKETING_ID_ATTRIBUTE]; + bucketingId = filteredAttributes[ControlAttributes.BUCKETING_ID_ATTRIBUTE].ToString(); Logger.Log(LogLevel.DEBUG, string.Format("Setting the bucketing ID to \"{0}\"", bucketingId)); } diff --git a/OptimizelySDK/Entity/UserAttributes.cs b/OptimizelySDK/Entity/UserAttributes.cs index fcfdc3c3..ae3233da 100644 --- a/OptimizelySDK/Entity/UserAttributes.cs +++ b/OptimizelySDK/Entity/UserAttributes.cs @@ -18,12 +18,12 @@ namespace OptimizelySDK.Entity { - public class UserAttributes : Dictionary + public class UserAttributes : Dictionary { public UserAttributes FilterNullValues(ILogger logger) { UserAttributes answer = new UserAttributes(); - foreach (KeyValuePair pair in this) { + foreach (KeyValuePair pair in this) { if (pair.Value != null) { answer[pair.Key] = pair.Value; } else { diff --git a/OptimizelySDK/Utils/ConditionEvaluator.cs b/OptimizelySDK/Utils/ConditionEvaluator.cs index a3327b44..604f91dd 100644 --- a/OptimizelySDK/Utils/ConditionEvaluator.cs +++ b/OptimizelySDK/Utils/ConditionEvaluator.cs @@ -71,7 +71,7 @@ public bool Evaluate(JToken conditions, UserAttributes userAttributes) string conditionName = conditions["name"].ToString(); return userAttributes != null && userAttributes.ContainsKey(conditionName) - && userAttributes[conditionName] == conditions["value"].ToString(); + && userAttributes[conditionName].ToString() == conditions["value"].ToString(); } public bool Evaluate(object[] conditions, UserAttributes userAttributes) From 233995fc3f9d4d2d59b2ac09e19000f28b0b8d1d Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Wed, 12 Sep 2018 20:36:13 +0500 Subject: [PATCH 2/4] Fixed code review defects. --- OptimizelySDK.Tests/OptimizelyTest.cs | 33 +++++++++++++++++++ .../UtilsTests/ConditionEvaluatorTest.cs | 15 +++++++++ OptimizelySDK/Bucketing/DecisionService.cs | 13 ++++++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index bab40b22..5af4a255 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -458,6 +458,39 @@ public void TestActivateExperimentNotRunning() Assert.IsNull(variationkey); } + + [Test] + public void TestActivateWithTypedAttributes() + { + var userAttributes = new UserAttributes + { + {"device_type", "iPhone" }, + {"location", "San Francisco" }, + {"is_firefox", false }, + {"num_users", 15 }, + {"pi_value", 3.14 } + }; + + EventBuilderMock.Setup(b => b.CreateImpressionEvent(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(new LogEvent("logx.optimizely.com/decision", OptimizelyHelper.SingleParameter, "POST", new Dictionary { })); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); + + var variation = (Variation)optly.Invoke("Activate", "test_experiment", "test_user", userAttributes); + + EventBuilderMock.Verify(b => b.CreateImpressionEvent(It.IsAny(), Config.GetExperimentFromKey("test_experiment"), + "7722370027", "test_user", userAttributes), Times.Once); + + LoggerMock.Verify(l => l.Log(It.IsAny(), It.IsAny()), Times.Exactly(6)); + LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, "Assigned bucket [3037] to user [test_user] with bucketing ID [test_user]."), Times.Once); + LoggerMock.Verify(l => l.Log(LogLevel.INFO, "User [test_user] is in variation [control] of experiment [test_experiment]."), Times.Once); + LoggerMock.Verify(l => l.Log(LogLevel.INFO, "Activating user test_user in experiment test_experiment."), Times.Once); + LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, @"Dispatching impression event to URL logx.optimizely.com/decision with params {""param1"":""val1""}."), Times.Once); + + Assert.IsTrue(TestData.CompareObjects(VariationWithKeyControl, variation)); + } #endregion #region Test GetVariation diff --git a/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs b/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs index ddf9e0ad..45aa67f3 100644 --- a/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs +++ b/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs @@ -83,5 +83,20 @@ public void TestEvaluateNullUserAttributes() Assert.IsFalse(ConditionEvaluator.Evaluate(Conditions, userAttributes)); } + [Test] + public void TestTypedUserAttributesEvaluateTrue() + { + var userAttributes = new UserAttributes + { + {"device_type", "iPhone" }, + {"is_firefox", false }, + {"num_users", 15 }, + {"pi_value", 3.14 } + }; + + string typedConditionStr = @"[""and"", [""or"", [""or"", {""name"": ""device_type"", ""type"": ""custom_attribute"", ""value"": ""iPhone""}]], [""or"", [""or"", {""name"": ""is_firefox"", ""type"": ""custom_attribute"", ""value"": false}]], [""or"", [""or"", {""name"": ""num_users"", ""type"": ""custom_attribute"", ""value"": 15}]], [""or"", [""or"", {""name"": ""pi_value"", ""type"": ""custom_attribute"", ""value"": 3.14}]]]"; + var typedConditions = Newtonsoft.Json.JsonConvert.DeserializeObject(typedConditionStr); + Assert.IsTrue(ConditionEvaluator.Evaluate(typedConditions, userAttributes)); + } } } \ No newline at end of file diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 0bfef54a..68d28f74 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -393,7 +393,7 @@ public virtual FeatureDecision GetVariationForFeature(FeatureFlag featureFlag, s /// /// User Identifier /// The user's attributes. - /// User ID if bucketing Id is not provided in user attributes, bucketing Id otherwise. + /// Bucketing Id if it is a string type in attributes, user Id otherwise. private string GetBucketingId(string userId, UserAttributes filteredAttributes) { string bucketingId = userId; @@ -401,8 +401,15 @@ private string GetBucketingId(string userId, UserAttributes filteredAttributes) // If the bucketing ID key is defined in attributes, then use that in place of the userID for the murmur hash key if (filteredAttributes != null && filteredAttributes.ContainsKey(ControlAttributes.BUCKETING_ID_ATTRIBUTE)) { - bucketingId = filteredAttributes[ControlAttributes.BUCKETING_ID_ATTRIBUTE].ToString(); - Logger.Log(LogLevel.DEBUG, string.Format("Setting the bucketing ID to \"{0}\"", bucketingId)); + if (filteredAttributes[ControlAttributes.BUCKETING_ID_ATTRIBUTE] is string) + { + bucketingId = (string)filteredAttributes[ControlAttributes.BUCKETING_ID_ATTRIBUTE]; + Logger.Log(LogLevel.DEBUG, $"BucketingId is valid: \"{bucketingId}\""); + } + else + { + Logger.Log(LogLevel.WARN, "BucketingID attribute is not a string. Defaulted to userId"); + } } return bucketingId; From 4be9eb6e49e7b1c973f7af4de255514e4a486aa6 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Thu, 13 Sep 2018 17:22:04 +0500 Subject: [PATCH 3/4] Add event builder and decision service tests for attribute types. --- OptimizelySDK.Tests/DecisionServiceTest.cs | 14 +++ .../EventTests/EventBuilderTest.cs | 113 ++++++++++++++++++ OptimizelySDK.Tests/OptimizelyTest.cs | 13 +- OptimizelySDK/Utils/ConditionEvaluator.cs | 28 ++++- 4 files changed, 159 insertions(+), 9 deletions(-) diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index bc9c67d7..bf58cbc4 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -392,6 +392,14 @@ public void TestGetVariationWithBucketingId() {ControlAttributes.BUCKETING_ID_ATTRIBUTE, testBucketingIdVariation} }; + var userAttributesWithInvalidBucketingId = new UserAttributes + { + {"device_type", "iPhone"}, + {"company", "Optimizely"}, + {"location", "San Francisco"}, + {ControlAttributes.BUCKETING_ID_ATTRIBUTE, 1.59} + }; + var invalidUserAttributesWithBucketingId = new UserAttributes { {"company", "Optimizely"}, @@ -407,6 +415,12 @@ public void TestGetVariationWithBucketingId() // confirm valid bucketing with bucketing ID set in attributes actualVariation = optlyObject.GetVariation(experimentKey, userId, userAttributesWithBucketingId); Assert.IsTrue(TestData.CompareObjects(VariationWithKeyVariation, actualVariation)); + LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, $"BucketingId is valid: \"{testBucketingIdVariation}\"")); + + // check audience with invalid bucketing Id + actualVariation = optlyObject.GetVariation(experimentKey, userId, userAttributesWithInvalidBucketingId); + Assert.IsTrue(TestData.CompareObjects(VariationWithKeyControl, actualVariation)); + LoggerMock.Verify(l => l.Log(LogLevel.WARN, "BucketingID attribute is not a string. Defaulted to userId")); // check invalid audience with bucketing ID actualVariation = optlyObject.GetVariation(experimentKey, userId, invalidUserAttributesWithBucketingId); diff --git a/OptimizelySDK.Tests/EventTests/EventBuilderTest.cs b/OptimizelySDK.Tests/EventTests/EventBuilderTest.cs index 35435eec..e25a0ff6 100644 --- a/OptimizelySDK.Tests/EventTests/EventBuilderTest.cs +++ b/OptimizelySDK.Tests/EventTests/EventBuilderTest.cs @@ -215,6 +215,119 @@ public void TestCreateImpressionEventWithAttributes() Assert.IsTrue(TestData.CompareObjects(expectedLogEvent, logEvent)); } + [Test] + public void TestCreateImpressionEventWithTypedAttributes() + { + var guid = Guid.NewGuid(); + var timeStamp = TestData.SecondsSince1970(); + + var payloadParams = new Dictionary + { + { "visitors", new object[] + { + new Dictionary() + { + { "snapshots", new object[] + { + new Dictionary + { + { "decisions", new object[] + { + new Dictionary + { + {"campaign_id", "7719770039" }, + {"experiment_id", "7716830082" }, + {"variation_id", "7722370027" } + } + } + }, + { "events", new object[] + { + new Dictionary + { + {"entity_id", "7719770039" }, + {"timestamp", timeStamp }, + {"uuid", guid }, + {"key", "campaign_activated" } + } + } + } + } + } + }, + {"attributes", new object[] + { + new Dictionary + { + {"entity_id", "7723280020" }, + {"key", "device_type" }, + {"type", "custom" }, + {"value", "iPhone"} + }, + new Dictionary + { + {"entity_id", "323434545" }, + {"key", "boolean_key" }, + {"type", "custom" }, + {"value", true} + }, + new Dictionary + { + {"entity_id", "616727838" }, + {"key", "integer_key" }, + {"type", "custom" }, + {"value", 15} + }, + new Dictionary + { + {"entity_id", "808797686" }, + {"key", "double_key" }, + {"type", "custom" }, + {"value", 3.14} + }, + new Dictionary + { + {"entity_id", ControlAttributes.BOT_FILTERING_ATTRIBUTE}, + {"key", ControlAttributes.BOT_FILTERING_ATTRIBUTE}, + {"type", "custom" }, + {"value", true } + } + } + }, + { "visitor_id", TestUserId } + } + } + }, + {"project_id", "7720880029" }, + {"account_id", "1592310167" }, + {"client_name", "csharp-sdk" }, + {"client_version", Optimizely.SDK_VERSION }, + {"revision", "15" }, + {"anonymize_ip", false} + }; + + var expectedLogEvent = new LogEvent("https://logx.optimizely.com/v1/events", + payloadParams, + "POST", + new Dictionary + { + { "Content-Type", "application/json" } + }); + + var userAttributes = new UserAttributes + { + {"device_type", "iPhone" }, + {"boolean_key", true }, + {"integer_key", 15 }, + {"double_key", 3.14 } + }; + + var logEvent = EventBuilder.CreateImpressionEvent(Config, Config.GetExperimentFromKey("test_experiment"), "7722370027", TestUserId, userAttributes); + TestData.ChangeGUIDAndTimeStamp(logEvent.Params, timeStamp, guid); + + Assert.IsTrue(TestData.CompareObjects(expectedLogEvent, logEvent)); + } + [Test] public void TestCreateConversionEventNoAttributesNoValue() { diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index 5af4a255..14a17a57 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -42,7 +42,6 @@ public class OptimizelyTest private Mock ErrorHandlerMock; private Mock EventDispatcherMock; private Optimizely Optimizely; - private IEventDispatcher EventDispatcher; private const string TestUserId = "testUserId"; private OptimizelyHelper Helper; private Mock OptimizelyMock; @@ -466,15 +465,15 @@ public void TestActivateWithTypedAttributes() { {"device_type", "iPhone" }, {"location", "San Francisco" }, - {"is_firefox", false }, - {"num_users", 15 }, - {"pi_value", 3.14 } + {"boolean_key", true }, + {"integer_key", 15 }, + {"double_key", 3.14 } }; EventBuilderMock.Setup(b => b.CreateImpressionEvent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())) .Returns(new LogEvent("logx.optimizely.com/decision", OptimizelyHelper.SingleParameter, "POST", new Dictionary { })); - + var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); @@ -798,7 +797,7 @@ public void TestTrackNoAttributesWithDeprecatedEventValue() [Test] public void TestForcedVariationPreceedsWhitelistedVariation() { - var optimizely = new Optimizely(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object); var projectConfig = ProjectConfig.Create(TestData.Datafile, LoggerMock.Object, ErrorHandlerMock.Object); Variation expectedVariation1 = projectConfig.GetVariationFromKey("etag3", "vtag5"); Variation expectedVariation2 = projectConfig.GetVariationFromKey("etag3", "vtag6"); @@ -846,7 +845,7 @@ public void TestForcedVariationPreceedsUserProfile() userProfileServiceMock.Setup(_ => _.Lookup(userId)).Returns(userProfile.ToMap()); - var optimizely = new Optimizely(TestData.Datafile, EventDispatcher, LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); + var optimizely = new Optimizely(TestData.Datafile, EventDispatcherMock.Object, LoggerMock.Object, ErrorHandlerMock.Object, userProfileServiceMock.Object); var projectConfig = ProjectConfig.Create(TestData.Datafile, LoggerMock.Object, ErrorHandlerMock.Object); Variation expectedFbVariation = projectConfig.GetVariationFromKey(experimentKey, fbVariationKey); Variation expectedVariation = projectConfig.GetVariationFromKey(experimentKey, variationKey); diff --git a/OptimizelySDK/Utils/ConditionEvaluator.cs b/OptimizelySDK/Utils/ConditionEvaluator.cs index 604f91dd..3185d91f 100644 --- a/OptimizelySDK/Utils/ConditionEvaluator.cs +++ b/OptimizelySDK/Utils/ConditionEvaluator.cs @@ -16,6 +16,7 @@ using Newtonsoft.Json; using Newtonsoft.Json.Linq; using OptimizelySDK.Entity; +using System; using System.Linq; namespace OptimizelySDK.Utils @@ -70,8 +71,7 @@ public bool Evaluate(JToken conditions, UserAttributes userAttributes) } string conditionName = conditions["name"].ToString(); - return userAttributes != null && userAttributes.ContainsKey(conditionName) - && userAttributes[conditionName].ToString() == conditions["value"].ToString(); + return userAttributes != null && userAttributes.ContainsKey(conditionName) && CompareValues(userAttributes[conditionName], conditions["value"]); } public bool Evaluate(object[] conditions, UserAttributes userAttributes) @@ -90,5 +90,29 @@ public static JToken DecodeConditions(string conditions) { return JToken.Parse(conditions); } + + private bool CompareValues(object attribute, JToken condition) + { + try + { + switch (condition.Type) + { + case JTokenType.Integer: + return (int)condition == Convert.ToInt32(attribute); + case JTokenType.Float: + return (double)condition == Convert.ToDouble(attribute); + case JTokenType.String: + return (string)condition == Convert.ToString(attribute); + case JTokenType.Boolean: + return (bool)condition == Convert.ToBoolean(attribute); + default: + return false; + } + } + catch (Exception) + { + return false; + } + } } } \ No newline at end of file From acad96a8c707f68fc95f761ffad46c2e374b4ab1 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Thu, 13 Sep 2018 19:02:58 +0500 Subject: [PATCH 4/4] Update headers. --- OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs | 2 +- OptimizelySDK/Entity/UserAttributes.cs | 2 +- OptimizelySDK/Utils/ConditionEvaluator.cs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs b/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs index 45aa67f3..baa119c6 100644 --- a/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs +++ b/OptimizelySDK.Tests/UtilsTests/ConditionEvaluatorTest.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017, Optimizely + * Copyright 2017-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK/Entity/UserAttributes.cs b/OptimizelySDK/Entity/UserAttributes.cs index ae3233da..e189cfe1 100644 --- a/OptimizelySDK/Entity/UserAttributes.cs +++ b/OptimizelySDK/Entity/UserAttributes.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017, Optimizely + * Copyright 2017-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. diff --git a/OptimizelySDK/Utils/ConditionEvaluator.cs b/OptimizelySDK/Utils/ConditionEvaluator.cs index 3185d91f..220d715f 100644 --- a/OptimizelySDK/Utils/ConditionEvaluator.cs +++ b/OptimizelySDK/Utils/ConditionEvaluator.cs @@ -1,5 +1,5 @@ /* - * Copyright 2017, Optimizely + * Copyright 2017-2018, Optimizely * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License.