Update DecisionService to return a source for feature variation (#153)

Author: elliotykim

core-api/src/main/java/com/optimizely/ab/Optimizely.java

@@ -18,6 +18,7 @@
 import com.optimizely.ab.annotations.VisibleForTesting;
 import com.optimizely.ab.bucketing.Bucketer;
 import com.optimizely.ab.bucketing.DecisionService;
+import com.optimizely.ab.bucketing.FeatureDecision;
 import com.optimizely.ab.bucketing.UserProfileService;
 import com.optimizely.ab.config.Attribute;
 import com.optimizely.ab.config.EventType;
@@ -41,19 +42,21 @@
 import com.optimizely.ab.internal.EventTagUtils;
 import com.optimizely.ab.notification.NotificationBroadcaster;
 import com.optimizely.ab.notification.NotificationListener;
+
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import javax.annotation.CheckForNull;
-import javax.annotation.Nonnull;
-import javax.annotation.Nullable;
-import javax.annotation.concurrent.ThreadSafe;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
 
+import javax.annotation.CheckForNull;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import javax.annotation.concurrent.ThreadSafe;
+
 /**
  * Top-level container class for Optimizely functionality.
  * Thread-safe, so can be created as a singleton and safely passed around.
@@ -324,34 +327,29 @@ public void track(@Nonnull String eventName,
                                               @Nonnull Map<String, String> attributes) {
         FeatureFlag featureFlag = projectConfig.getFeatureKeyMapping().get(featureKey);
         if (featureFlag == null) {
-            logger.info("No feature flag was found for key \"" + featureKey + "\".");
+            logger.info("No feature flag was found for key \"{}\".", featureKey);
             return false;
         }
 
         Map<String, String> filteredAttributes = filterAttributes(projectConfig, attributes);
 
-        Variation variation = decisionService.getVariationForFeature(featureFlag, userId, filteredAttributes);
-
-        if (variation != null) {
-            Experiment experiment = projectConfig.getExperimentForVariationId(variation.getId());
-            if (experiment != null) {
-                // the user is in an experiment for the feature
+        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, filteredAttributes);
+        if (featureDecision.variation != null) {
+            if (featureDecision.decisionSource.equals(FeatureDecision.DecisionSource.EXPERIMENT)) {
                 sendImpression(
                         projectConfig,
-                        experiment,
+                        featureDecision.experiment,
                         userId,
                         filteredAttributes,
-                        variation);
-            }
-            else {
-                logger.info("The user \"" + userId +
-                        "\" is not being experimented on in feature \"" + featureKey + "\".");
+                        featureDecision.variation);
+            } else {
+                logger.info("The user \"{}\" is not included in an experiment for feature \"{}\".",
+                        userId, featureKey);
             }
-            logger.info("Feature \"" + featureKey + "\" is enabled for user \"" + userId + "\".");
+            logger.info("Feature \"{}\" is enabled for user \"{}\".", featureKey, userId);
             return true;
-        }
-        else {
-            logger.info("Feature \"" + featureKey + "\" is not enabled for user \"" + userId + "\".");
+        } else {
+            logger.info("Feature \"{}\" is not enabled for user \"{}\".", featureKey, userId);
             return false;
         }
     }
@@ -433,10 +431,9 @@ public void track(@Nonnull String eventName,
         if (variableValue != null) {
             try {
                 return Double.parseDouble(variableValue);
-            }
-            catch (NumberFormatException exception) {
+            } catch (NumberFormatException exception) {
                 logger.error("NumberFormatException while trying to parse \"" + variableValue +
-                "\" as Double. " + exception);
+                        "\" as Double. " + exception);
             }
         }
         return null;
@@ -479,10 +476,9 @@ public void track(@Nonnull String eventName,
         if (variableValue != null) {
             try {
                 return Integer.parseInt(variableValue);
-            }
-            catch (NumberFormatException exception) {
+            } catch (NumberFormatException exception) {
                 logger.error("NumberFormatException while trying to parse \"" + variableValue +
-                "\" as Integer. " + exception.toString());
+                        "\" as Integer. " + exception.toString());
             }
         }
         return null;
@@ -531,17 +527,16 @@ String getFeatureVariableValueForType(@Nonnull String featureKey,
                                                   @Nonnull LiveVariable.VariableType variableType) {
         FeatureFlag featureFlag = projectConfig.getFeatureKeyMapping().get(featureKey);
         if (featureFlag == null) {
-            logger.info("No feature flag was found for key \"" + featureKey + "\".");
+            logger.info("No feature flag was found for key \"{}\".", featureKey);
             return null;
         }
 
         LiveVariable variable = featureFlag.getVariableKeyToLiveVariableMap().get(variableKey);
-        if (variable ==  null) {
-            logger.info("No feature variable was found for key \"" + variableKey + "\" in feature flag \"" +
-                    featureKey + "\".");
+        if (variable == null) {
+            logger.info("No feature variable was found for key \"{}\" in feature flag \"{}\".",
+                    variableKey, featureKey);
             return null;
-        }
-        else if (!variable.getType().equals(variableType)) {
+        } else if (!variable.getType().equals(variableType)) {
             logger.info("The feature variable \"" + variableKey +
                     "\" is actually of type \"" + variable.getType().toString() +
                     "\" type. You tried to access it as type \"" + variableType.toString() +
@@ -551,23 +546,19 @@ else if (!variable.getType().equals(variableType)) {
 
         String variableValue = variable.getDefaultValue();
 
-        Variation variation = decisionService.getVariationForFeature(featureFlag, userId, attributes);
-
-        if (variation != null) {
+        FeatureDecision featureDecision = decisionService.getVariationForFeature(featureFlag, userId, attributes);
+        if (featureDecision.variation != null) {
             LiveVariableUsageInstance liveVariableUsageInstance =
-                    variation.getVariableIdToLiveVariableUsageInstanceMap().get(variable.getId());
+                    featureDecision.variation.getVariableIdToLiveVariableUsageInstanceMap().get(variable.getId());
             if (liveVariableUsageInstance != null) {
                 variableValue = liveVariableUsageInstance.getValue();
-            }
-            else {
+            } else {
                 variableValue = variable.getDefaultValue();
             }
-        }
-        else {
-            logger.info("User \"" + userId +
-                    "\" was not bucketed into any variation for feature flag \"" + featureKey +
-                    "\". The default value \"" + variableValue +
-                    "\" for \"" + variableKey + "\" is being returned."
+        } else {
+            logger.info("User \"{}\" was not bucketed into any variation for feature flag \"{}\". " +
+                            "The default value \"{}\" for \"{}\" is being returned.",
+                    userId, featureKey, variableValue, variableKey
             );
         }