feature(API): Adds input validation in all API methods and validates empty user Id. (#104)

Author: mfahadahmed

OptimizelySDK.Tests/OptimizelyTest.cs

@@ -943,6 +943,34 @@ public void TestSetForcedVariation()
             Assert.IsTrue(TestData.CompareObjects(VariationWithKeyVariation, actualForcedVariation));
         }
 
+        [Test]
+        public void TestSetForcedVariationWithNullAndEmptyUserId()
+        {
+            Assert.False(Optimizely.SetForcedVariation("test_experiment", null, "variation"));
+            Assert.True(Optimizely.SetForcedVariation("test_experiment", "", "variation"));
+        }
+
+        [Test]
+        public void TestSetForcedVariationWithInvalidExperimentKey()
+        {
+            var userId = "test_user";
+            var variation = "variation";
+
+            Assert.False(Optimizely.SetForcedVariation("test_experiment_not_in_datafile", userId, variation));
+            Assert.False(Optimizely.SetForcedVariation("", userId, variation));
+            Assert.False(Optimizely.SetForcedVariation(null, userId, variation));
+        }
+
+        [Test]
+        public void TestSetForcedVariationWithInvalidVariationKey()
+        {
+            var userId = "test_user";
+            var experimentKey = "test_experiment";
+
+            Assert.False(Optimizely.SetForcedVariation(experimentKey, userId, "variation_not_in_datafile"));
+            Assert.False(Optimizely.SetForcedVariation(experimentKey, userId, ""));
+        }
+
         // check that the get forced variation is correct.
         [Test]
         public void TestGetForcedVariation()
@@ -986,6 +1014,28 @@ public void TestGetForcedVariation()
             Assert.IsTrue(TestData.CompareObjects(expectedForcedVariation, actualForcedVariation));
         }
 
+        [Test]
+        public void TestGetForcedVariationWithInvalidUserID()
+        {
+            var experimentKey = "test_experiment";
+            Optimizely.SetForcedVariation(experimentKey, "test_user", "test_variation");
+
+            Assert.Null(Optimizely.GetForcedVariation(experimentKey, null));
+            Assert.Null(Optimizely.GetForcedVariation(experimentKey, "invalid_user"));
+        }
+
+        [Test]
+        public void TestGetForcedVariationWithInvalidExperimentKey()
+        {
+            var userId = "test_user";
+            var experimentKey = "test_experiment";
+            Optimizely.SetForcedVariation(experimentKey, userId, "test_variation");
+            
+            Assert.Null(Optimizely.GetForcedVariation("test_experiment", userId));
+            Assert.Null(Optimizely.GetForcedVariation("", userId));
+            Assert.Null(Optimizely.GetForcedVariation(null, userId));
+        }
+
         [Test]
         public void TestGetVariationAudienceMatchAfterSetForcedVariation()
         {
@@ -1325,9 +1375,9 @@ public void TestGetFeatureVariableValueForTypeGivenNullOrEmptyArguments()
             Assert.IsNull(Optimizely.GetFeatureVariableValueForType(featureKey, variableKey, null, null, variableType));
             Assert.IsNull(Optimizely.GetFeatureVariableValueForType(featureKey, variableKey, "", null, variableType));
 
-            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Feature flag key must not be empty."), Times.Exactly(2));
-            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Variable key must not be empty."), Times.Exactly(2));
-            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "User ID must not be empty."), Times.Exactly(2));
+            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Provided Feature Key is in invalid format."), Times.Exactly(2));
+            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Provided Variable Key is in invalid format."), Times.Exactly(2));
+            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Provided User Id is in invalid format."), Times.Exactly(1));
         }
 
         // Should return null and log error message when feature key or variable key does not get found.
@@ -1487,8 +1537,8 @@ public void TestIsFeatureEnabledGivenNullOrEmptyArguments()
             Assert.IsFalse(Optimizely.IsFeatureEnabled(null, TestUserId, null));
             Assert.IsFalse(Optimizely.IsFeatureEnabled("", TestUserId, null));
 
-            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "User ID must not be empty."), Times.Exactly(2));
-            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Feature flag key must not be empty."), Times.Exactly(2));
+            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Provided Feature Key is in invalid format."), Times.Exactly(2));
+            LoggerMock.Verify(l => l.Log(LogLevel.ERROR, "Provided User Id is in invalid format."), Times.Exactly(1));
         }
 
         // Should return false and log error message when feature flag key is not found in the datafile.
@@ -1941,5 +1991,48 @@ public void TestTrackValidateInputValues()
         }
 
         #endregion // Test ValidateStringInputs
+
+        #region TestValidateStringInputs
+
+        [Test]
+        public void TestValidateStringInputsWithValidValues()
+        {
+            var optly = Helper.CreatePrivateOptimizely();
+
+            bool result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.EXPERIMENT_KEY, "test_experiment" } });
+            Assert.True(result);
+
+            result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.EVENT_KEY, "buy_now_event" } });
+            Assert.True(result);
+        }
+
+        [Test]
+        public void TestValidateStringInputsWithInvalidValues()
+        {
+            var optly = Helper.CreatePrivateOptimizely();
+
+            bool result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.EXPERIMENT_KEY, "" } });
+            Assert.False(result);
+
+            result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.EVENT_KEY, null } });
+            Assert.False(result);
+        }
+
+        [Test]
+        public void TestValidateStringInputsWithUserId()
+        {
+            var optly = Helper.CreatePrivateOptimizely();
+
+            bool result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.USER_ID, "testUser" } });
+            Assert.True(result);
+
+            result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.USER_ID, "" } });
+            Assert.True(result);
+
+            result = (bool)optly.Invoke("ValidateStringInputs", new Dictionary<string, string> { { Optimizely.USER_ID, null } });
+            Assert.False(result);
+        }
+
+        #endregion //TestValidateStringInputs
     }
 }

OptimizelySDK.Tests/ProjectConfigTest.cs

@@ -723,65 +723,6 @@ public void TestGetForcedVariationLogs()
             LoggerMock.Verify(l => l.Log(LogLevel.DEBUG, string.Format(@"Variation ""{0}"" is mapped to experiment ""{1}"" and user ""{2}"" in the forced variation map", variationKey, experimentKey, userId)));
         }
 
-        [Test]
-        public void TestGetForcedVariationWithInvalidUserID()
-        {
-            var experimentKey = "test_experiment";
-
-            Config.SetForcedVariation(experimentKey, "test_user", "test_variation");
-
-            Assert.Null(Config.GetForcedVariation(experimentKey, null));
-            Assert.Null(Config.GetForcedVariation(experimentKey, ""));
-            Assert.Null(Config.GetForcedVariation(experimentKey, "invalid_user"));
-        }
-
-        [Test]
-        public void TestGetForcedVariationWithInvalidExperimentKey()
-        {
-            var userId = "test_user";
-            var experimentKey = "test_experiment";
-
-            Config.SetForcedVariation(experimentKey, userId, "test_variation");
-
-
-
-            Assert.Null(Config.GetForcedVariation("test_experiment", userId));
-            Assert.Null(Config.GetForcedVariation("", userId));
-            Assert.Null(Config.GetForcedVariation(null, userId));
-        }
-
-        [Test]
-        public void TestSetForcedVariationWithInvalidUserID()
-        {
-            var experimentKey = "test_experiment";
-            var variation = "variation";
-
-            Assert.False(Config.SetForcedVariation(experimentKey, null, variation));
-            Assert.False(Config.SetForcedVariation(experimentKey, "", variation));
-        }
-
-        [Test]
-        public void TestSetForcedVariationWithInvalidExperimentKey()
-        {
-            var userId = "test_user";
-            var variation = "variation";
-
-            Assert.False(Config.SetForcedVariation("test_experiment_not_in_datafile", userId, variation));
-            Assert.False(Config.SetForcedVariation("", userId, variation));
-            Assert.False(Config.SetForcedVariation(null, userId, variation));
-        }
-
-        [Test]
-        public void TestSetForcedVariationWithInvalidVariationKey()
-        {
-            var userId = "test_user";
-            var experimentKey = "test_experiment";
-
-            Assert.False(Config.SetForcedVariation(experimentKey, userId, "variation_not_in_datafile"));
-            Assert.False(Config.SetForcedVariation(experimentKey, userId, ""));
-            Assert.True(Config.SetForcedVariation(experimentKey, userId, null));
-        }
-        
         [Test]
         public void TestSetForcedVariationMultipleSets()
         {

OptimizelySDK/Optimizely.cs

@@ -75,6 +75,8 @@ public static String SDK_TYPE {
         public const string USER_ID = "User Id";
         public const string EXPERIMENT_KEY = "Experiment Key";
         public const string EVENT_KEY = "Event Key";
+        public const string FEATURE_KEY = "Feature Key";
+        public const string VARIABLE_KEY = "Variable Key";
 
         /// <summary>
         /// Optimizely constructor for managing Full Stack .NET projects.
@@ -337,7 +339,13 @@ public Variation GetVariation(string experimentKey, string userId, UserAttribute
         /// <returns>A boolean value that indicates if the set completed successfully.</returns>
         public bool SetForcedVariation(string experimentKey, string userId, string variationKey)
         {
-            return Config.SetForcedVariation(experimentKey, userId, variationKey);
+            var inputValues = new Dictionary<string, string>
+            {
+                { USER_ID, userId },
+                { EXPERIMENT_KEY, experimentKey }
+            };
+
+            return ValidateStringInputs(inputValues) && Config.SetForcedVariation(experimentKey, userId, variationKey);
         }
 
         /// <summary>
@@ -348,9 +356,16 @@ public bool SetForcedVariation(string experimentKey, string userId, string varia
         /// <returns>null|string The variation key.</returns>
         public Variation GetForcedVariation(string experimentKey, string userId)
         {
-            var forcedVariation = Config.GetForcedVariation(experimentKey, userId);
+            var inputValues = new Dictionary<string, string>
+            {
+                { USER_ID, userId },
+                { EXPERIMENT_KEY, experimentKey }
+            };
 
-            return forcedVariation;
+            if (!ValidateStringInputs(inputValues))
+                return null;
+
+            return Config.GetForcedVariation(experimentKey, userId);
         }
 
         #region  FeatureFlag APIs
@@ -365,15 +380,14 @@ public Variation GetForcedVariation(string experimentKey, string userId)
         /// <returns>True if feature is enabled, false or null otherwise</returns>
         public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttributes userAttributes = null)
         {
-            if (string.IsNullOrEmpty(userId)) {
-                Logger.Log(LogLevel.ERROR, "User ID must not be empty.");
-                return false;
-            }
+            var inputValues = new Dictionary<string, string>
+            {
+                { USER_ID, userId },
+                { FEATURE_KEY, featureKey }
+            };
 
-            if (string.IsNullOrEmpty(featureKey)) {
-                Logger.Log(LogLevel.ERROR, "Feature flag key must not be empty.");
+            if (!ValidateStringInputs(inputValues))
                 return false;
-            }
 
             var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
             if (string.IsNullOrEmpty(featureFlag.Key))
@@ -395,7 +409,6 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
                 }
             }
 
-
             Logger.Log(LogLevel.INFO, $@"Feature flag ""{featureKey}"" is not enabled for user ""{userId}"".");
             return false;
         }
@@ -412,23 +425,15 @@ public virtual bool IsFeatureEnabled(string featureKey, string userId, UserAttri
         public virtual string GetFeatureVariableValueForType(string featureKey, string variableKey, string userId, 
             UserAttributes userAttributes, FeatureVariable.VariableType variableType)
         {
-            if (string.IsNullOrEmpty(featureKey))
-            {
-                Logger.Log(LogLevel.ERROR, "Feature flag key must not be empty.");
-                return null;
-            }
-
-            if (string.IsNullOrEmpty(variableKey))
+            var inputValues = new Dictionary<string, string>
             {
-                Logger.Log(LogLevel.ERROR, "Variable key must not be empty.");
-                return null;
-            }
+                { USER_ID, userId },
+                { FEATURE_KEY, featureKey },
+                { VARIABLE_KEY, variableKey }
+            };
 
-            if (string.IsNullOrEmpty(userId))
-            {
-                Logger.Log(LogLevel.ERROR, "User ID must not be empty.");
+            if (!ValidateStringInputs(inputValues))
                 return null;
-            }
 
             var featureFlag = Config.GetFeatureFlagFromKey(featureKey);
             if (string.IsNullOrEmpty(featureFlag.Key))
@@ -614,6 +619,9 @@ public List<string> GetEnabledFeatures(string userId, UserAttributes userAttribu
                 return enabledFeaturesList;
             }
 
+            if (!ValidateStringInputs(new Dictionary<string, string> { { USER_ID, userId } }))
+                return enabledFeaturesList;
+
             foreach (var feature in Config.FeatureKeyMap.Values)
             {
                 var featureKey = feature.Key;
@@ -634,6 +642,19 @@ public List<string> GetEnabledFeatures(string userId, UserAttributes userAttribu
         private bool ValidateStringInputs(Dictionary<string, string> inputs)
         {
             bool isValid = true;
+
+            // Empty user Id is valid value.
+            if (inputs.ContainsKey(USER_ID))
+            {
+                if (inputs[USER_ID] == null)
+                {
+                    Logger.Log(LogLevel.ERROR, $"Provided {USER_ID} is in invalid format.");
+                    isValid = false;
+                }
+
+                inputs.Remove(USER_ID);
+            }
+
             foreach(var input in inputs)
             {
                 if (string.IsNullOrEmpty(input.Value))

OptimizelySDK/ProjectConfig.cs

@@ -458,12 +458,6 @@ public Variation GetVariationFromId(string experimentKey, string variationId)
         /// <returns>Variation entity which the given user and experiment should be forced into.</returns>
         public Variation GetForcedVariation(string experimentKey, string userId)
         {
-            if (string.IsNullOrEmpty(userId))
-            {
-                Logger.Log(LogLevel.DEBUG, "User ID is invalid.");
-                return null;
-            }
-
             if (_ForcedVariationMap.ContainsKey(userId) == false)
             {
                 Logger.Log(LogLevel.DEBUG, string.Format(@"User ""{0}"" is not in the forced variation map.", userId));
@@ -514,19 +508,7 @@ public Variation GetForcedVariation(string experimentKey, string userId)
         /// <returns>A boolean value that indicates if the set completed successfully.</returns>
         public bool SetForcedVariation(string experimentKey, string userId, string variationKey)
         {
-            if (string.IsNullOrEmpty(userId))
-            {
-                Logger.Log(LogLevel.DEBUG, "User ID is invalid.");
-                return false;
-            }
-
-            if (string.IsNullOrEmpty(experimentKey))
-            {
-                Logger.Log(LogLevel.DEBUG, "Experiment key is invalid.");
-                return false;
-            }
-
-            // Empty variation key is invalid.
+            // Empty variation key is considered as invalid.
             if (variationKey != null && variationKey.Length == 0)
             {
                 Logger.Log(LogLevel.DEBUG, "Variation key is invalid.");