Allow blank user key when evaluating. Extract client interface. No longer return null when expected type does not match variation from evaluation.

Author: Dan Richelson

src/main/java/com/launchdarkly/client/LDClient.java

@@ -11,7 +11,6 @@
 
 import javax.crypto.Mac;
 import javax.crypto.spec.SecretKeySpec;
-import java.io.Closeable;
 import java.io.IOException;
 import java.io.UnsupportedEncodingException;
 import java.net.URL;
@@ -30,7 +29,7 @@
  * a single {@code LDClient} for the lifetime of their application.
  */
 @ThreadSafe
-public class LDClient implements Closeable {
+public class LDClient implements LDClientInterface {
   private static final Logger logger = LoggerFactory.getLogger(LDClient.class);
   private static final String HMAC_ALGORITHM = "HmacSHA256";
   protected static final String CLIENT_VERSION = getClientVersion();
@@ -96,6 +95,7 @@ public LDClient(String sdkKey, LDConfig config) {
     }
   }
 
+  @Override
   public boolean initialized() {
     return isOffline() || config.useLdd || updateProcessor.initialized();
   }
@@ -128,6 +128,7 @@ protected PollingProcessor createPollingProcessor(LDConfig config) {
    * @param user      the user that performed the event
    * @param data      a JSON object containing additional data associated with the event
    */
+  @Override
   public void track(String eventName, LDUser user, JsonElement data) {
     if (isOffline()) {
       return;
@@ -147,6 +148,7 @@ public void track(String eventName, LDUser user, JsonElement data) {
    * @param eventName the name of the event
    * @param user      the user that performed the event
    */
+  @Override
   public void track(String eventName, LDUser user) {
     if (isOffline()) {
       return;
@@ -159,6 +161,7 @@ public void track(String eventName, LDUser user) {
    *
    * @param user the user to register
    */
+  @Override
   public void identify(LDUser user) {
     if (isOffline()) {
       return;
@@ -194,6 +197,7 @@ private void sendFlagRequestEvent(String featureKey, LDUser user, JsonElement va
    * @param user the end user requesting the feature flags
    * @return a map from feature flag keys to {@code JsonElement} for the specified user
    */
+  @Override
   public Map<String, JsonElement> allFlags(LDUser user) {
     if (isOffline()) {
       logger.warn("allFlags() was called when client is in offline mode! Returning null.");
@@ -233,17 +237,16 @@ public Map<String, JsonElement> allFlags(LDUser user) {
    * @param defaultValue the default value of the flag
    * @return whether or not the flag should be enabled, or {@code defaultValue} if the flag is disabled in the LaunchDarkly control panel
    */
+  @Override
   public boolean boolVariation(String featureKey, LDUser user, boolean defaultValue) {
-    JsonElement value = jsonVariation(featureKey, user, new JsonPrimitive(defaultValue));
-    if (value.isJsonPrimitive() && value.getAsJsonPrimitive().isBoolean()) {
-      return value.getAsJsonPrimitive().getAsBoolean();
-    }
-    return false;
+    JsonElement value = evaluate(featureKey, user, new JsonPrimitive(defaultValue), VariationType.Boolean);
+    return value.getAsJsonPrimitive().getAsBoolean();
   }
 
   /**
    * @deprecated use {@link #boolVariation(String, LDUser, boolean)}
    */
+  @Override
   @Deprecated
   public boolean toggle(String featureKey, LDUser user, boolean defaultValue) {
     logger.warn("Deprecated method: Toggle() called. Use boolVariation() instead.");
@@ -258,12 +261,10 @@ public boolean toggle(String featureKey, LDUser user, boolean defaultValue) {
    * @param defaultValue the default value of the flag
    * @return the variation for the given user, or {@code defaultValue} if the flag is disabled in the LaunchDarkly control panel
    */
+  @Override
   public Integer intVariation(String featureKey, LDUser user, int defaultValue) {
-    JsonElement value = jsonVariation(featureKey, user, new JsonPrimitive(defaultValue));
-    if (value.isJsonPrimitive() && value.getAsJsonPrimitive().isNumber()) {
-      return value.getAsJsonPrimitive().getAsInt();
-    }
-    return null;
+    JsonElement value = evaluate(featureKey, user, new JsonPrimitive(defaultValue), VariationType.Integer);
+    return value.getAsJsonPrimitive().getAsInt();
   }
 
   /**
@@ -274,12 +275,10 @@ public Integer intVariation(String featureKey, LDUser user, int defaultValue) {
    * @param defaultValue the default value of the flag
    * @return the variation for the given user, or {@code defaultValue} if the flag is disabled in the LaunchDarkly control panel
    */
+  @Override
   public Double doubleVariation(String featureKey, LDUser user, Double defaultValue) {
-    JsonElement value = jsonVariation(featureKey, user, new JsonPrimitive(defaultValue));
-    if (value.isJsonPrimitive() && value.getAsJsonPrimitive().isNumber()) {
-      return value.getAsJsonPrimitive().getAsDouble();
-    }
-    return null;
+    JsonElement value = evaluate(featureKey, user, new JsonPrimitive(defaultValue), VariationType.Double);
+    return value.getAsJsonPrimitive().getAsDouble();
   }
 
   /**
@@ -290,12 +289,10 @@ public Double doubleVariation(String featureKey, LDUser user, Double defaultValu
    * @param defaultValue the default value of the flag
    * @return the variation for the given user, or {@code defaultValue} if the flag is disabled in the LaunchDarkly control panel
    */
+  @Override
   public String stringVariation(String featureKey, LDUser user, String defaultValue) {
-    JsonElement value = jsonVariation(featureKey, user, new JsonPrimitive(defaultValue));
-    if (value.isJsonPrimitive() && value.getAsJsonPrimitive().isString()) {
-      return value.getAsJsonPrimitive().getAsString();
-    }
-    return null;
+    JsonElement value = evaluate(featureKey, user, new JsonPrimitive(defaultValue), VariationType.String);
+    return value.getAsJsonPrimitive().getAsString();
   }
 
   /**
@@ -306,21 +303,21 @@ public String stringVariation(String featureKey, LDUser user, String defaultValu
    * @param defaultValue the default value of the flag
    * @return the variation for the given user, or {@code defaultValue} if the flag is disabled in the LaunchDarkly control panel
    */
+  @Override
   public JsonElement jsonVariation(String featureKey, LDUser user, JsonElement defaultValue) {
-    if (isOffline()) {
-      return defaultValue;
-    }
-    JsonElement value = evaluate(featureKey, user, defaultValue);
+    JsonElement value = evaluate(featureKey, user, defaultValue, VariationType.Json);
     return value;
   }
 
-  private JsonElement evaluate(String featureKey, LDUser user, JsonElement defaultValue) {
+  private JsonElement evaluate(String featureKey, LDUser user, JsonElement defaultValue, VariationType expectedType) {
     if (user == null || user.getKey() == null) {
       logger.warn("Null user or null user key when evaluating flag: " + featureKey + "; returning default value");
       sendFlagRequestEvent(featureKey, user, defaultValue, defaultValue, null);
       return defaultValue;
     }
-
+    if (user.getKeyAsString().isEmpty()) {
+      logger.warn("User key is blank. Flag evaluation will proceed, but the user will not be stored in LaunchDarkly");
+    }
     if (!initialized()) {
       logger.warn("Evaluation called before Client has been initialized for feature flag " + featureKey + "; returning default value");
       sendFlagRequestEvent(featureKey, user, defaultValue, defaultValue, null);
@@ -341,6 +338,7 @@ private JsonElement evaluate(String featureKey, LDUser user, JsonElement default
         }
       }
       if (evalResult.getValue() != null) {
+        expectedType.assertResultType(evalResult.getValue());
         sendFlagRequestEvent(featureKey, user, evalResult.getValue(), defaultValue, featureFlag.getVersion());
         return evalResult.getValue();
       }
@@ -369,13 +367,15 @@ public void close() throws IOException {
   /**
    * Flushes all pending events
    */
+  @Override
   public void flush() {
     this.eventProcessor.flush();
   }
 
   /**
    * @return whether the client is in offline mode
    */
+  @Override
   public boolean isOffline() {
     return config.offline;
   }
@@ -385,6 +385,7 @@ public boolean isOffline() {
    * @param user The User to be hashed along with the sdk key
    * @return the hash, or null if the hash could not be calculated.
      */
+  @Override
   public String secureModeHash(LDUser user) {
     if (user == null || user.getKey() == null) {
       return null;

src/main/java/com/launchdarkly/client/LDClientInterface.java

@@ -0,0 +1,41 @@
+package com.launchdarkly.client;
+
+import com.google.gson.JsonElement;
+
+import java.io.Closeable;
+import java.io.IOException;
+import java.util.Map;
+
+public interface LDClientInterface extends Closeable {
+  boolean initialized();
+
+  void track(String eventName, LDUser user, JsonElement data);
+
+  void track(String eventName, LDUser user);
+
+  void identify(LDUser user);
+
+  Map<String, JsonElement> allFlags(LDUser user);
+
+  boolean boolVariation(String featureKey, LDUser user, boolean defaultValue);
+
+  @Deprecated
+  boolean toggle(String featureKey, LDUser user, boolean defaultValue);
+
+  Integer intVariation(String featureKey, LDUser user, int defaultValue);
+
+  Double doubleVariation(String featureKey, LDUser user, Double defaultValue);
+
+  String stringVariation(String featureKey, LDUser user, String defaultValue);
+
+  JsonElement jsonVariation(String featureKey, LDUser user, JsonElement defaultValue);
+
+  @Override
+  void close() throws IOException;
+
+  void flush();
+
+  boolean isOffline();
+
+  String secureModeHash(LDUser user);
+}

src/main/java/com/launchdarkly/client/VariationType.java

@@ -0,0 +1,50 @@
+package com.launchdarkly.client;
+
+
+import com.google.gson.JsonElement;
+
+public enum VariationType {
+  Boolean {
+    @Override
+    void assertResultType(JsonElement result) throws EvaluationException {
+      if (result.isJsonPrimitive() && result.getAsJsonPrimitive().isBoolean()) {
+        return;
+      }
+      throw new EvaluationException("Feature flag evaluation expected result as boolean type, but got non-boolean type.");
+    }
+  },
+  Integer {
+    @Override
+    void assertResultType(JsonElement result) throws EvaluationException {
+      if (result.isJsonPrimitive() && result.getAsJsonPrimitive().isNumber()) {
+        return;
+      }
+      throw new EvaluationException("Feature flag evaluation expected result as number type, but got non-number type.");
+    }
+  },
+  Double {
+    @Override
+    void assertResultType(JsonElement result) throws EvaluationException {
+      if (result.isJsonPrimitive() && result.getAsJsonPrimitive().isNumber()) {
+        return;
+      }
+      throw new EvaluationException("Feature flag evaluation expected result as number type, but got non-number type.");
+    }
+  },
+  String {
+    @Override
+    void assertResultType(JsonElement result) throws EvaluationException {
+      if (result.isJsonPrimitive() && result.getAsJsonPrimitive().isString()) {
+        return;
+      }
+      throw new EvaluationException("Feature flag evaluation expected result as string type, but got non-string type.");
+    }
+  },
+  Json {
+    @Override
+    void assertResultType(JsonElement result) {
+    }
+  };
+
+  abstract void assertResultType(JsonElement result) throws EvaluationException;
+}

src/test/java/com/launchdarkly/client/LDClientTest.java

@@ -24,7 +24,7 @@ public class LDClientTest extends EasyMockSupport {
   private PollingProcessor pollingProcessor;
   private EventProcessor eventProcessor;
   private Future initFuture;
-  private LDClient client;
+  private LDClientInterface client;
 
   @Before
   public void before() {
@@ -346,7 +346,7 @@ public void testSecureModeHash() {
     LDConfig config = new LDConfig.Builder()
             .offline(true)
             .build();
-    LDClient client = new LDClient("secret", config);
+    LDClientInterface client = new LDClient("secret", config);
     LDUser user = new LDUser.Builder("Message").build();
     assertEquals("aa747c502a898200f9e4fa21bac68136f886a0e27aec70ba06daf2e2a5cb5597", client.secureModeHash(user));
   }
@@ -356,7 +356,7 @@ private void assertDefaultValueIsReturned() {
     assertEquals(true, result);
   }
 
-  private LDClient createMockClient(LDConfig config) {
+  private LDClientInterface createMockClient(LDConfig config) {
     return new LDClient("SDK_KEY", config) {
       @Override
       protected FeatureRequestor createFeatureRequestor(String sdkKey, LDConfig config) {