Skip to content
Merged
4 changes: 0 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/Optimizely.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
21 changes: 17 additions & 4 deletions core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down Expand Up @@ -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;
}

Expand Down Expand Up @@ -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{" +
Expand Down
14 changes: 5 additions & 9 deletions core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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, ""));
}

/**
Expand Down Expand Up @@ -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
Expand All @@ -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(), ""));
}

/**
Expand Down Expand Up @@ -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());

}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 */
Expand Down