diff --git a/core-api/src/main/java/com/optimizely/ab/Optimizely.java b/core-api/src/main/java/com/optimizely/ab/Optimizely.java index 5e3ada52b..c7dda4454 100644 --- a/core-api/src/main/java/com/optimizely/ab/Optimizely.java +++ b/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; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java b/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java index 0eb22f23f..2d5fdcb07 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/ProjectConfig.java +++ b/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{" + diff --git a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java index aee52d4da..6f32af18e 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/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 featureFlags = (ArrayList) spyOptimizely.getEnabledFeatures("", new HashMap()); - logbackVerifier.expectMessage(Level.ERROR, "Non-empty user ID required"); - assertTrue(featureFlags.isEmpty()); + assertFalse(featureFlags.isEmpty()); } diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java index 6e48772c9..876410c5b 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTest.java +++ b/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 */