From d1a949f7b81f72a125025251b4a1f38231ca664c Mon Sep 17 00:00:00 2001 From: Muhammad Fahad Ahmed Date: Wed, 15 Nov 2017 19:42:28 +0500 Subject: [PATCH 1/7] Feature Notification Center (#25) Feature Notification Center --- .../OptimizelySDK.Net35.csproj | 3 + .../OptimizelySDK.Net40.csproj | 3 + .../OptimizelySDK.NetStandard16.csproj | 1 + .../NotificationCenterTests.cs | 224 +++++++++++++++ .../OptimizelySDK.Tests.csproj | 1 + OptimizelySDK.Tests/OptimizelyTest.cs | 236 +++++++++++++++ .../Notifications/NotificationCenter.cs | 268 ++++++++++++++++++ OptimizelySDK/Optimizely.cs | 23 +- OptimizelySDK/OptimizelySDK.csproj | 3 +- 9 files changed, 755 insertions(+), 7 deletions(-) create mode 100644 OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs create mode 100644 OptimizelySDK/Notifications/NotificationCenter.cs diff --git a/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj b/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj index 0b479154..326fa803 100644 --- a/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj +++ b/OptimizelySDK.Net35/OptimizelySDK.Net35.csproj @@ -119,6 +119,9 @@ Logger\NoOpLogger.cs + + + Notifications\NotificationCenter.cs Optimizely.cs diff --git a/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj b/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj index 2adbe146..3b1290d0 100644 --- a/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj +++ b/OptimizelySDK.Net40/OptimizelySDK.Net40.csproj @@ -120,6 +120,9 @@ Logger\NoOpLogger.cs + + + Notifications\NotificationCenter.cs Optimizely.cs diff --git a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj index 101e9828..73917fc5 100644 --- a/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj +++ b/OptimizelySDK.NetStandard16/OptimizelySDK.NetStandard16.csproj @@ -38,6 +38,7 @@ + diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs new file mode 100644 index 00000000..5ed6c0cf --- /dev/null +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -0,0 +1,224 @@ +/* + * Copyright 2017, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using Moq; +using NUnit.Framework; +using OptimizelySDK.Entity; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Event; +using OptimizelySDK.Logger; +using OptimizelySDK.Notifications; + +namespace OptimizelySDK.Tests.NotificationTests +{ + public class NotificationCenterTests + { + private Mock LoggerMock; + private NotificationCenter NotificationCenter; + private TestNotificationCallbacks TestNotificationCallbacks; + private NotificationCenter.NotificationType NotificationTypeDecision = NotificationCenter.NotificationType.Decision; + private NotificationCenter.NotificationType NotificationTypeTrack = NotificationCenter.NotificationType.Track; + private NotificationCenter.NotificationType NotificationTypeFeatureAccess = NotificationCenter.NotificationType.FeatureAccess; + + [SetUp] + public void Setup() + { + LoggerMock = new Mock(); + LoggerMock.Setup(i => i.Log(It.IsAny(), It.IsAny())); + + NotificationCenter = new NotificationCenter(LoggerMock.Object); + TestNotificationCallbacks = new TestNotificationCallbacks(); + } + + [Test] + public void TestAddAndRemoveNotificationListener() + { + // Verify that callback added successfully. + Assert.AreEqual(1, NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback)); + Assert.AreEqual(1, NotificationCenter.NotificationsCount); + + // Verify that callback removed successfully. + Assert.AreEqual(true, NotificationCenter.RemoveNotification(1)); + Assert.AreEqual(0, NotificationCenter.NotificationsCount); + } + + [Test] + public void TestAddMultipleNotificationListeners() + { + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + + // Verify that multiple notifications will be added for same notification type. + Assert.AreEqual(2, NotificationCenter.NotificationsCount); + + // Verify that notifications of other types will also gets added successfully. + NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestTrackCallback); + Assert.AreEqual(3, NotificationCenter.NotificationsCount); + } + + [Test] + public void TestAddSameNotificationListenerMultipleTimes() + { + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); + + // Verify that adding same callback multiple times will gets failed. + Assert.AreEqual(-1, NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback)); + Assert.AreEqual(1, NotificationCenter.NotificationsCount); + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "The notification callback already exists."), Times.Once); + } + + [Test] + public void TestAddInvalidNotificationListeners() + { + // Verify that AddNotification gets failed on adding invalid notification listeners. + Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeTrack, + TestNotificationCallbacks.TestDecisionCallback)); + Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeFeatureAccess, + TestNotificationCallbacks.TestTrackCallback)); + Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeDecision, + TestNotificationCallbacks.TestFeatureAccessCallback)); + + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeDecision}"" callback."), + Times.Once); + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeTrack}"" callback."), + Times.Once); + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeFeatureAccess}"" callback."), + Times.Once); + + // Verify that no notifion has been added. + Assert.AreEqual(0, NotificationCenter.NotificationsCount); + } + + [Test] + public void TestClearNotifications() + { + // Add decision notifications. + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + + // Add track notification. + NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestTrackCallback); + + // Verify that callbacks added successfully. + Assert.AreEqual(3, NotificationCenter.NotificationsCount); + + // Verify that only decision callbacks are removed. + NotificationCenter.ClearNotifications(NotificationTypeDecision); + Assert.AreEqual(1, NotificationCenter.NotificationsCount); + + // Verify that ClearNotifications does not break on calling twice for same type. + NotificationCenter.ClearNotifications(NotificationTypeDecision); + NotificationCenter.ClearNotifications(NotificationTypeDecision); + + // Verify that ClearNotifications does not break after calling ClearAllNotifications. + NotificationCenter.ClearAllNotifications(); + NotificationCenter.ClearNotifications(NotificationTypeTrack); + } + + [Test] + public void TestClearAllNotifications() + { + // Add decision notifications. + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + + // Add track notification. + NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestTrackCallback); + + // Verify that callbacks added successfully. + Assert.AreEqual(3, NotificationCenter.NotificationsCount); + + // Verify that ClearAllNotifications remove all the callbacks. + NotificationCenter.ClearAllNotifications(); + Assert.AreEqual(0, NotificationCenter.NotificationsCount); + + // Verify that ClearAllNotifications does not break on calling twice or after ClearNotifications. + NotificationCenter.ClearNotifications(NotificationTypeDecision); + NotificationCenter.ClearAllNotifications(); + NotificationCenter.ClearAllNotifications(); + } + + [Test] + public void TestFireNotifications() + { + var config = ProjectConfig.Create(TestData.Datafile, LoggerMock.Object, new NoOpErrorHandler()); + + // Mocking notification callbacks. + var notificationCallbackMock = new Mock(); + notificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + notificationCallbackMock.Setup(nc => nc.TestAnotherDecisionCallback(It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); + + // Adding decision notifications. + NotificationCenter.AddNotification(NotificationTypeDecision, notificationCallbackMock.Object.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeDecision, notificationCallbackMock.Object.TestAnotherDecisionCallback); + + // Adding track notifications. + NotificationCenter.AddNotification(NotificationTypeTrack, notificationCallbackMock.Object.TestTrackCallback); + + // Firing decision type notifications. + NotificationCenter.FireNotifications(NotificationTypeDecision, config.GetExperimentFromKey("test_experiment"), + "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); + + // Verify that only the registered notifications of decision type are called. + notificationCallbackMock.Verify(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); + notificationCallbackMock.Verify(nc => nc.TestAnotherDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); + notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + // Verify that FireNotifications does not break when no notification exists. + NotificationCenter.ClearAllNotifications(); + NotificationCenter.FireNotifications(NotificationTypeDecision, config.GetExperimentFromKey("test_experiment"), + "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); + } + } + + #region Test Notification callbacks class. + + /// + /// Test class containing dummy notification callbacks. + /// + public class TestNotificationCallbacks + { + public virtual void TestDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + Variation variation, LogEvent logEvent) { + } + + public virtual void TestAnotherDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + Variation variation, LogEvent logEvent) { + } + + public static void TestStaticDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + Variation variation, LogEvent logEvent) { + } + + public virtual void TestTrackCallback(string eventKey, string userId, UserAttributes userAttributes, + EventTags eventTags, LogEvent logEvent) { + } + + public virtual void TestAnotherTrackCallback(string eventKey, string userId, UserAttributes userAttributes, + EventTags eventTags, LogEvent logEvent) { + } + + public virtual void TestFeatureAccessCallback(string featureKey, string userId, UserAttributes userAttributes, + Variation variation) { + } + } + #endregion // Test Notification callbacks class. +} diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index efd4162c..46a40ce6 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -75,6 +75,7 @@ + diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index dc5d6ec4..e8cdd179 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -26,6 +26,8 @@ using NUnit.Framework; using OptimizelySDK.Tests.UtilsTests; using OptimizelySDK.Bucketing; +using OptimizelySDK.Notifications; +using OptimizelySDK.Tests.NotificationTests; namespace OptimizelySDK.Tests { @@ -42,6 +44,8 @@ public class OptimizelyTest private OptimizelyHelper Helper; private Mock OptimizelyMock; private Mock DecisionServiceMock; + private NotificationCenter NotificationCenter; + private Mock NotificationCallbackMock; #region Test Life Cycle [SetUp] @@ -85,6 +89,9 @@ public void Initialize() DecisionServiceMock = new Mock(new Bucketer(LoggerMock.Object), ErrorHandlerMock.Object, Config, null, LoggerMock.Object); + + NotificationCenter = new NotificationCenter(LoggerMock.Object); + NotificationCallbackMock = new Mock(); } [TestFixtureTearDown] @@ -1495,5 +1502,234 @@ public void TestIsFeatureEnabledGivenFeatureFlagIsEnabledAndUserIsBeingExperimen } #endregion // Test IsFeatureEnabled method + + #region Test NotificationCenter + + [Test] + public void TestActivateListenerWithoutAttributes() + { + TestActivateListener(null); + } + + [Test] + public void TestActivateListenerWithAttributes() + { + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + TestActivateListener(userAttributes); + } + + public void TestActivateListener(UserAttributes userAttributes) + { + var experimentKey = "test_experiment"; + var variationKey = "control"; + var featureKey = "boolean_feature"; + var experiment = Config.GetExperimentFromKey(experimentKey); + var variation = Config.GetVariationFromKey(experimentKey, variationKey); + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, + "POST", new Dictionary { }); + + // Mocking objects. + NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestAnotherDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, TestUserId, userAttributes)).Returns(variation); + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); + + // Adding notification listeners. + var notificationType = NotificationCenter.NotificationType.Decision; + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestDecisionCallback); + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestAnotherDecisionCallback); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); + + // Calling Activate and IsFeatureEnabled. + optly.Invoke("Activate", experimentKey, TestUserId, userAttributes); + optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); + + // Verify that all the registered callbacks are called once for both Activate and IsFeatureEnabled. + NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); + NotificationCallbackMock.Verify(nc => nc.TestAnotherDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); + } + + [Test] + public void TestTrackListenerWithoutAttributesAndEventTags() + { + TestTrackListener(null, null); + } + + [Test] + public void TestTrackListenerWithAttributesWithoutEventTags() + { + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + TestTrackListener(userAttributes, null); + } + + [Test] + public void TestTrackListenerWithAttributesAndEventTags() + { + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + var eventTags = new EventTags + { + { "revenue", 42 } + }; + + TestTrackListener(userAttributes, eventTags); + } + + public void TestTrackListener(UserAttributes userAttributes, EventTags eventTags) + { + var experimentKey = "test_experiment"; + var variationKey = "control"; + var eventKey = "purchase"; + var experiment = Config.GetExperimentFromKey(experimentKey); + var variation = Config.GetVariationFromKey(experimentKey, variationKey); + var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, + "POST", new Dictionary { }); + + // Mocking objects. + NotificationCallbackMock.Setup(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestAnotherTrackCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + EventBuilderMock.Setup(ebm => ebm.CreateConversionEvent(It.IsAny(), It.IsAny(), + It.IsAny>(), It.IsAny(), It.IsAny(), + It.IsAny())).Returns(logEvent); + DecisionServiceMock.Setup(ds => ds.GetVariation(experiment, TestUserId, userAttributes)).Returns(variation); + + // Adding notification listeners. + var notificationType = NotificationCenter.NotificationType.Track; + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestTrackCallback); + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestAnotherTrackCallback); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); + + // Calling Track. + optly.Invoke("Track", eventKey, TestUserId, userAttributes, eventTags); + + // Verify that all the registered callbacks for Track are called. + NotificationCallbackMock.Verify(nc => nc.TestTrackCallback(eventKey, TestUserId, userAttributes, eventTags, logEvent), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestAnotherTrackCallback(eventKey, TestUserId, userAttributes, eventTags, logEvent), Times.Exactly(1)); + } + + [Test] + public void TestFeatureAccessListenerWithFeatureExperiment() + { + var featureKey = "boolean_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var experiment = Config.GetExperimentFromId(featureFlag.ExperimentIds[0]); + var variation = experiment.Variations[0]; + var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, + "POST", new Dictionary { }); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + // Mocking objects. + NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestFeatureAccessCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())); + EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); + + // Adding notification listeners. + NotificationCenter.AddNotification(NotificationCenter.NotificationType.Decision, + NotificationCallbackMock.Object.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureAccess, + NotificationCallbackMock.Object.TestFeatureAccessCallback); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); + + // Calling IsFeatureEnabled. + optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); + + // Verify that both of the decision and feature access callbacks are called in case of feature experiment. + NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestFeatureAccessCallback(featureKey, TestUserId, userAttributes, variation), Times.Exactly(1)); + } + + [Test] + public void TestFeatureAccessListenerWithRolloutRule() + { + var featureKey = "boolean_single_variable_feature"; + var featureFlag = Config.GetFeatureFlagFromKey(featureKey); + var rollout = Config.GetRolloutFromId(featureFlag.RolloutId); + var experiment = rollout.Experiments[0]; + var variation = experiment.Variations[0]; + var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, + "POST", new Dictionary { }); + var userAttributes = new UserAttributes + { + { "device_type", "iPhone" }, + { "company", "Optimizely" }, + { "location", "San Francisco" } + }; + + // Mocking objects. + NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestFeatureAccessCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())); + EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); + DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); + + // Adding notification listeners. + NotificationCenter.AddNotification(NotificationCenter.NotificationType.Decision, + NotificationCallbackMock.Object.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureAccess, + NotificationCallbackMock.Object.TestFeatureAccessCallback); + + var optly = Helper.CreatePrivateOptimizely(); + optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); + optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); + optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); + + // Calling IsFeatureEnabled. + optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); + + // Verify that only feature access callback gets called in case of rollout rule. + NotificationCallbackMock.Verify(nc => nc.TestFeatureAccessCallback(featureKey, TestUserId, userAttributes, variation), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Never); + } + + #endregion // Test NotificationCenter } } diff --git a/OptimizelySDK/Notifications/NotificationCenter.cs b/OptimizelySDK/Notifications/NotificationCenter.cs new file mode 100644 index 00000000..a5d0fc47 --- /dev/null +++ b/OptimizelySDK/Notifications/NotificationCenter.cs @@ -0,0 +1,268 @@ +/* + * Copyright 2017, Optimizely + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +using OptimizelySDK.Entity; +using OptimizelySDK.Event; +using OptimizelySDK.Logger; +using System; +using System.Collections.Generic; +using System.Linq; + +namespace OptimizelySDK.Notifications +{ + /// + /// NotificationCenter class for sending notifications. + /// + public class NotificationCenter + { + /// + /// Enum representing notification types. + /// + public enum NotificationType + { + Decision, // Activate called. + Track, // Event tracked. + FeatureAccess // Feature accessed + }; + + /// + /// Delegate for decision notifcations. + /// + /// The experiment entity + /// The user identifier + /// Associative array of attributes for the user + /// The variation entity + /// The impression event + public delegate void DecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + Variation variation, LogEvent logEvent); + + /// + /// Delegate for track notifcations. + /// + /// The event key + /// The user identifier + /// Associative array of attributes for the user + /// Associative array of EventTags representing metadata associated with the event + /// The conversion event + public delegate void TrackCallback(string eventKey, string userId, UserAttributes userAttributes, EventTags eventTags, + LogEvent logEvent); + + /// + /// Delegate for feature access notifcations. + /// + /// The feature key + /// The user identifier + /// Associative array of attributes for the user + /// The variation entity + public delegate void FeatureAccessCallback(string featureKey, string userId, UserAttributes userAttributes, + Variation variation); + + private ILogger Logger; + + // Notification Id represeting number of notifications. + public int NotificationId { get; private set; } = 1; + + // Associative array of notification type to notification id and notification pair. + private Dictionary> Notifications = + new Dictionary>(); + + /// + /// Property representing total notifications count. + /// + public int NotificationsCount + { + get + { + int notificationsCount = 0; + foreach (var notificationsMap in Notifications.Values) + { + notificationsCount += notificationsMap.Count; + } + + return notificationsCount; + } + } + + /// + /// NotificationCenter constructor + /// + /// The logger object + public NotificationCenter(ILogger logger = null) + { + Logger = logger ?? new NoOpLogger(); + + foreach (NotificationType notificationType in Enum.GetValues(typeof(NotificationType))) + { + Notifications[notificationType] = new Dictionary(); + } + } + + /// + /// Add a notification callback of decision type to the notification center. + /// + /// Notification type + /// Callback function to call when event gets triggered + /// int | 0 for invalid notification type, -1 for adding existing notification + /// or the notification id of newly added notification. + public int AddNotification(NotificationType notificationType, DecisionCallback decisionCallBack) + { + if (!IsNotificationTypeValid(notificationType, NotificationType.Decision)) + return 0; + + return AddNotification(notificationType, (object)decisionCallBack); + } + + /// + /// Add a notification callback of track type to the notification center. + /// + /// Notification type + /// Callback function to call when event gets triggered + /// int | 0 for invalid notification type, -1 for adding existing notification + /// or the notification id of newly added notification. + public int AddNotification(NotificationType notificationType, TrackCallback trackCallback) + { + if (!IsNotificationTypeValid(notificationType, NotificationType.Track)) + return 0; + + return AddNotification(notificationType, (object)trackCallback); + } + + /// + /// Add a notification callback of feature access type to the notification center. + /// + /// Notification type + /// Callback function to call when event gets triggered + /// int | 0 for invalid notification type, -1 for adding existing notification + /// or the notification id of newly added notification. + public int AddNotification(NotificationType notificationType, FeatureAccessCallback featureAccessCallback) + { + if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureAccess)) + return 0; + + return AddNotification(notificationType, (object)featureAccessCallback); + } + + /// + /// Validate notification type. + /// + /// Provided notification type + /// expected notification type + /// true if notification type is valid, false otherwise + private bool IsNotificationTypeValid(NotificationType providedNotificationType, NotificationType expectedNotificationType) + { + if (providedNotificationType != expectedNotificationType) + { + Logger.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{expectedNotificationType}"" callback."); + return false; + } + + return true; + } + + + /// + /// Add a notification callback to the notification center. + /// + /// Notification type + /// Callback function to call when event gets triggered + /// -1 for adding existing notification or the notification id of newly added notification. + private int AddNotification(NotificationType notificationType, object notificationCallback) + { + var notificationHoldersList = Notifications[notificationType]; + + if (!Notifications.ContainsKey(notificationType) || Notifications[notificationType].Count == 0) + Notifications[notificationType][NotificationId] = notificationCallback; + else + { + foreach(var notification in this.Notifications[notificationType]) + { + if ((Delegate)notification.Value == (Delegate)notificationCallback) + { + Logger.Log(LogLevel.ERROR, "The notification callback already exists."); + return -1; + } + } + + Notifications[notificationType][NotificationId] = notificationCallback; + } + + int retVal = NotificationId; + NotificationId += 1; + + return retVal; + } + + /// + /// Remove a previously added notification callback. + /// + /// Id of notification + /// Returns true if found and removed, false otherwise. + public bool RemoveNotification(int notificationId) + { + foreach(var key in Notifications.Keys) + { + if (Notifications[key] != null && Notifications[key].Any(notification => notification.Key == notificationId)) + { + Notifications[key].Remove(notificationId); + return true; + } + } + + return false; + } + + /// + /// Remove all notifications for the specified notification type. + /// + /// The notification type + public void ClearNotifications(NotificationType notificationType) + { + Notifications[notificationType].Clear(); + } + + /// + /// Removes all notifications. + /// + public void ClearAllNotifications() + { + foreach (var notificationsMap in Notifications.Values) + { + notificationsMap.Clear(); + } + } + + /// + /// Fire notifications of specified notification type when the event gets triggered. + /// + /// The notification type + /// Arguments to pass in notification callbacks + public void FireNotifications(NotificationType notificationType, params object[] args) + { + foreach (var notification in Notifications[notificationType]) + { + try + { + Delegate d = notification.Value as Delegate; + d.DynamicInvoke(args); + } + catch (Exception exception) + { + Logger.Log(LogLevel.ERROR, "Problem calling notify callback. Error: " + exception.Message); + } + } + } + } +} diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 9027f7e7..bdf3ab0d 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -20,6 +20,7 @@ using OptimizelySDK.Event.Dispatcher; using OptimizelySDK.Logger; using OptimizelySDK.Utils; +using OptimizelySDK.Notifications; using System; using System.Collections.Generic; using System.Reflection; @@ -28,7 +29,6 @@ namespace OptimizelySDK { public class Optimizely { - private Bucketer Bucketer; private EventBuilder EventBuilder; @@ -45,6 +45,8 @@ public class Optimizely private DecisionService DecisionService; + private NotificationCenter NotificationCenter; + public bool IsValid { get; private set; } public static String SDK_VERSION { @@ -91,6 +93,7 @@ public Optimizely(string datafile, Bucketer = new Bucketer(Logger); EventBuilder = new EventBuilder(Bucketer); UserProfileService = userProfileService; + NotificationCenter = new NotificationCenter(Logger); try { @@ -175,7 +178,7 @@ public string Activate(string experimentKey, string userId, UserAttributes userA userAttributes = userAttributes.FilterNullValues(Logger); } - SendImpressionEvent(experiment, variation.Id, userId, userAttributes); + SendImpressionEvent(experiment, variation, userId, userAttributes); return variation.Key; } @@ -262,6 +265,8 @@ public void Track(string eventKey, string userId, UserAttributes userAttributes Logger.Log(LogLevel.ERROR, string.Format("Unable to dispatch conversion event. Error {0}", exception.Message)); } + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Track, eventKey, userId, + userAttributes, eventTags, conversionEvent); } else { @@ -360,11 +365,14 @@ public Variation GetForcedVariation(string experimentKey, string userId) var experiment = Config.GetExperimentForVariationId(variation.Id); if (!string.IsNullOrEmpty(experiment.Key)) - SendImpressionEvent(experiment, variation.Id, userId, userAttributes); + SendImpressionEvent(experiment, variation, userId, userAttributes); else Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}""."); Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}""."); + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureAccess, featureKey, userId, + userAttributes, variation); + return true; } @@ -533,15 +541,15 @@ public string GetFeatureVariableString(string featureKey, string variableKey, st /// Sends impression event. /// /// The experiment - /// The variation Id + /// The variation entity /// The user ID /// The user's attributes - private void SendImpressionEvent(Experiment experiment, string variationId, string userId, + private void SendImpressionEvent(Experiment experiment, Variation variation, string userId, UserAttributes userAttributes) { if (experiment.IsExperimentRunning) { - var impressionEvent = EventBuilder.CreateImpressionEvent(Config, experiment, variationId, userId, userAttributes); + var impressionEvent = EventBuilder.CreateImpressionEvent(Config, experiment, variation.Id, userId, userAttributes); Logger.Log(LogLevel.INFO, string.Format("Activating user {0} in experiment {1}.", userId, experiment.Key)); Logger.Log(LogLevel.DEBUG, string.Format("Dispatching impression event to URL {0} with params {1}.", impressionEvent.Url, impressionEvent.GetParamsAsJson())); @@ -554,6 +562,9 @@ private void SendImpressionEvent(Experiment experiment, string variationId, stri { Logger.Log(LogLevel.ERROR, string.Format("Unable to dispatch impression event. Error {0}", exception.Message)); } + + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Decision, experiment, userId, + userAttributes, variation, impressionEvent); } else { diff --git a/OptimizelySDK/OptimizelySDK.csproj b/OptimizelySDK/OptimizelySDK.csproj index 5a072fc8..8e21fb92 100644 --- a/OptimizelySDK/OptimizelySDK.csproj +++ b/OptimizelySDK/OptimizelySDK.csproj @@ -93,6 +93,7 @@ + @@ -121,4 +122,4 @@ --> - + \ No newline at end of file From e86590e38c5458e73ed597fc772c4e73dcf8fedf Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Thu, 16 Nov 2017 21:32:54 +0500 Subject: [PATCH 2/7] Added FeatureExperiment and FeatureRollout notification types. --- .../NotificationCenterTests.cs | 77 ++++++++++--------- OptimizelySDK.Tests/OptimizelyTest.cs | 58 +++++++------- .../Notifications/NotificationCenter.cs | 60 +++++++++++---- OptimizelySDK/Optimizely.cs | 23 +++++- OptimizelySDK/ProjectConfig.cs | 35 +++++++++ 5 files changed, 165 insertions(+), 88 deletions(-) diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index 5ed6c0cf..38b6e665 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -29,9 +29,9 @@ public class NotificationCenterTests private Mock LoggerMock; private NotificationCenter NotificationCenter; private TestNotificationCallbacks TestNotificationCallbacks; - private NotificationCenter.NotificationType NotificationTypeDecision = NotificationCenter.NotificationType.Decision; + private NotificationCenter.NotificationType NotificationTypeActivate = NotificationCenter.NotificationType.Activate; private NotificationCenter.NotificationType NotificationTypeTrack = NotificationCenter.NotificationType.Track; - private NotificationCenter.NotificationType NotificationTypeFeatureAccess = NotificationCenter.NotificationType.FeatureAccess; + private NotificationCenter.NotificationType NotificationTypeFeatureExperiment = NotificationCenter.NotificationType.FeatureExperiment; [SetUp] public void Setup() @@ -47,7 +47,7 @@ public void Setup() public void TestAddAndRemoveNotificationListener() { // Verify that callback added successfully. - Assert.AreEqual(1, NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback)); + Assert.AreEqual(1, NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback)); Assert.AreEqual(1, NotificationCenter.NotificationsCount); // Verify that callback removed successfully. @@ -58,8 +58,8 @@ public void TestAddAndRemoveNotificationListener() [Test] public void TestAddMultipleNotificationListeners() { - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestAnotherActivateCallback); // Verify that multiple notifications will be added for same notification type. Assert.AreEqual(2, NotificationCenter.NotificationsCount); @@ -72,10 +72,10 @@ public void TestAddMultipleNotificationListeners() [Test] public void TestAddSameNotificationListenerMultipleTimes() { - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback); // Verify that adding same callback multiple times will gets failed. - Assert.AreEqual(-1, NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback)); + Assert.AreEqual(-1, NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback)); Assert.AreEqual(1, NotificationCenter.NotificationsCount); LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "The notification callback already exists."), Times.Once); } @@ -85,17 +85,17 @@ public void TestAddInvalidNotificationListeners() { // Verify that AddNotification gets failed on adding invalid notification listeners. Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeTrack, - TestNotificationCallbacks.TestDecisionCallback)); - Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeFeatureAccess, + TestNotificationCallbacks.TestActivateCallback)); + Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeFeatureExperiment, TestNotificationCallbacks.TestTrackCallback)); - Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeDecision, - TestNotificationCallbacks.TestFeatureAccessCallback)); + Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeActivate, + TestNotificationCallbacks.TestFeatureExperimentCallback)); - LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeDecision}"" callback."), + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeActivate}"" callback."), Times.Once); LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeTrack}"" callback."), Times.Once); - LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeFeatureAccess}"" callback."), + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeFeatureExperiment}"" callback."), Times.Once); // Verify that no notifion has been added. @@ -106,8 +106,8 @@ public void TestAddInvalidNotificationListeners() public void TestClearNotifications() { // Add decision notifications. - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestAnotherActivateCallback); // Add track notification. NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestTrackCallback); @@ -116,12 +116,12 @@ public void TestClearNotifications() Assert.AreEqual(3, NotificationCenter.NotificationsCount); // Verify that only decision callbacks are removed. - NotificationCenter.ClearNotifications(NotificationTypeDecision); + NotificationCenter.ClearNotifications(NotificationTypeActivate); Assert.AreEqual(1, NotificationCenter.NotificationsCount); // Verify that ClearNotifications does not break on calling twice for same type. - NotificationCenter.ClearNotifications(NotificationTypeDecision); - NotificationCenter.ClearNotifications(NotificationTypeDecision); + NotificationCenter.ClearNotifications(NotificationTypeActivate); + NotificationCenter.ClearNotifications(NotificationTypeActivate); // Verify that ClearNotifications does not break after calling ClearAllNotifications. NotificationCenter.ClearAllNotifications(); @@ -132,8 +132,8 @@ public void TestClearNotifications() public void TestClearAllNotifications() { // Add decision notifications. - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationTypeDecision, TestNotificationCallbacks.TestAnotherDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestAnotherActivateCallback); // Add track notification. NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestTrackCallback); @@ -146,7 +146,7 @@ public void TestClearAllNotifications() Assert.AreEqual(0, NotificationCenter.NotificationsCount); // Verify that ClearAllNotifications does not break on calling twice or after ClearNotifications. - NotificationCenter.ClearNotifications(NotificationTypeDecision); + NotificationCenter.ClearNotifications(NotificationTypeActivate); NotificationCenter.ClearAllNotifications(); NotificationCenter.ClearAllNotifications(); } @@ -158,33 +158,33 @@ public void TestFireNotifications() // Mocking notification callbacks. var notificationCallbackMock = new Mock(); - notificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + notificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - notificationCallbackMock.Setup(nc => nc.TestAnotherDecisionCallback(It.IsAny(), + notificationCallbackMock.Setup(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); // Adding decision notifications. - NotificationCenter.AddNotification(NotificationTypeDecision, notificationCallbackMock.Object.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationTypeDecision, notificationCallbackMock.Object.TestAnotherDecisionCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestActivateCallback); + NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestAnotherActivateCallback); // Adding track notifications. NotificationCenter.AddNotification(NotificationTypeTrack, notificationCallbackMock.Object.TestTrackCallback); // Firing decision type notifications. - NotificationCenter.FireNotifications(NotificationTypeDecision, config.GetExperimentFromKey("test_experiment"), + NotificationCenter.FireNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); // Verify that only the registered notifications of decision type are called. - notificationCallbackMock.Verify(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + notificationCallbackMock.Verify(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); - notificationCallbackMock.Verify(nc => nc.TestAnotherDecisionCallback(It.IsAny(), It.IsAny(), + notificationCallbackMock.Verify(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); // Verify that FireNotifications does not break when no notification exists. NotificationCenter.ClearAllNotifications(); - NotificationCenter.FireNotifications(NotificationTypeDecision, config.GetExperimentFromKey("test_experiment"), + NotificationCenter.FireNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); } } @@ -196,18 +196,14 @@ public void TestFireNotifications() /// public class TestNotificationCallbacks { - public virtual void TestDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + public virtual void TestActivateCallback(Experiment experiment, string userId, UserAttributes userAttributes, Variation variation, LogEvent logEvent) { } - public virtual void TestAnotherDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + public virtual void TestAnotherActivateCallback(Experiment experiment, string userId, UserAttributes userAttributes, Variation variation, LogEvent logEvent) { } - - public static void TestStaticDecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, - Variation variation, LogEvent logEvent) { - } - + public virtual void TestTrackCallback(string eventKey, string userId, UserAttributes userAttributes, EventTags eventTags, LogEvent logEvent) { } @@ -216,8 +212,13 @@ public virtual void TestAnotherTrackCallback(string eventKey, string userId, Use EventTags eventTags, LogEvent logEvent) { } - public virtual void TestFeatureAccessCallback(string featureKey, string userId, UserAttributes userAttributes, - Variation variation) { + public virtual void TestFeatureExperimentCallback(string featureKey, string userId, UserAttributes userAttributes, + Experiment experiment, Variation variation) { + } + + public virtual void TestFeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, + Audience audience) + { } } #endregion // Test Notification callbacks class. diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index e8cdd179..c028a44f 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1536,9 +1536,9 @@ public void TestActivateListener(UserAttributes userAttributes) "POST", new Dictionary { }); // Mocking objects. - NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - NotificationCallbackMock.Setup(nc => nc.TestAnotherDecisionCallback(It.IsAny(), It.IsAny(), + NotificationCallbackMock.Setup(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); NotificationCallbackMock.Setup(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); @@ -1548,9 +1548,9 @@ public void TestActivateListener(UserAttributes userAttributes) DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); // Adding notification listeners. - var notificationType = NotificationCenter.NotificationType.Decision; - NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestDecisionCallback); - NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestAnotherDecisionCallback); + var notificationType = NotificationCenter.NotificationType.Activate; + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestActivateCallback); + NotificationCenter.AddNotification(notificationType, NotificationCallbackMock.Object.TestAnotherActivateCallback); var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); @@ -1562,8 +1562,8 @@ public void TestActivateListener(UserAttributes userAttributes) optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); // Verify that all the registered callbacks are called once for both Activate and IsFeatureEnabled. - NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); - NotificationCallbackMock.Verify(nc => nc.TestAnotherDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); + NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); + NotificationCallbackMock.Verify(nc => nc.TestAnotherActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(2)); } [Test] @@ -1642,7 +1642,7 @@ public void TestTrackListener(UserAttributes userAttributes, EventTags eventTags } [Test] - public void TestFeatureAccessListenerWithFeatureExperiment() + public void TestFeatureExperimentListener() { var featureKey = "boolean_feature"; var featureFlag = Config.GetFeatureFlagFromKey(featureKey); @@ -1658,19 +1658,19 @@ public void TestFeatureAccessListenerWithFeatureExperiment() }; // Mocking objects. - NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - NotificationCallbackMock.Setup(nc => nc.TestFeatureAccessCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestFeatureExperimentCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny())); EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); // Adding notification listeners. - NotificationCenter.AddNotification(NotificationCenter.NotificationType.Decision, - NotificationCallbackMock.Object.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureAccess, - NotificationCallbackMock.Object.TestFeatureAccessCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.Activate, + NotificationCallbackMock.Object.TestActivateCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureExperiment, + NotificationCallbackMock.Object.TestFeatureExperimentCallback); var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); @@ -1680,13 +1680,13 @@ public void TestFeatureAccessListenerWithFeatureExperiment() // Calling IsFeatureEnabled. optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); - // Verify that both of the decision and feature access callbacks are called in case of feature experiment. - NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(1)); - NotificationCallbackMock.Verify(nc => nc.TestFeatureAccessCallback(featureKey, TestUserId, userAttributes, variation), Times.Exactly(1)); + // Verify that both of the activate and feature experiment callbacks are called in case of feature experiment. + NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestFeatureExperimentCallback(featureKey, TestUserId, userAttributes, experiment, variation), Times.Exactly(1)); } [Test] - public void TestFeatureAccessListenerWithRolloutRule() + public void TestFeatureRolloutListener() { var featureKey = "boolean_single_variable_feature"; var featureFlag = Config.GetFeatureFlagFromKey(featureKey); @@ -1703,19 +1703,19 @@ public void TestFeatureAccessListenerWithRolloutRule() }; // Mocking objects. - NotificationCallbackMock.Setup(nc => nc.TestDecisionCallback(It.IsAny(), It.IsAny(), + NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - NotificationCallbackMock.Setup(nc => nc.TestFeatureAccessCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())); + NotificationCallbackMock.Setup(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())); EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); // Adding notification listeners. - NotificationCenter.AddNotification(NotificationCenter.NotificationType.Decision, - NotificationCallbackMock.Object.TestDecisionCallback); - NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureAccess, - NotificationCallbackMock.Object.TestFeatureAccessCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.Activate, + NotificationCallbackMock.Object.TestActivateCallback); + NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureRollout, + NotificationCallbackMock.Object.TestFeatureRolloutCallback); var optly = Helper.CreatePrivateOptimizely(); optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); @@ -1725,9 +1725,9 @@ public void TestFeatureAccessListenerWithRolloutRule() // Calling IsFeatureEnabled. optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); - // Verify that only feature access callback gets called in case of rollout rule. - NotificationCallbackMock.Verify(nc => nc.TestFeatureAccessCallback(featureKey, TestUserId, userAttributes, variation), Times.Exactly(1)); - NotificationCallbackMock.Verify(nc => nc.TestDecisionCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Never); + // Verify that only feature rollout callback gets called in case of rollout rule. + NotificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(featureKey, TestUserId, userAttributes, It.IsAny()), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Never); } #endregion // Test NotificationCenter diff --git a/OptimizelySDK/Notifications/NotificationCenter.cs b/OptimizelySDK/Notifications/NotificationCenter.cs index a5d0fc47..e0bc9015 100644 --- a/OptimizelySDK/Notifications/NotificationCenter.cs +++ b/OptimizelySDK/Notifications/NotificationCenter.cs @@ -33,20 +33,21 @@ public class NotificationCenter /// public enum NotificationType { - Decision, // Activate called. + Activate, // Activate called. Track, // Event tracked. - FeatureAccess // Feature accessed + FeatureExperiment, // Feature accessed from experiment. + FeatureRollout // Feature accessed from rollout rule. }; /// - /// Delegate for decision notifcations. + /// Delegate for activate notifcations. /// /// The experiment entity /// The user identifier /// Associative array of attributes for the user /// The variation entity /// The impression event - public delegate void DecisionCallback(Experiment experiment, string userId, UserAttributes userAttributes, + public delegate void ActivateCallback(Experiment experiment, string userId, UserAttributes userAttributes, Variation variation, LogEvent logEvent); /// @@ -61,15 +62,26 @@ public delegate void TrackCallback(string eventKey, string userId, UserAttribute LogEvent logEvent); /// - /// Delegate for feature access notifcations. + /// Delegate for feature experiment notifcations. /// /// The feature key /// The user identifier /// Associative array of attributes for the user + /// The experiment entity /// The variation entity - public delegate void FeatureAccessCallback(string featureKey, string userId, UserAttributes userAttributes, - Variation variation); - + public delegate void FeatureExperimentCallback(string featureKey, string userId, UserAttributes userAttributes, + Experiment experiment, Variation variation); + + /// + /// Delegate for feature rollout notifcations. + /// + /// The feature key + /// The user identifier + /// Associative array of attributes for the user + /// The audience entity + public delegate void FeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, + Audience audience); + private ILogger Logger; // Notification Id represeting number of notifications. @@ -117,12 +129,12 @@ public NotificationCenter(ILogger logger = null) /// Callback function to call when event gets triggered /// int | 0 for invalid notification type, -1 for adding existing notification /// or the notification id of newly added notification. - public int AddNotification(NotificationType notificationType, DecisionCallback decisionCallBack) + public int AddNotification(NotificationType notificationType, ActivateCallback activateCallback) { - if (!IsNotificationTypeValid(notificationType, NotificationType.Decision)) + if (!IsNotificationTypeValid(notificationType, NotificationType.Activate)) return 0; - return AddNotification(notificationType, (object)decisionCallBack); + return AddNotification(notificationType, (object)activateCallback); } /// @@ -141,18 +153,33 @@ public int AddNotification(NotificationType notificationType, TrackCallback trac } /// - /// Add a notification callback of feature access type to the notification center. + /// Add a notification callback of feature experiment type to the notification center. /// /// Notification type - /// Callback function to call when event gets triggered + /// Callback function to call when event gets triggered /// int | 0 for invalid notification type, -1 for adding existing notification /// or the notification id of newly added notification. - public int AddNotification(NotificationType notificationType, FeatureAccessCallback featureAccessCallback) + public int AddNotification(NotificationType notificationType, FeatureExperimentCallback featureExperimentCallback) { - if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureAccess)) + if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureExperiment)) return 0; - return AddNotification(notificationType, (object)featureAccessCallback); + return AddNotification(notificationType, (object)featureExperimentCallback); + } + + /// + /// Add a notification callback of feature rollout type to the notification center. + /// + /// Notification type + /// Callback function to call when event gets triggered + /// int | 0 for invalid notification type, -1 for adding existing notification + /// or the notification id of newly added notification. + public int AddNotification(NotificationType notificationType, FeatureRolloutCallback featureRolloutCallback) + { + if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureRollout)) + return 0; + + return AddNotification(notificationType, (object)featureRolloutCallback); } /// @@ -172,7 +199,6 @@ private bool IsNotificationTypeValid(NotificationType providedNotificationType, return true; } - /// /// Add a notification callback to the notification center. /// diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index bdf3ab0d..a80090d4 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -365,14 +365,29 @@ public Variation GetForcedVariation(string experimentKey, string userId) var experiment = Config.GetExperimentForVariationId(variation.Id); if (!string.IsNullOrEmpty(experiment.Key)) + { SendImpressionEvent(experiment, variation, userId, userAttributes); + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureExperiment, featureKey, userId, + userAttributes, experiment, variation); + } else + { + Audience audience = null; + var rolloutRule = Config.GetRolloutRuleForVariationId(variation.Id); + + if (!string.IsNullOrEmpty(rolloutRule.Key) + && rolloutRule.AudienceIds != null + && rolloutRule.AudienceIds.Length > 0) + { + audience = Config.GetAudience(rolloutRule.AudienceIds[0]); + } + + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureRollout, featureKey, userId, + userAttributes, audience); Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}""."); + } Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is enabled for user ""{userId}""."); - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureAccess, featureKey, userId, - userAttributes, variation); - return true; } @@ -563,7 +578,7 @@ private void SendImpressionEvent(Experiment experiment, Variation variation, str Logger.Log(LogLevel.ERROR, string.Format("Unable to dispatch impression event. Error {0}", exception.Message)); } - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Decision, experiment, userId, + NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Activate, experiment, userId, userAttributes, variation, impressionEvent); } else diff --git a/OptimizelySDK/ProjectConfig.cs b/OptimizelySDK/ProjectConfig.cs index 539ebcc9..2e928a5b 100644 --- a/OptimizelySDK/ProjectConfig.cs +++ b/OptimizelySDK/ProjectConfig.cs @@ -132,6 +132,12 @@ private Dictionary> _VariationIdMap private Dictionary _VariationIdToExperimentMap = new Dictionary(); public Dictionary VariationIdToExperimentMap{ get { return _VariationIdToExperimentMap; } } + /// + /// Associative array of Variation ID to Rollout Rules(s) in the datafile + /// + private Dictionary _VariationIdToRolloutRuleMap = new Dictionary(); + public Dictionary VariationIdToRolloutRuleMap { get { return _VariationIdToRolloutRuleMap; } } + //========================= Callbacks =========================== /// @@ -248,6 +254,19 @@ private void Initialize() } } } + + // Generate Variation ID to Rollout Rule map. + foreach (var rollout in Rollouts) + { + foreach (var rolloutRule in rollout.Experiments) + { + if (rolloutRule.Variations != null) + { + foreach (var variation in rolloutRule.Variations) + _VariationIdToRolloutRuleMap[variation.Id] = rolloutRule; + } + } + } } public static ProjectConfig Create(string content, ILogger logger, IErrorHandler errorHandler) @@ -557,5 +576,21 @@ public Experiment GetExperimentForVariationId(string variationId) return new Experiment(); } + + /// + /// Get rollout rule for variation. + /// + /// Variation ID + /// Rollout Rule corresponding to the variation ID or a dummy entity if ID is invalid + public Experiment GetRolloutRuleForVariationId(string variationId) + { + if (_VariationIdToRolloutRuleMap.ContainsKey(variationId)) + return _VariationIdToRolloutRuleMap[variationId]; + + Logger.Log(LogLevel.ERROR, $@"No rollout rule has been defined in datafile for variation ""{variationId}""."); + ErrorHandler.HandleError(new Exceptions.InvalidVariationException("No rollout rule has been found for provided variation Id in the datafile.")); + + return new Experiment(); + } } } From 857b9344c2d95e2ee4580481757baf1d495d8802 Mon Sep 17 00:00:00 2001 From: mfahadahmed Date: Fri, 17 Nov 2017 14:54:56 +0500 Subject: [PATCH 3/7] Fixed code review defects. --- .../NotificationTests/NotificationCenterTests.cs | 10 +++++----- OptimizelySDK.Tests/OptimizelyTest.cs | 4 ++-- OptimizelySDK/Notifications/NotificationCenter.cs | 6 +++--- OptimizelySDK/Optimizely.cs | 14 +++++++------- 4 files changed, 17 insertions(+), 17 deletions(-) diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index 38b6e665..6faae0bb 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -152,7 +152,7 @@ public void TestClearAllNotifications() } [Test] - public void TestFireNotifications() + public void TestSendNotifications() { var config = ProjectConfig.Create(TestData.Datafile, LoggerMock.Object, new NoOpErrorHandler()); @@ -171,7 +171,7 @@ public void TestFireNotifications() NotificationCenter.AddNotification(NotificationTypeTrack, notificationCallbackMock.Object.TestTrackCallback); // Firing decision type notifications. - NotificationCenter.FireNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), + NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); // Verify that only the registered notifications of decision type are called. @@ -182,9 +182,9 @@ public void TestFireNotifications() notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Verify that FireNotifications does not break when no notification exists. + // Verify that SendNotifications does not break when no notification exists. NotificationCenter.ClearAllNotifications(); - NotificationCenter.FireNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), + NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); } } @@ -217,7 +217,7 @@ public virtual void TestFeatureExperimentCallback(string featureKey, string user } public virtual void TestFeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, - Audience audience) + Audience[] audiences) { } } diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index c028a44f..69cd9913 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1706,7 +1706,7 @@ public void TestFeatureRolloutListener() NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); NotificationCallbackMock.Setup(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())); + It.IsAny(), It.IsAny())); EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); @@ -1726,7 +1726,7 @@ public void TestFeatureRolloutListener() optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); // Verify that only feature rollout callback gets called in case of rollout rule. - NotificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(featureKey, TestUserId, userAttributes, It.IsAny()), Times.Exactly(1)); + NotificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(featureKey, TestUserId, userAttributes, It.IsAny()), Times.Exactly(1)); NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Never); } diff --git a/OptimizelySDK/Notifications/NotificationCenter.cs b/OptimizelySDK/Notifications/NotificationCenter.cs index e0bc9015..2eb8a69e 100644 --- a/OptimizelySDK/Notifications/NotificationCenter.cs +++ b/OptimizelySDK/Notifications/NotificationCenter.cs @@ -78,9 +78,9 @@ public delegate void FeatureExperimentCallback(string featureKey, string userId, /// The feature key /// The user identifier /// Associative array of attributes for the user - /// The audience entity + /// Array of audience public delegate void FeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, - Audience audience); + Audience[] audiences); private ILogger Logger; @@ -275,7 +275,7 @@ public void ClearAllNotifications() /// /// The notification type /// Arguments to pass in notification callbacks - public void FireNotifications(NotificationType notificationType, params object[] args) + public void SendNotifications(NotificationType notificationType, params object[] args) { foreach (var notification in Notifications[notificationType]) { diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index a80090d4..9974a4b1 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -265,7 +265,7 @@ public void Track(string eventKey, string userId, UserAttributes userAttributes Logger.Log(LogLevel.ERROR, string.Format("Unable to dispatch conversion event. Error {0}", exception.Message)); } - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Track, eventKey, userId, + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Track, eventKey, userId, userAttributes, eventTags, conversionEvent); } else @@ -367,23 +367,23 @@ public Variation GetForcedVariation(string experimentKey, string userId) if (!string.IsNullOrEmpty(experiment.Key)) { SendImpressionEvent(experiment, variation, userId, userAttributes); - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureExperiment, featureKey, userId, + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.FeatureExperiment, featureKey, userId, userAttributes, experiment, variation); } else { - Audience audience = null; + var audiences = new Audience[1]; var rolloutRule = Config.GetRolloutRuleForVariationId(variation.Id); if (!string.IsNullOrEmpty(rolloutRule.Key) && rolloutRule.AudienceIds != null && rolloutRule.AudienceIds.Length > 0) { - audience = Config.GetAudience(rolloutRule.AudienceIds[0]); + audiences[0] = Config.GetAudience(rolloutRule.AudienceIds[0]); } - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.FeatureRollout, featureKey, userId, - userAttributes, audience); + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.FeatureRollout, featureKey, userId, + userAttributes, audiences); Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}""."); } @@ -578,7 +578,7 @@ private void SendImpressionEvent(Experiment experiment, Variation variation, str Logger.Log(LogLevel.ERROR, string.Format("Unable to dispatch impression event. Error {0}", exception.Message)); } - NotificationCenter.FireNotifications(NotificationCenter.NotificationType.Activate, experiment, userId, + NotificationCenter.SendNotifications(NotificationCenter.NotificationType.Activate, experiment, userId, userAttributes, variation, impressionEvent); } else From 79d6a9ac725a827ee0bd63ba287e8ed3b9f293a6 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Tue, 21 Nov 2017 00:18:16 -0800 Subject: [PATCH 4/7] TestBucketLogsCorrectlyWhenUserProfileFailsToSave fixed. Added one more test case for FeatureRollout Notification type. Removed ConfigParser method which was not being used. --- OptimizelySDK.Tests/DecisionServiceTest.cs | 4 +- .../DefaultErrorHandlerTest.cs | 77 +++++++++++++++++++ .../NotificationCenterTests.cs | 59 ++++++++++++-- .../OptimizelySDK.Tests.csproj | 1 + OptimizelySDK/Bucketing/DecisionService.cs | 4 +- OptimizelySDK/Utils/ConfigParser.cs | 12 --- 6 files changed, 134 insertions(+), 23 deletions(-) create mode 100644 OptimizelySDK.Tests/DefaultErrorHandlerTest.cs diff --git a/OptimizelySDK.Tests/DecisionServiceTest.cs b/OptimizelySDK.Tests/DecisionServiceTest.cs index bf743c8f..f766cd62 100644 --- a/OptimizelySDK.Tests/DecisionServiceTest.cs +++ b/OptimizelySDK.Tests/DecisionServiceTest.cs @@ -277,7 +277,6 @@ public void TestGetVariationSavesBucketedVariationIntoUserProfile() } [Test] - [ExpectedException] public void TestBucketLogsCorrectlyWhenUserProfileFailsToSave() { Experiment experiment = NoAudienceProjectConfig.Experiments[0]; @@ -300,8 +299,9 @@ public void TestBucketLogsCorrectlyWhenUserProfileFailsToSave() decisionService.SaveVariation(experiment, variation, saveUserProfile); LoggerMock.Verify(l => l.Log(LogLevel.ERROR, string.Format - ("Failed to save variation \"{0}\" of experiment \"{1}\" for user \"{2}\".", UserProfileId, variation.Id, experiment.Id)) + ("Failed to save variation \"{0}\" of experiment \"{1}\" for user \"{2}\".", variation.Id, experiment.Id, UserProfileId)) , Times.Once); + ErrorHandlerMock.Verify(er => er.HandleError(It.IsAny()), Times.Once); } [Test] diff --git a/OptimizelySDK.Tests/DefaultErrorHandlerTest.cs b/OptimizelySDK.Tests/DefaultErrorHandlerTest.cs new file mode 100644 index 00000000..07db4493 --- /dev/null +++ b/OptimizelySDK.Tests/DefaultErrorHandlerTest.cs @@ -0,0 +1,77 @@ +/** + * + * Copyright 2017, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +using System; +using System.Collections.Generic; +using Moq; +using OptimizelySDK.Logger; +using OptimizelySDK.ErrorHandler; +using OptimizelySDK.Entity; +using NUnit.Framework; +using OptimizelySDK.Bucketing; +using OptimizelySDK.Exceptions; + +namespace OptimizelySDK.Tests +{ + public class DefaultErrorHandlerTest + { + private DefaultErrorHandler DefaultErrorHandler; + private Mock LoggerMock; + + [SetUp] + public void Setup() + { + LoggerMock = new Mock(); + } + + [Test] + public void TestErrorHandlerMessage() + { + DefaultErrorHandler = new DefaultErrorHandler(LoggerMock.Object, false); + string testingException = "Testing exception"; + try + { + throw new OptimizelyException("Testing exception"); + } + catch(OptimizelyException ex) + { + DefaultErrorHandler.HandleError(ex); + } + + LoggerMock.Verify(log => log.Log(LogLevel.ERROR, testingException), Times.Once); + } + + [Test] + [ExpectedException] + public void TestErrorHandlerMessageWithThrowException() + { + DefaultErrorHandler = new DefaultErrorHandler(LoggerMock.Object, true); + string testingException = "Testing and throwing exception"; + try + { + throw new OptimizelyException("Testing exception"); + } + catch (OptimizelyException ex) + { + //have to throw exception. + DefaultErrorHandler.HandleError(ex); + } + + LoggerMock.Verify(log => log.Log(LogLevel.ERROR, testingException), Times.Once); + } + + } +} diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index 6faae0bb..5e762c61 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -21,6 +21,7 @@ using OptimizelySDK.Event; using OptimizelySDK.Logger; using OptimizelySDK.Notifications; +using NotificationType = OptimizelySDK.Notifications.NotificationCenter.NotificationType; namespace OptimizelySDK.Tests.NotificationTests { @@ -29,9 +30,11 @@ public class NotificationCenterTests private Mock LoggerMock; private NotificationCenter NotificationCenter; private TestNotificationCallbacks TestNotificationCallbacks; - private NotificationCenter.NotificationType NotificationTypeActivate = NotificationCenter.NotificationType.Activate; - private NotificationCenter.NotificationType NotificationTypeTrack = NotificationCenter.NotificationType.Track; - private NotificationCenter.NotificationType NotificationTypeFeatureExperiment = NotificationCenter.NotificationType.FeatureExperiment; + + private NotificationType NotificationTypeActivate = NotificationType.Activate; + private NotificationType NotificationTypeTrack = NotificationType.Track; + private NotificationType NotificationTypeFeatureExperiment = NotificationType.FeatureExperiment; + private NotificationType NotificationTypeFeatureRollout = NotificationType.FeatureRollout; [SetUp] public void Setup() @@ -53,6 +56,13 @@ public void TestAddAndRemoveNotificationListener() // Verify that callback removed successfully. Assert.AreEqual(true, NotificationCenter.RemoveNotification(1)); Assert.AreEqual(0, NotificationCenter.NotificationsCount); + + //Verify return false with invalid ID. + Assert.AreEqual(false, NotificationCenter.RemoveNotification(1)); + + // Verify that callback added successfully and return right notification ID. + Assert.AreEqual(NotificationCenter.NotificationId, NotificationCenter.AddNotification(NotificationTypeActivate, TestNotificationCallbacks.TestActivateCallback)); + Assert.AreEqual(1, NotificationCenter.NotificationsCount); } [Test] @@ -158,34 +168,69 @@ public void TestSendNotifications() // Mocking notification callbacks. var notificationCallbackMock = new Mock(); + notificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); + notificationCallbackMock.Setup(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); + notificationCallbackMock.Setup(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny())); + // Adding decision notifications. NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestActivateCallback); NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestAnotherActivateCallback); + NotificationCenter.AddNotification(NotificationTypeFeatureRollout, notificationCallbackMock.Object.TestFeatureRolloutCallback); + // Adding track notifications. NotificationCenter.AddNotification(NotificationTypeTrack, notificationCallbackMock.Object.TestTrackCallback); - // Firing decision type notifications. + // Fire decision type notifications. NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); // Verify that only the registered notifications of decision type are called. notificationCallbackMock.Verify(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); + It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + notificationCallbackMock.Verify(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny()), Times.Exactly(1)); + It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Verify that SendNotifications does not break when no notification exists. + + // Fire feature rollout notification + NotificationCenter.SendNotifications(NotificationTypeFeatureRollout, "featureKey", "testUser", new UserAttributes(), null); + + notificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Once); + + + // Verify that after clearing notifications, SendNotification should not call any notification + // which were previously registered. NotificationCenter.ClearAllNotifications(); + notificationCallbackMock.ResetCalls(); + NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); + + + // Again verify notifications which were registered are not called. + notificationCallbackMock.Verify(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + notificationCallbackMock.Verify(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); + + notificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), + It.IsAny(), It.IsAny()), Times.Never); + } } diff --git a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj index 46a40ce6..e9edfc4a 100644 --- a/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj +++ b/OptimizelySDK.Tests/OptimizelySDK.Tests.csproj @@ -71,6 +71,7 @@ + diff --git a/OptimizelySDK/Bucketing/DecisionService.cs b/OptimizelySDK/Bucketing/DecisionService.cs index 8fc8cbbc..c3ea6638 100644 --- a/OptimizelySDK/Bucketing/DecisionService.cs +++ b/OptimizelySDK/Bucketing/DecisionService.cs @@ -241,8 +241,8 @@ public void SaveVariation(Experiment experiment, Variation variation, UserProfil } catch (Exception exception) { - Logger.Log(LogLevel.ERROR, string.Format("Failed to save variation \"{0}\" of experiment \"{1}\" for user \"{2}\": {3}.", - variation.Id, experiment.Id, userProfile.UserId, exception.Message)); + Logger.Log(LogLevel.ERROR, string.Format("Failed to save variation \"{0}\" of experiment \"{1}\" for user \"{2}\".", + variation.Id, experiment.Id, userProfile.UserId)); ErrorHandler.HandleError(new Exceptions.OptimizelyRuntimeException(exception.Message)); } } diff --git a/OptimizelySDK/Utils/ConfigParser.cs b/OptimizelySDK/Utils/ConfigParser.cs index 454a1237..1d1b73c0 100644 --- a/OptimizelySDK/Utils/ConfigParser.cs +++ b/OptimizelySDK/Utils/ConfigParser.cs @@ -32,17 +32,5 @@ public static Dictionary GenerateMap(IEnumerable entities, Func getKey(e), e => clone ? (T)e.Clone() : e); } - - /// - /// Creates an array of entities from the entity - /// (not sure this is really needed) - /// - /// Original Entities - /// Whether or not to clone the original entity - /// array of entities - public static T[] GenerateMap(IEnumerable entities, bool clone) - { - return entities.Select(e => clone ? (T)e.Clone() : e).ToArray(); - } } } \ No newline at end of file From 3478d57dd4d9f08b82031f900a093ae5be59e8de Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Wed, 22 Nov 2017 12:20:45 -0800 Subject: [PATCH 5/7] remvoed feature experiment and featurue rollout notifications. --- .../NotificationCenterTests.cs | 21 +---- OptimizelySDK.Tests/OptimizelyTest.cs | 89 ------------------- .../Notifications/NotificationCenter.cs | 54 +---------- OptimizelySDK/Optimizely.cs | 4 - 4 files changed, 2 insertions(+), 166 deletions(-) diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index 5e762c61..e3bc39fe 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -33,8 +33,6 @@ public class NotificationCenterTests private NotificationType NotificationTypeActivate = NotificationType.Activate; private NotificationType NotificationTypeTrack = NotificationType.Track; - private NotificationType NotificationTypeFeatureExperiment = NotificationType.FeatureExperiment; - private NotificationType NotificationTypeFeatureRollout = NotificationType.FeatureRollout; [SetUp] public void Setup() @@ -96,17 +94,10 @@ public void TestAddInvalidNotificationListeners() // Verify that AddNotification gets failed on adding invalid notification listeners. Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeTrack, TestNotificationCallbacks.TestActivateCallback)); - Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeFeatureExperiment, - TestNotificationCallbacks.TestTrackCallback)); - Assert.AreEqual(0, NotificationCenter.AddNotification(NotificationTypeActivate, - TestNotificationCallbacks.TestFeatureExperimentCallback)); + LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeActivate}"" callback."), Times.Once); - LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeTrack}"" callback."), - Times.Once); - LoggerMock.Verify(l => l.Log(LogLevel.ERROR, $@"Invalid notification type provided for ""{NotificationTypeFeatureExperiment}"" callback."), - Times.Once); // Verify that no notifion has been added. Assert.AreEqual(0, NotificationCenter.NotificationsCount); @@ -181,7 +172,6 @@ public void TestSendNotifications() // Adding decision notifications. NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestActivateCallback); NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestAnotherActivateCallback); - NotificationCenter.AddNotification(NotificationTypeFeatureRollout, notificationCallbackMock.Object.TestFeatureRolloutCallback); // Adding track notifications. @@ -202,11 +192,6 @@ public void TestSendNotifications() It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - // Fire feature rollout notification - NotificationCenter.SendNotifications(NotificationTypeFeatureRollout, "featureKey", "testUser", new UserAttributes(), null); - - notificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny()), Times.Once); // Verify that after clearing notifications, SendNotification should not call any notification @@ -227,10 +212,6 @@ public void TestSendNotifications() notificationCallbackMock.Verify(nc => nc.TestTrackCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny()), Times.Never); - - notificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny()), Times.Never); - } } diff --git a/OptimizelySDK.Tests/OptimizelyTest.cs b/OptimizelySDK.Tests/OptimizelyTest.cs index 69cd9913..6746459f 100644 --- a/OptimizelySDK.Tests/OptimizelyTest.cs +++ b/OptimizelySDK.Tests/OptimizelyTest.cs @@ -1640,95 +1640,6 @@ public void TestTrackListener(UserAttributes userAttributes, EventTags eventTags NotificationCallbackMock.Verify(nc => nc.TestTrackCallback(eventKey, TestUserId, userAttributes, eventTags, logEvent), Times.Exactly(1)); NotificationCallbackMock.Verify(nc => nc.TestAnotherTrackCallback(eventKey, TestUserId, userAttributes, eventTags, logEvent), Times.Exactly(1)); } - - [Test] - public void TestFeatureExperimentListener() - { - var featureKey = "boolean_feature"; - var featureFlag = Config.GetFeatureFlagFromKey(featureKey); - var experiment = Config.GetExperimentFromId(featureFlag.ExperimentIds[0]); - var variation = experiment.Variations[0]; - var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, - "POST", new Dictionary { }); - var userAttributes = new UserAttributes - { - { "device_type", "iPhone" }, - { "company", "Optimizely" }, - { "location", "San Francisco" } - }; - - // Mocking objects. - NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny())); - NotificationCallbackMock.Setup(nc => nc.TestFeatureExperimentCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny())); - EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); - DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); - - // Adding notification listeners. - NotificationCenter.AddNotification(NotificationCenter.NotificationType.Activate, - NotificationCallbackMock.Object.TestActivateCallback); - NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureExperiment, - NotificationCallbackMock.Object.TestFeatureExperimentCallback); - - var optly = Helper.CreatePrivateOptimizely(); - optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); - optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); - optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); - - // Calling IsFeatureEnabled. - optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); - - // Verify that both of the activate and feature experiment callbacks are called in case of feature experiment. - NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Exactly(1)); - NotificationCallbackMock.Verify(nc => nc.TestFeatureExperimentCallback(featureKey, TestUserId, userAttributes, experiment, variation), Times.Exactly(1)); - } - - [Test] - public void TestFeatureRolloutListener() - { - var featureKey = "boolean_single_variable_feature"; - var featureFlag = Config.GetFeatureFlagFromKey(featureKey); - var rollout = Config.GetRolloutFromId(featureFlag.RolloutId); - var experiment = rollout.Experiments[0]; - var variation = experiment.Variations[0]; - var logEvent = new LogEvent("https://logx.optimizely.com/v1/events", OptimizelyHelper.SingleParameter, - "POST", new Dictionary { }); - var userAttributes = new UserAttributes - { - { "device_type", "iPhone" }, - { "company", "Optimizely" }, - { "location", "San Francisco" } - }; - - // Mocking objects. - NotificationCallbackMock.Setup(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny())); - NotificationCallbackMock.Setup(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())); - EventBuilderMock.Setup(ebm => ebm.CreateImpressionEvent(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny(), It.IsAny())).Returns(logEvent); - DecisionServiceMock.Setup(ds => ds.GetVariationForFeature(featureFlag, TestUserId, userAttributes)).Returns(variation); - - // Adding notification listeners. - NotificationCenter.AddNotification(NotificationCenter.NotificationType.Activate, - NotificationCallbackMock.Object.TestActivateCallback); - NotificationCenter.AddNotification(NotificationCenter.NotificationType.FeatureRollout, - NotificationCallbackMock.Object.TestFeatureRolloutCallback); - - var optly = Helper.CreatePrivateOptimizely(); - optly.SetFieldOrProperty("NotificationCenter", NotificationCenter); - optly.SetFieldOrProperty("DecisionService", DecisionServiceMock.Object); - optly.SetFieldOrProperty("EventBuilder", EventBuilderMock.Object); - - // Calling IsFeatureEnabled. - optly.Invoke("IsFeatureEnabled", featureKey, TestUserId, userAttributes); - - // Verify that only feature rollout callback gets called in case of rollout rule. - NotificationCallbackMock.Verify(nc => nc.TestFeatureRolloutCallback(featureKey, TestUserId, userAttributes, It.IsAny()), Times.Exactly(1)); - NotificationCallbackMock.Verify(nc => nc.TestActivateCallback(experiment, TestUserId, userAttributes, variation, logEvent), Times.Never); - } #endregion // Test NotificationCenter } diff --git a/OptimizelySDK/Notifications/NotificationCenter.cs b/OptimizelySDK/Notifications/NotificationCenter.cs index 2eb8a69e..bdd6e7dc 100644 --- a/OptimizelySDK/Notifications/NotificationCenter.cs +++ b/OptimizelySDK/Notifications/NotificationCenter.cs @@ -34,9 +34,7 @@ public class NotificationCenter public enum NotificationType { Activate, // Activate called. - Track, // Event tracked. - FeatureExperiment, // Feature accessed from experiment. - FeatureRollout // Feature accessed from rollout rule. + Track }; /// @@ -61,27 +59,6 @@ public delegate void ActivateCallback(Experiment experiment, string userId, User public delegate void TrackCallback(string eventKey, string userId, UserAttributes userAttributes, EventTags eventTags, LogEvent logEvent); - /// - /// Delegate for feature experiment notifcations. - /// - /// The feature key - /// The user identifier - /// Associative array of attributes for the user - /// The experiment entity - /// The variation entity - public delegate void FeatureExperimentCallback(string featureKey, string userId, UserAttributes userAttributes, - Experiment experiment, Variation variation); - - /// - /// Delegate for feature rollout notifcations. - /// - /// The feature key - /// The user identifier - /// Associative array of attributes for the user - /// Array of audience - public delegate void FeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, - Audience[] audiences); - private ILogger Logger; // Notification Id represeting number of notifications. @@ -152,35 +129,6 @@ public int AddNotification(NotificationType notificationType, TrackCallback trac return AddNotification(notificationType, (object)trackCallback); } - /// - /// Add a notification callback of feature experiment type to the notification center. - /// - /// Notification type - /// Callback function to call when event gets triggered - /// int | 0 for invalid notification type, -1 for adding existing notification - /// or the notification id of newly added notification. - public int AddNotification(NotificationType notificationType, FeatureExperimentCallback featureExperimentCallback) - { - if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureExperiment)) - return 0; - - return AddNotification(notificationType, (object)featureExperimentCallback); - } - - /// - /// Add a notification callback of feature rollout type to the notification center. - /// - /// Notification type - /// Callback function to call when event gets triggered - /// int | 0 for invalid notification type, -1 for adding existing notification - /// or the notification id of newly added notification. - public int AddNotification(NotificationType notificationType, FeatureRolloutCallback featureRolloutCallback) - { - if (!IsNotificationTypeValid(notificationType, NotificationType.FeatureRollout)) - return 0; - - return AddNotification(notificationType, (object)featureRolloutCallback); - } /// /// Validate notification type. diff --git a/OptimizelySDK/Optimizely.cs b/OptimizelySDK/Optimizely.cs index 9974a4b1..4a510238 100644 --- a/OptimizelySDK/Optimizely.cs +++ b/OptimizelySDK/Optimizely.cs @@ -367,8 +367,6 @@ public Variation GetForcedVariation(string experimentKey, string userId) if (!string.IsNullOrEmpty(experiment.Key)) { SendImpressionEvent(experiment, variation, userId, userAttributes); - NotificationCenter.SendNotifications(NotificationCenter.NotificationType.FeatureExperiment, featureKey, userId, - userAttributes, experiment, variation); } else { @@ -382,8 +380,6 @@ public Variation GetForcedVariation(string experimentKey, string userId) audiences[0] = Config.GetAudience(rolloutRule.AudienceIds[0]); } - NotificationCenter.SendNotifications(NotificationCenter.NotificationType.FeatureRollout, featureKey, userId, - userAttributes, audiences); Logger.Log(LogLevel.INFO, $@"The user ""{userId}"" is not being experimented on feature ""{featureKey}""."); } From 4b096d116d00e04ccdfcee6131f377da6603e3a3 Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Mon, 27 Nov 2017 13:04:09 -0800 Subject: [PATCH 6/7] removed feature experiment and feature rollback callbacks. --- .../NotificationTests/NotificationCenterTests.cs | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index e3bc39fe..7fdd3d56 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -166,8 +166,6 @@ public void TestSendNotifications() notificationCallbackMock.Setup(nc => nc.TestAnotherActivateCallback(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny())); - notificationCallbackMock.Setup(nc => nc.TestFeatureRolloutCallback(It.IsAny(), It.IsAny(), - It.IsAny(), It.IsAny())); // Adding decision notifications. NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestActivateCallback); @@ -179,7 +177,7 @@ public void TestSendNotifications() // Fire decision type notifications. NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), - "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), null); + "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), LoggerMock.Object); // Verify that only the registered notifications of decision type are called. notificationCallbackMock.Verify(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(), @@ -237,15 +235,6 @@ public virtual void TestTrackCallback(string eventKey, string userId, UserAttrib public virtual void TestAnotherTrackCallback(string eventKey, string userId, UserAttributes userAttributes, EventTags eventTags, LogEvent logEvent) { } - - public virtual void TestFeatureExperimentCallback(string featureKey, string userId, UserAttributes userAttributes, - Experiment experiment, Variation variation) { - } - - public virtual void TestFeatureRolloutCallback(string featureKey, string userId, UserAttributes userAttributes, - Audience[] audiences) - { - } } #endregion // Test Notification callbacks class. } From 0e80f49306aecfd798543bbf0817733f487b55bc Mon Sep 17 00:00:00 2001 From: Sohail Hussain Date: Mon, 27 Nov 2017 13:26:47 -0800 Subject: [PATCH 7/7] fixed unit test issue. --- .../NotificationTests/NotificationCenterTests.cs | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs index 7fdd3d56..20eea771 100644 --- a/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs +++ b/OptimizelySDK.Tests/NotificationTests/NotificationCenterTests.cs @@ -21,6 +21,7 @@ using OptimizelySDK.Event; using OptimizelySDK.Logger; using OptimizelySDK.Notifications; +using System.Collections.Generic; using NotificationType = OptimizelySDK.Notifications.NotificationCenter.NotificationType; namespace OptimizelySDK.Tests.NotificationTests @@ -156,7 +157,7 @@ public void TestClearAllNotifications() public void TestSendNotifications() { var config = ProjectConfig.Create(TestData.Datafile, LoggerMock.Object, new NoOpErrorHandler()); - + var logEventMocker = new Mock("http://mockedurl", new Dictionary(), "POST", new Dictionary()); // Mocking notification callbacks. var notificationCallbackMock = new Mock(); @@ -170,14 +171,14 @@ public void TestSendNotifications() // Adding decision notifications. NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestActivateCallback); NotificationCenter.AddNotification(NotificationTypeActivate, notificationCallbackMock.Object.TestAnotherActivateCallback); - + // Adding track notifications. NotificationCenter.AddNotification(NotificationTypeTrack, notificationCallbackMock.Object.TestTrackCallback); // Fire decision type notifications. NotificationCenter.SendNotifications(NotificationTypeActivate, config.GetExperimentFromKey("test_experiment"), - "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), LoggerMock.Object); + "testUser", new UserAttributes(), config.GetVariationFromId("test_experiment", "7722370027"), logEventMocker.Object); // Verify that only the registered notifications of decision type are called. notificationCallbackMock.Verify(nc => nc.TestActivateCallback(It.IsAny(), It.IsAny(),