fix: Allowing Empty UserID as valid field (#220)

Author: mnoman09

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

@@ -819,10 +819,6 @@ private boolean validateUserId(String userId) {
  logger.error("The user ID parameter must be nonnull.");
  return false;
  }
- if (userId.trim().isEmpty()) {
- logger.error("Non-empty user ID required");
- return false;
- }
 
  return true;
  }

core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java

@@ -496,8 +496,7 @@ public boolean setForcedVariation(@Nonnull String experimentKey,
  }
 
  // if the user id is invalid, return false.
- if (userId == null || userId.trim().isEmpty()) {
- logger.error("User ID is invalid");
+ if (!validateUserId(userId)) {
  return false;
  }
 
@@ -554,8 +553,7 @@ public boolean setForcedVariation(@Nonnull String experimentKey,
  @Nonnull String userId) {
 
  // if the user id is invalid, return false.
- if (userId == null || userId.trim().isEmpty()) {
- logger.error("User ID is invalid");
+ if (!validateUserId(userId)) {
  return null;
  }
 
@@ -587,6 +585,21 @@ public boolean setForcedVariation(@Nonnull String experimentKey,
  return null;
  }
 
+ /**
+ * Helper function to check that the provided userId is valid
+ *
+ * @param userId the userId being validated
+ * @return whether the user ID is valid
+ */
+ private boolean validateUserId(String userId) {
+ if (userId == null) {
+ logger.error("User ID is invalid");
+ return false;
+ }
+
+ return true;
+ }
+
  @Override
  public String toString() {
  return "ProjectConfig{" +

core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java

@@ -1385,7 +1385,7 @@ public void activateUserNoAttributesWithAudiences() throws Exception {
  }
 
  /**
- * Verify that {@link Optimizely#activate(String, String)} doesn't return a variation when provided an empty string.
+ * Verify that {@link Optimizely#activate(String, String)} return a variation when provided an empty string.
  */
  @Test
  public void activateWithEmptyUserId() throws Exception {
@@ -1397,9 +1397,7 @@ public void activateWithEmptyUserId() throws Exception {
  .withErrorHandler(new RaiseExceptionErrorHandler())
  .build();
 
- logbackVerifier.expectMessage(Level.ERROR, "Non-empty user ID required");
- logbackVerifier.expectMessage(Level.INFO, "Not activating user for experiment \"" + experimentKey + "\".");
- assertNull(optimizely.activate(experimentKey, ""));
+ assertNotNull(optimizely.activate(experimentKey, ""));
  }
 
  /**
@@ -2695,7 +2693,7 @@ public void getVariationWithUnknownExperimentKeyAndRaiseExceptionErrorHandler()
  }
 
  /**
- * Verify that {@link Optimizely#getVariation(String, String)} doesn't return a variation when provided an
+ * Verify that {@link Optimizely#getVariation(String, String)} return a variation when provided an
  * empty string.
  */
  @Test
@@ -2707,8 +2705,7 @@ public void getVariationWithEmptyUserId() throws Exception {
  .withErrorHandler(new RaiseExceptionErrorHandler())
  .build();
 
- logbackVerifier.expectMessage(Level.ERROR, "Non-empty user ID required");
- assertNull(optimizely.getVariation(experiment.getKey(), ""));
+ assertNotNull(optimizely.getVariation(experiment.getKey(), ""));
  }
 
  /**
@@ -4049,8 +4046,7 @@ public void getEnabledFeatureWithEmptyUserId() throws ConfigParseException{
  .build());
  ArrayList<String> featureFlags = (ArrayList<String>) spyOptimizely.getEnabledFeatures("",
  new HashMap<String, String>());
- logbackVerifier.expectMessage(Level.ERROR, "Non-empty user ID required");
- assertTrue(featureFlags.isEmpty());
+ assertFalse(featureFlags.isEmpty());
 
  }
 

core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java

@@ -31,11 +31,7 @@
 
 import static java.util.Arrays.asList;
 import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.*;
 
 
 import com.optimizely.ab.internal.LogbackVerifier;
@@ -244,12 +240,12 @@ public void getForcedVariationNullUserId() {
 
  @Test
  public void setForcedVariationEmptyUserId() {
- assertFalse(projectConfig.setForcedVariation("etag1", "", "vtag1"));
+ assertTrue(projectConfig.setForcedVariation("etag1", "", "vtag1"));
  }
 
  @Test
  public void getForcedVariationEmptyUserId() {
- assertNull(projectConfig.getForcedVariation("etag1", ""));
+ assertNotNull(projectConfig.getForcedVariation("etag1", ""));
  }
 
  /* Invalid Experiement */