From e6a72c4b056883b23730b410607091d1fbe35a37 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Fri, 31 Aug 2018 10:22:05 -0700 Subject: [PATCH 01/32] update with some thoughts on how to implement new match types. --- .../ab/config/audience/AndCondition.java | 2 +- .../ab/config/audience/Condition.java | 2 +- .../ab/config/audience/MatchType.java | 120 ++++++++++++++++++ .../ab/config/audience/NotCondition.java | 2 +- .../ab/config/audience/OrCondition.java | 2 +- .../ab/config/audience/UserAttribute.java | 10 +- 6 files changed, 132 insertions(+), 6 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 375205e42..b215b6198 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -36,7 +36,7 @@ public List getConditions() { return conditions; } - public boolean evaluate(Map attributes) { + public Boolean evaluate(Map attributes) { for (Condition condition : conditions) { if (!condition.evaluate(attributes)) return false; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index c84f39aef..31b8fd242 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -23,5 +23,5 @@ */ public interface Condition { - boolean evaluate(Map attributes); + Boolean evaluate(Map attributes); } \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java new file mode 100644 index 000000000..e23e0c479 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -0,0 +1,120 @@ +package com.optimizely.ab.config.audience; + +import javax.annotation.Nonnegative; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +interface LeafMatcher { + public Boolean eval(Object otherValue); +} + +abstract class LeafMatch implements LeafMatcher { + T convert(Object o) { + try { + T rv = (T)o; + return rv; + } catch(java.lang.ClassCastException e) { + return null; + } + + + } +} + +class ExactMatch extends LeafMatch { + T value; + protected ExactMatch(T value) { + this.value = value; + } + + public @Nullable Boolean eval(Object otherValue) { + return value.equals(convert(otherValue)); + } +} + +class GTMatch extends LeafMatch { + Number value; + protected GTMatch(Number value) { + this.value = value; + } + + public @Nullable Boolean eval(Object otherValue) { + try { + return value.doubleValue() < + ((Number) convert(otherValue)).doubleValue(); + } + catch (Exception e) { + return null; + } + } +} + +class LTMatch extends LeafMatch { + Number value; + protected LTMatch(Number value) { + this.value = value; + } + + public @Nullable Boolean eval(Object otherValue) { + try { + return value.doubleValue() < + ((Number) convert(otherValue)).doubleValue(); + } + catch (Exception e) { + return null; + } + } +} + +public class MatchType { + + private String type; + private LeafMatcher matcher; + + public static MatchType getMatchType(String type, Object value) { + switch (type) { + case "exact": + if (value instanceof String) { + return new MatchType(type, new ExactMatch((String) value)); + } + else if (value instanceof Number) { + return new MatchType(type, new ExactMatch((Number) value)); + } + else if (value instanceof Boolean) { + return new MatchType(type, new ExactMatch((Boolean) value)); + } + case "gt": + if (value instanceof Number) { + return new MatchType(type, new GTMatch((Number) value)); + } + case "lt": + if (value instanceof Number) { + return new MatchType(type, new LTMatch((Number) value)); + } + case "custom_dimension": + if (value instanceof String) { + return new MatchType(type, new ExactMatch((String) value)); + } + default: + return null; + } + } + + + private MatchType(String type, LeafMatcher matcher) { + this.type = type; + this.matcher = matcher; + } + + public @Nonnull + LeafMatcher getMatcher() { + return matcher; + } + + @Override + public String toString() { + return type; + } + + +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java index 84b8c010e..f215a1db7 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java @@ -37,7 +37,7 @@ public Condition getCondition() { return condition; } - public boolean evaluate(Map attributes) { + public Boolean evaluate(Map attributes) { return !condition.evaluate(attributes); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index 52a53c952..c88f851ea 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -36,7 +36,7 @@ public List getConditions() { return conditions; } - public boolean evaluate(Map attributes) { + public Boolean evaluate(Map attributes) { for (Condition condition : conditions) { if (condition.evaluate(attributes)) return true; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index c8627a4a1..74b3809f6 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -49,13 +49,19 @@ public Object getValue() { return value; } - public boolean evaluate(Map attributes) { + public Boolean evaluate(Map attributes) { // Valid for primative types, but needs to change when a value is an object or an array Object userAttributeValue = attributes.get(name); if (value != null) { // if there is a value in the condition // check user attribute value is equal - return value.equals(userAttributeValue); + try { + return MatchType.getMatchType(type, value).getMatcher().eval(attributes.get(name)); + } + catch (NullPointerException np) { + return false; + } + } else if (userAttributeValue != null) { // if the datafile value is null but user has a value for this attribute // return false since null != nonnull From c77ab0b4a76a8bef7736cab03a868c06003f66b6 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 4 Sep 2018 13:03:35 -0700 Subject: [PATCH 02/32] add exists --- .../ab/config/audience/MatchType.java | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index e23e0c479..7eb44a5f2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -66,6 +66,22 @@ protected LTMatch(Number value) { } } +class ExistsMatch extends LeafMatch { + Object value; + protected ExistsMatch(Object value) { + this.value = value; + } + + public @Nullable Boolean eval(Object otherValue) { + try { + return otherValue != null; + } + catch (Exception e) { + return null; + } + } +} + public class MatchType { private String type; @@ -73,6 +89,8 @@ public class MatchType { public static MatchType getMatchType(String type, Object value) { switch (type) { + case "exists": + return new MatchType(type, new ExistsMatch(value)); case "exact": if (value instanceof String) { return new MatchType(type, new ExactMatch((String) value)); From a757dde2add3711957a40800938a083313cf5337 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 4 Sep 2018 15:16:31 -0700 Subject: [PATCH 03/32] get the lt gt logic correct --- .../java/com/optimizely/ab/config/audience/MatchType.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index 7eb44a5f2..25b5e358e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -40,8 +40,7 @@ protected GTMatch(Number value) { public @Nullable Boolean eval(Object otherValue) { try { - return value.doubleValue() < - ((Number) convert(otherValue)).doubleValue(); + return ((Number) convert(otherValue)).doubleValue() > value.doubleValue(); } catch (Exception e) { return null; @@ -57,8 +56,7 @@ protected LTMatch(Number value) { public @Nullable Boolean eval(Object otherValue) { try { - return value.doubleValue() < - ((Number) convert(otherValue)).doubleValue(); + return ((Number) convert(otherValue)).doubleValue() < value.doubleValue(); } catch (Exception e) { return null; From 42a4590ce2a9b8267722354f5619f6136a370dc9 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 4 Sep 2018 15:33:36 -0700 Subject: [PATCH 04/32] suppress unread field in 'exists' match type --- .../main/java/com/optimizely/ab/config/audience/MatchType.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index 25b5e358e..e559bc3af 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -1,5 +1,7 @@ package com.optimizely.ab.config.audience; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + import javax.annotation.Nonnegative; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -65,6 +67,7 @@ protected LTMatch(Number value) { } class ExistsMatch extends LeafMatch { + @SuppressFBWarnings("URF_UNREAD_FIELD") Object value; protected ExistsMatch(Object value) { this.value = value; From d63ed0bba4d426ae6ea1ffc4189566b7db658869 Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Tue, 4 Sep 2018 16:18:41 -0700 Subject: [PATCH 05/32] nullmatch, substringmatch implementaiton --- .../ab/config/audience/MatchType.java | 43 +++++++++++++++++-- 1 file changed, 39 insertions(+), 4 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index e559bc3af..33848e8b2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -18,8 +18,18 @@ T convert(Object o) { } catch(java.lang.ClassCastException e) { return null; } + } +} +class NullMatch extends LeafMatch { + @SuppressFBWarnings("URF_UNREAD_FIELD") + Object value; + protected NullMatch() { + this.value = null; + } + public @Nullable Boolean eval(Object otherValue) { + return null; } } @@ -83,6 +93,22 @@ protected ExistsMatch(Object value) { } } +class SubstringMatch extends LeafMatch { + String value; + protected SubstringMatch(String value) { + this.value = value; + } + + public @Nullable Boolean eval(Object otherValue) { + try { + return value.contains(convert(otherValue)); + } + catch (Exception e) { + return null; + } + } +} + public class MatchType { private String type; @@ -102,21 +128,32 @@ else if (value instanceof Number) { else if (value instanceof Boolean) { return new MatchType(type, new ExactMatch((Boolean) value)); } + break; + case "substring": + if (value instanceof String) { + return new MatchType(type, new SubstringMatch((String) value)); + } + break; case "gt": if (value instanceof Number) { return new MatchType(type, new GTMatch((Number) value)); } + break; case "lt": if (value instanceof Number) { return new MatchType(type, new LTMatch((Number) value)); } + break; case "custom_dimension": if (value instanceof String) { return new MatchType(type, new ExactMatch((String) value)); } + break; default: - return null; + return new MatchType(type, new NullMatch()); } + + return new MatchType(type, new NullMatch()); } @@ -134,6 +171,4 @@ LeafMatcher getMatcher() { public String toString() { return type; } - - -} +} \ No newline at end of file From fd79cb73f6121ca1711d4cc91a88b34e05aaa21c Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Tue, 4 Sep 2018 18:11:59 -0700 Subject: [PATCH 06/32] unit tests --- .../AudienceConditionEvaluationTest.java | 187 ++++++++++++++++-- 1 file changed, 170 insertions(+), 17 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 74d4024e3..a42399fbd 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -30,6 +30,7 @@ import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; +import static org.junit.Assert.assertNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; @@ -56,6 +57,7 @@ public void initialize() { testTypedUserAttributes.put("num_counts", 3.55); testTypedUserAttributes.put("num_size", 3); testTypedUserAttributes.put("meta_data", testUserAttributes); + testTypedUserAttributes.put("null_val", null); } /** @@ -67,23 +69,6 @@ public void userAttributeEvaluateTrue() throws Exception { assertTrue(testInstance.evaluate(testUserAttributes)); } - - /** - * Verify that UserAttribute.evaluate returns true on exact-matching visitor attribute data. - */ - @Test - public void typedUserAttributeEvaluateTrue() throws Exception { - UserAttribute testInstance = new UserAttribute("meta_data", "custom_dimension", testUserAttributes); - UserAttribute testInstance2 = new UserAttribute("is_firefox", "custom_dimension", true); - UserAttribute testInstance3 = new UserAttribute("num_counts", "custom_dimension", 3.55); - UserAttribute testInstance4 = new UserAttribute("num_size", "custom_dimension", 3); - - assertTrue(testInstance.evaluate(testTypedUserAttributes)); - assertTrue(testInstance2.evaluate(testTypedUserAttributes)); - assertTrue(testInstance3.evaluate(testTypedUserAttributes)); - assertTrue(testInstance4.evaluate(testTypedUserAttributes)); - } - /** * Verify that UserAttribute.evaluate returns false on non-exact-matching visitor attribute data. */ @@ -102,6 +87,174 @@ public void userAttributeUnknownAttribute() throws Exception { assertFalse(testInstance.evaluate(testUserAttributes)); } + @Test + public void invalidMatchCondition() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", "chrome"); + assertNull(testInstance.evaluate(testUserAttributes)); + } + + @Test + public void existsMatchConditionEvaluatesTrue() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "exists", "firefox"); + assertTrue(testInstance.evaluate(testUserAttributes)); + + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exists", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "exists", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exists", 4.55); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "exists", testUserAttributes); + assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceObject.evaluate(testTypedUserAttributes)); + } + + @Test + public void existsMatchConditionEvaluatesFalse() throws Exception { + UserAttribute testInstance = new UserAttribute("bad_var", "exists", "chrome"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "exists", "chrome"); + assertFalse(testInstance.evaluate(testTypedUserAttributes)); + assertFalse(testInstanceNull.evaluate(testTypedUserAttributes)); + } + + @Test + public void exactMatchConditionEvaluatesTrue() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "chrome"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", true); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", 3); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", 3.55); + + assertTrue(testInstanceString.evaluate(testUserAttributes)); + assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + @Test + public void exactMatchConditionEvaluatesFalse() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "firefox"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", 5.55); + + assertFalse(testInstanceString.evaluate(testUserAttributes)); + assertFalse(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + @Test + public void exactMatchConditionEvaluatesNull() throws Exception { + UserAttribute testInstanceObject = new UserAttribute("meta_data", "exact", testUserAttributes); + + UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", true); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", "true"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", "3"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", "3.55"); + + assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); + + // hmm??? passes because we aren't specifying the type of the value + // assertNull(testInstanceString.evaluate(testUserAttributes)); + // assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); + // assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); + // assertNull(testInstanceDouble.evaluate(testTypedUserAttributes)); + + // UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); + // assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); + } + + + @Test + public void gtMatchConditionEvaluatesTrue() throws Exception { + UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "gt", 2.55); + + assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + @Test + public void gtMatchConditionEvaluatesFalse() throws Exception { + UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "gt", 5.55); + + assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + @Test + public void gtMatchConditionEvaluatesNull() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "gt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "gt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "gt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "gt", 3.5); + + assertNull(testInstanceString.evaluate(testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); + } + + @Test + public void ltMatchConditionEvaluatesTrue() throws Exception { + UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "lt", 5.55); + + assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + @Test + public void ltMatchConditionEvaluatesFalse() throws Exception { + UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "lt", 2.55); + + assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); + } + + + @Test + public void ltMatchConditionEvaluatesNull() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "lt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "lt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "lt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "lt", 3.5); + + assertNull(testInstanceString.evaluate(testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); + } + + @Test + public void substringMatchConditionEvaluatesTrue() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chrome1"); + assertTrue(testInstanceString.evaluate(testUserAttributes)); + } + + @Test + public void substringMatchConditionEvaluatesFalse() throws Exception { + UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chr"); + assertFalse(testInstanceString.evaluate(testUserAttributes)); + } + + @Test + public void substringMatchConditionEvaluatesNull() throws Exception { + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "substring", "chrome1"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "substring", "chrome1"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "substring", "chrome1"); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "substring", "chrome1"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "substring", "chrome1"); + + assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertNull(testInstanceDouble.evaluate(testTypedUserAttributes)); + assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); + assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); + } + + /** * Verify that NotCondition.evaluate returns true when its condition operand evaluates to false. */ From 775be994311ba074a9d91eff559b1fc9e874f466 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 5 Sep 2018 10:08:47 -0700 Subject: [PATCH 07/32] fix Exact to pass back null if the value types are different. added CustomDimensionMatcher to mimic current custom dimension behavior. This should be changed to use Exact. --- .../ab/config/audience/MatchType.java | 19 ++++++++++++++++++- .../AudienceConditionEvaluationTest.java | 12 ++++++------ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index 33848e8b2..588212d2c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -39,6 +39,23 @@ protected ExactMatch(T value) { this.value = value; } + public @Nullable Boolean eval(Object otherValue) { + if (!value.getClass().isInstance(otherValue)) return null; + return value.equals(convert(otherValue)); + } +} + +/** + * This is a temporary class. It mimics the current behaviour for + * custom dimension. This will be dropped for ExactMatch and the unit tests need to be fixed. + * @param + */ +class CustomDimensionMatch extends LeafMatch { + T value; + protected CustomDimensionMatch(T value) { + this.value = value; + } + public @Nullable Boolean eval(Object otherValue) { return value.equals(convert(otherValue)); } @@ -146,7 +163,7 @@ else if (value instanceof Boolean) { break; case "custom_dimension": if (value instanceof String) { - return new MatchType(type, new ExactMatch((String) value)); + return new MatchType(type, new CustomDimensionMatch((String) value)); } break; default: diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index a42399fbd..f15c5ddd4 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -154,13 +154,13 @@ public void exactMatchConditionEvaluatesNull() throws Exception { assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); // hmm??? passes because we aren't specifying the type of the value - // assertNull(testInstanceString.evaluate(testUserAttributes)); - // assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); - // assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); - // assertNull(testInstanceDouble.evaluate(testTypedUserAttributes)); + assertNull(testInstanceString.evaluate(testUserAttributes)); + assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); + assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); + assertNull(testInstanceDouble.evaluate(testTypedUserAttributes)); - // UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); - // assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); + UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); + assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); } From 4fbc2e4d1c2827bebe3b99599f4428891814db2c Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Wed, 5 Sep 2018 11:08:40 -0700 Subject: [PATCH 08/32] unit test descriptions --- .../AudienceConditionEvaluationTest.java | 72 ++++++++++++++++--- 1 file changed, 64 insertions(+), 8 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index f15c5ddd4..65f157ed5 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -87,12 +87,19 @@ public void userAttributeUnknownAttribute() throws Exception { assertFalse(testInstance.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate returns null on invalid match type. + */ @Test public void invalidMatchCondition() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", "chrome"); assertNull(testInstance.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor + * attributes with non-null instances + */ @Test public void existsMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "exists", "firefox"); @@ -108,6 +115,10 @@ public void existsMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceObject.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXIST match type returns false for unknown visitor + * attributes OR null visitor attributes. + */ @Test public void existsMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstance = new UserAttribute("bad_var", "exists", "chrome"); @@ -116,6 +127,10 @@ public void existsMatchConditionEvaluatesFalse() throws Exception { assertFalse(testInstanceNull.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXACT match type returns true for known visitor + * attributes where the values and the value's type are the same + */ @Test public void exactMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "chrome"); @@ -129,6 +144,10 @@ public void exactMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXACT match type returns false for known visitor + * attributes where the value's type are the same, but the values are different + */ @Test public void exactMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "firefox"); @@ -142,28 +161,32 @@ public void exactMatchConditionEvaluatesFalse() throws Exception { assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXACT match type returns null for known visitor + * attributes where the value's type are different OR for values with null and object type. + */ @Test public void exactMatchConditionEvaluatesNull() throws Exception { UserAttribute testInstanceObject = new UserAttribute("meta_data", "exact", testUserAttributes); - UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", true); UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", "true"); UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", "3"); UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", "3.55"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); - - // hmm??? passes because we aren't specifying the type of the value assertNull(testInstanceString.evaluate(testUserAttributes)); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); assertNull(testInstanceDouble.evaluate(testTypedUserAttributes)); - - UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); } - + /** + * Verify that UserAttribute.evaluate for GT match type returns true for known visitor + * attributes where the value's type is a number, and the UserAttribute's value is greater than + * the condition's value. + */ @Test public void gtMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 2); @@ -173,6 +196,11 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for GT match type returns false for known visitor + * attributes where the value's type is a number, and the UserAttribute's value is not greater + * than the condition's value. + */ @Test public void gtMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 5); @@ -182,6 +210,10 @@ public void gtMatchConditionEvaluatesFalse() throws Exception { assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for GT match type returns null if the UserAttribute's + * value type is not a number. + */ @Test public void gtMatchConditionEvaluatesNull() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "gt", 3.5); @@ -195,6 +227,11 @@ public void gtMatchConditionEvaluatesNull() throws Exception { assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for GT match type returns true for known visitor + * attributes where the value's type is a number, and the UserAttribute's value is less than + * the condition's value. + */ @Test public void ltMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 5); @@ -204,6 +241,11 @@ public void ltMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for GT match type returns true for known visitor + * attributes where the value's type is a number, and the UserAttribute's value is not less + * than the condition's value. + */ @Test public void ltMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 2); @@ -213,7 +255,10 @@ public void ltMatchConditionEvaluatesFalse() throws Exception { assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); } - + /** + * Verify that UserAttribute.evaluate for LT match type returns null if the UserAttribute's + * value type is not a number. + */ @Test public void ltMatchConditionEvaluatesNull() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "lt", 3.5); @@ -227,18 +272,30 @@ public void ltMatchConditionEvaluatesNull() throws Exception { assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for SUBSTRING match type returns true if the + * UserAttribute's value is a substring of the condition's value. + */ @Test public void substringMatchConditionEvaluatesTrue() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chrome1"); assertTrue(testInstanceString.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for SUBSTRING match type returns true if the + * UserAttribute's value is NOT a substring of the condition's value. + */ @Test public void substringMatchConditionEvaluatesFalse() throws Exception { UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chr"); assertFalse(testInstanceString.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for SUBSTRING match type returns null if the + * UserAttribute's value type is not a string. + */ @Test public void substringMatchConditionEvaluatesNull() throws Exception { UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "substring", "chrome1"); @@ -254,7 +311,6 @@ public void substringMatchConditionEvaluatesNull() throws Exception { assertNull(testInstanceNull.evaluate(testTypedUserAttributes)); } - /** * Verify that NotCondition.evaluate returns true when its condition operand evaluates to false. */ From b20e7300c9436e84e88cb4ac50c606f73003d4a3 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 6 Sep 2018 15:07:59 -0700 Subject: [PATCH 09/32] update condition evaluation to return null under matrix given here https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# --- .../ab/config/audience/AndCondition.java | 21 ++++++++++++++++--- .../ab/config/audience/Condition.java | 2 ++ .../ab/config/audience/NotCondition.java | 6 ++++-- .../ab/config/audience/OrCondition.java | 17 +++++++++++++-- 4 files changed, 39 insertions(+), 7 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index b215b6198..144feff37 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -17,6 +17,7 @@ package com.optimizely.ab.config.audience; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import java.util.List; import java.util.Map; @@ -36,13 +37,27 @@ public List getConditions() { return conditions; } - public Boolean evaluate(Map attributes) { + public @Nullable + Boolean evaluate(Map attributes) { + boolean foundNull = false; + // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# + // According to the matix mentioned in the above document. for (Condition condition : conditions) { - if (!condition.evaluate(attributes)) + Boolean conditionEval = condition.evaluate(attributes); + if (conditionEval == null) { + foundNull = true; + } + else if (!conditionEval) { // false with nulls or trues is false. return false; + } + // true and nulls with no false will be null. } - return true; + if (foundNull) { // true and null or all null returns null + return null; + } + + return true; // otherwise, return true } @Override diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index 31b8fd242..bd5c1503e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.config.audience; +import javax.annotation.Nullable; import java.util.Map; /** @@ -23,5 +24,6 @@ */ public interface Condition { + @Nullable Boolean evaluate(Map attributes); } \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java index f215a1db7..e8121f76e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/NotCondition.java @@ -16,6 +16,7 @@ */ package com.optimizely.ab.config.audience; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import javax.annotation.Nonnull; @@ -37,8 +38,9 @@ public Condition getCondition() { return condition; } - public Boolean evaluate(Map attributes) { - return !condition.evaluate(attributes); + public @Nullable Boolean evaluate(Map attributes) { + Boolean conditionEval = condition.evaluate(attributes); + return (conditionEval == null ? null : !conditionEval); } @Override diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index c88f851ea..da8530666 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -17,6 +17,7 @@ package com.optimizely.ab.config.audience; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; import java.util.List; import java.util.Map; @@ -36,10 +37,22 @@ public List getConditions() { return conditions; } - public Boolean evaluate(Map attributes) { + // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# + // According to the matix mentioned in the above document. + public @Nullable Boolean evaluate(Map attributes) { + boolean foundNull = false; for (Condition condition : conditions) { - if (condition.evaluate(attributes)) + Boolean conditionEval = condition.evaluate(attributes); + if (conditionEval == null) {// true with falses and nulls is still true + foundNull = true; + } + else if (conditionEval) { return true; + } + } + + if (foundNull) {// if found null and false return null. all false return false + return null; } return false; From d6f762ed89c9b7ad57110d158807bd34605d659a Mon Sep 17 00:00:00 2001 From: Patrick Shih Date: Thu, 6 Sep 2018 17:24:51 -0700 Subject: [PATCH 10/32] some unit tests (and) --- .../AudienceConditionEvaluationTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 65f157ed5..ba02a2b79 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -412,6 +412,7 @@ public void andConditionEvaluateFalse() throws Exception { OrCondition orCondition2 = mock(OrCondition.class); when(orCondition2.evaluate(testUserAttributes)).thenReturn(true); + // and[false, true] List conditions = new ArrayList(); conditions.add(orCondition1); conditions.add(orCondition2); @@ -421,8 +422,36 @@ public void andConditionEvaluateFalse() throws Exception { verify(orCondition1, times(1)).evaluate(testUserAttributes); // shouldn't be called due to short-circuiting in 'And' evaluation verify(orCondition2, times(0)).evaluate(testUserAttributes); + + OrCondition orCondition3 = mock(OrCondition.class); + when(orCondition3.evaluate(testUserAttributes)).thenReturn(null); + + // and[null, false] + List conditions2 = new ArrayList(); + conditions2.add(orCondition3); + conditions2.add(orCondition1); + + AndCondition andCondition2 = new AndCondition(conditions2); + assertFalse(andCondition2.evaluate(testUserAttributes)); + + // and[true, false, null] + List conditions3 = new ArrayList(); + conditions3.add(orCondition2); + conditions3.add(orCondition3); + conditions3.add(orCondition1); + + AndCondition andCondition3 = new AndCondition(conditions3); + assertFalse(andCondition3.evaluate(testUserAttributes)); } + /** + * Verify that AndCondition.evaluate returns null when any one of its operand conditions evaluate to false. + */ + // @Test + // public void andConditionEvaluateNull() throws Exception { + + // } + /** * Verify that {@link UserAttribute#evaluate(Map)} * called when its attribute value is null From 86ac4349d973d5a8402512741c016150db44e801 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 6 Sep 2018 19:55:05 -0700 Subject: [PATCH 11/32] add new match type? --- .../ab/config/audience/MatchType.java | 2 + .../ab/config/audience/UserAttribute.java | 29 +++--- .../parser/AudienceGsonDeserializer.java | 2 +- .../parser/AudienceJacksonDeserializer.java | 2 +- .../ab/config/parser/JsonConfigParser.java | 1 + .../config/parser/JsonSimpleConfigParser.java | 2 +- .../ab/config/ProjectConfigTest.java | 2 +- .../ab/config/ProjectConfigTestUtils.java | 4 +- .../ab/config/ValidProjectConfigV4.java | 9 +- .../AudienceConditionEvaluationTest.java | 97 ++++++++++--------- 10 files changed, 76 insertions(+), 74 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index 588212d2c..4dad2c3be 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -132,6 +132,8 @@ public class MatchType { private LeafMatcher matcher; public static MatchType getMatchType(String type, Object value) { + if (type == null) type = "custom_dimension"; + switch (type) { case "exists": return new MatchType(type, new ExistsMatch(value)); diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 74b3809f6..78e5119c2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -29,11 +29,13 @@ public class UserAttribute implements Condition { private final String name; private final String type; + private final String match; private final Object value; - public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable Object value) { + public UserAttribute(@Nonnull String name, @Nonnull String type, @Nullable String match, @Nullable Object value) { this.name = name; this.type = type; + this.match = match; this.value = value; } @@ -45,6 +47,10 @@ public String getType() { return type; } + public String getMatch() { + return match; + } + public Object getValue() { return value; } @@ -53,23 +59,14 @@ public Boolean evaluate(Map attributes) { // Valid for primative types, but needs to change when a value is an object or an array Object userAttributeValue = attributes.get(name); - if (value != null) { // if there is a value in the condition - // check user attribute value is equal - try { - return MatchType.getMatchType(type, value).getMatcher().eval(attributes.get(name)); - } - catch (NullPointerException np) { - return false; - } - + // check user attribute value is equal + try { + return MatchType.getMatchType(match, value).getMatcher().eval(userAttributeValue); } - else if (userAttributeValue != null) { // if the datafile value is null but user has a value for this attribute - // return false since null != nonnull - return false; - } - else { // both are null - return true; + catch (NullPointerException np) { + return null; } + } @Override diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java index 8158aeb4b..621f47ba1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java @@ -69,7 +69,7 @@ private Condition parseConditions(List rawObjectList) { } else { LinkedTreeMap conditionMap = (LinkedTreeMap)rawObjectList.get(i); conditions.add(new UserAttribute(conditionMap.get("name"), conditionMap.get("type"), - conditionMap.get("value"))); + conditionMap.get("match"), conditionMap.get("value"))); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java index 1d0efc3e7..ae801312e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java @@ -61,7 +61,7 @@ private Condition parseConditions(List rawObjectList) { } else { HashMap conditionMap = (HashMap)rawObjectList.get(i); conditions.add(new UserAttribute(conditionMap.get("name"), conditionMap.get("type"), - conditionMap.get("value"))); + conditionMap.get("match"), conditionMap.get("value"))); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 593fa56ae..50d189b1f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -296,6 +296,7 @@ private Condition parseConditions(JSONArray conditionJson) { conditions.add(new UserAttribute( (String)conditionMap.get("name"), (String)conditionMap.get("type"), + (String)conditionMap.get("match"), value )); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index c4784b5c4..dee0d7c30 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -298,7 +298,7 @@ private Condition parseConditions(JSONArray conditionJson) { } else { JSONObject conditionMap = (JSONObject)obj; conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"), - (String)conditionMap.get("value"))); + (String)conditionMap.get("match"), (String)conditionMap.get("value"))); } } 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 2d64e71fe..3bec4ba4f 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 @@ -106,7 +106,7 @@ public void verifyGetExperimentsForInvalidEvent() throws Exception { @Test public void verifyGetAudienceConditionsFromValidId() throws Exception { List userAttributes = new ArrayList(); - userAttributes.add(new UserAttribute("browser_type", "custom_dimension", "firefox")); + userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null,"firefox")); OrCondition orInner = new OrCondition(userAttributes); diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java index a731830ac..ce598baf7 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java @@ -88,7 +88,7 @@ private static ProjectConfig generateValidProjectConfigV2() { new EventType("100", "no_running_experiments", singletonList("118"))); List userAttributes = new ArrayList(); - userAttributes.add(new UserAttribute("browser_type", "custom_dimension", "firefox")); + userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null, "firefox")); OrCondition orInner = new OrCondition(userAttributes); @@ -245,7 +245,7 @@ private static ProjectConfig generateValidProjectConfigV3() { new EventType("100", "no_running_experiments", singletonList("118"))); List userAttributes = new ArrayList(); - userAttributes.add(new UserAttribute("browser_type", "custom_dimension", "firefox")); + userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null,"firefox")); OrCondition orInner = new OrCondition(userAttributes); diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index 19b3a4ae1..eac41d606 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -72,7 +72,7 @@ public class ValidProjectConfigV4 { new AndCondition(Collections.singletonList( new OrCondition(Collections.singletonList( new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_HOUSE_KEY, - CUSTOM_DIMENSION_TYPE, + CUSTOM_DIMENSION_TYPE, null, AUDIENCE_GRYFFINDOR_VALUE))))))) ); private static final String AUDIENCE_SLYTHERIN_ID = "3988293898"; @@ -84,7 +84,7 @@ public class ValidProjectConfigV4 { new AndCondition(Collections.singletonList( new OrCondition(Collections.singletonList( new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_HOUSE_KEY, - CUSTOM_DIMENSION_TYPE, + CUSTOM_DIMENSION_TYPE, null, AUDIENCE_SLYTHERIN_VALUE))))))) ); @@ -97,7 +97,7 @@ public class ValidProjectConfigV4 { new AndCondition(Collections.singletonList( new OrCondition(Collections.singletonList( new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_NATIONALITY_KEY, - CUSTOM_DIMENSION_TYPE, + CUSTOM_DIMENSION_TYPE, null, AUDIENCE_ENGLISH_CITIZENS_VALUE))))))) ); private static final String AUDIENCE_WITH_MISSING_VALUE_ID = "2196265320"; @@ -105,12 +105,13 @@ public class ValidProjectConfigV4 { public static final String AUDIENCE_WITH_MISSING_VALUE_VALUE = "English"; private static final UserAttribute ATTRIBUTE_WITH_VALUE = new UserAttribute( ATTRIBUTE_NATIONALITY_KEY, - CUSTOM_DIMENSION_TYPE, + CUSTOM_DIMENSION_TYPE, null, AUDIENCE_WITH_MISSING_VALUE_VALUE ); private static final UserAttribute ATTRIBUTE_WITHOUT_VALUE = new UserAttribute( ATTRIBUTE_NATIONALITY_KEY, CUSTOM_DIMENSION_TYPE, + null, null ); private static final Audience AUDIENCE_WITH_MISSING_VALUE = new Audience( diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 65f157ed5..e7fa1811d 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -65,7 +65,7 @@ public void initialize() { */ @Test public void userAttributeEvaluateTrue() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", "chrome"); + UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", null,"chrome"); assertTrue(testInstance.evaluate(testUserAttributes)); } @@ -74,7 +74,7 @@ public void userAttributeEvaluateTrue() throws Exception { */ @Test public void userAttributeEvaluateFalse() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", "firefox"); + UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", null,"firefox"); assertFalse(testInstance.evaluate(testUserAttributes)); } @@ -83,7 +83,7 @@ public void userAttributeEvaluateFalse() throws Exception { */ @Test public void userAttributeUnknownAttribute() throws Exception { - UserAttribute testInstance = new UserAttribute("unknown_dim", "custom_dimension", "unknown"); + UserAttribute testInstance = new UserAttribute("unknown_dim", "custom_dimension", null,"unknown"); assertFalse(testInstance.evaluate(testUserAttributes)); } @@ -92,7 +92,7 @@ public void userAttributeUnknownAttribute() throws Exception { */ @Test public void invalidMatchCondition() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", "chrome"); + UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null,"chrome"); assertNull(testInstance.evaluate(testUserAttributes)); } @@ -102,13 +102,13 @@ public void invalidMatchCondition() throws Exception { */ @Test public void existsMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "exists", "firefox"); + UserAttribute testInstance = new UserAttribute("browser_type", "custome_attribute","exists", "firefox"); assertTrue(testInstance.evaluate(testUserAttributes)); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exists", false); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "exists", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exists", 4.55); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "exists", testUserAttributes); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exists", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exists", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exists", 4.55); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","exists", testUserAttributes); assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -121,8 +121,8 @@ public void existsMatchConditionEvaluatesTrue() throws Exception { */ @Test public void existsMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstance = new UserAttribute("bad_var", "exists", "chrome"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "exists", "chrome"); + UserAttribute testInstance = new UserAttribute("bad_var", "custome_attribute","exists", "chrome"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","exists", "chrome"); assertFalse(testInstance.evaluate(testTypedUserAttributes)); assertFalse(testInstanceNull.evaluate(testTypedUserAttributes)); } @@ -133,10 +133,10 @@ public void existsMatchConditionEvaluatesFalse() throws Exception { */ @Test public void exactMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "chrome"); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", true); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", 3); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", 3.55); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", "chrome"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", true); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", 3); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exact", 3.55); assertTrue(testInstanceString.evaluate(testUserAttributes)); assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -150,10 +150,10 @@ public void exactMatchConditionEvaluatesTrue() throws Exception { */ @Test public void exactMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", "firefox"); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", false); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", 5.55); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", "firefox"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exact", 5.55); assertFalse(testInstanceString.evaluate(testUserAttributes)); assertFalse(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -167,12 +167,12 @@ public void exactMatchConditionEvaluatesFalse() throws Exception { */ @Test public void exactMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceObject = new UserAttribute("meta_data", "exact", testUserAttributes); - UserAttribute testInstanceString = new UserAttribute("browser_type", "exact", true); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "exact", "true"); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "exact", "3"); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "exact", "3.55"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "exact", "null_val"); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","exact", testUserAttributes); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", true); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", "true"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", "3"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute", "exact", "3.55"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","exact", "null_val"); assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); assertNull(testInstanceString.evaluate(testUserAttributes)); @@ -189,8 +189,8 @@ public void exactMatchConditionEvaluatesNull() throws Exception { */ @Test public void gtMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 2); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "gt", 2.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","gt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","gt", 2.55); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -203,8 +203,8 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { */ @Test public void gtMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "gt", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "gt", 5.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","gt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","gt", 5.55); assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -216,10 +216,10 @@ public void gtMatchConditionEvaluatesFalse() throws Exception { */ @Test public void gtMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "gt", 3.5); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "gt", 3.5); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "gt", 3.5); - UserAttribute testInstanceNull = new UserAttribute("null_val", "gt", 3.5); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","gt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","gt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","gt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","gt", 3.5); assertNull(testInstanceString.evaluate(testUserAttributes)); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -234,8 +234,8 @@ public void gtMatchConditionEvaluatesNull() throws Exception { */ @Test public void ltMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "lt", 5.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","lt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","lt", 5.55); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -248,8 +248,8 @@ public void ltMatchConditionEvaluatesTrue() throws Exception { */ @Test public void ltMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "lt", 2); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "lt", 2.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","lt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","lt", 2.55); assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -261,10 +261,10 @@ public void ltMatchConditionEvaluatesFalse() throws Exception { */ @Test public void ltMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "lt", 3.5); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "lt", 3.5); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "lt", 3.5); - UserAttribute testInstanceNull = new UserAttribute("null_val", "lt", 3.5); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","lt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","lt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","lt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","lt", 3.5); assertNull(testInstanceString.evaluate(testUserAttributes)); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -278,7 +278,7 @@ public void ltMatchConditionEvaluatesNull() throws Exception { */ @Test public void substringMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chrome1"); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","substring", "chrome1"); assertTrue(testInstanceString.evaluate(testUserAttributes)); } @@ -288,7 +288,7 @@ public void substringMatchConditionEvaluatesTrue() throws Exception { */ @Test public void substringMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "substring", "chr"); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","substring", "chr"); assertFalse(testInstanceString.evaluate(testUserAttributes)); } @@ -298,11 +298,11 @@ public void substringMatchConditionEvaluatesFalse() throws Exception { */ @Test public void substringMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "substring", "chrome1"); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "substring", "chrome1"); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "substring", "chrome1"); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "substring", "chrome1"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "substring", "chrome1"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","substring", "chrome1"); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); @@ -439,6 +439,7 @@ public void nullValueEvaluate() throws Exception { UserAttribute nullValueAttribute = new UserAttribute( attributeName, attributeType, + "exact", attributeValue ); From 23cd6e134d8d0c55052e2f6966a2d566fb292323 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Mon, 10 Sep 2018 12:27:09 -0700 Subject: [PATCH 12/32] update tests after evaluation of audience with no attributes --- .../ab/config/audience/UserAttribute.java | 3 + .../ab/config/parser/JsonConfigParser.java | 2 +- .../ab/internal/ExperimentUtils.java | 9 +- .../com/optimizely/ab/OptimizelyTest.java | 71 ++++++++----- .../ab/bucketing/DecisionServiceTest.java | 12 --- .../AudienceConditionEvaluationTest.java | 100 +++++++++--------- .../ab/internal/ExperimentUtilsTest.java | 6 +- 7 files changed, 106 insertions(+), 97 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index 78e5119c2..b6c7804d9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -59,6 +59,9 @@ public Boolean evaluate(Map attributes) { // Valid for primative types, but needs to change when a value is an object or an array Object userAttributeValue = attributes.get(name); + if (!"custom_attribute".equals(type)) { + return null; // unknown type + } // check user attribute value is equal try { return MatchType.getMatchType(match, value).getMatcher().eval(userAttributeValue); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 50d189b1f..f3bb40508 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -296,7 +296,7 @@ private Condition parseConditions(JSONArray conditionJson) { conditions.add(new UserAttribute( (String)conditionMap.get("name"), (String)conditionMap.get("type"), - (String)conditionMap.get("match"), + conditionMap.has("match")?(String)conditionMap.get("match"):null, value )); } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index 5ca27449e..f520e546e 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -66,14 +66,11 @@ public static boolean isUserInExperiment(@Nonnull ProjectConfig projectConfig, return true; } - // if there are audiences, but no user attributes, the user is not in the experiment. - if (attributes.isEmpty()) { - return false; - } - for (String audienceId : experimentAudienceIds) { Condition conditions = projectConfig.getAudienceConditionsFromId(audienceId); - if (conditions.evaluate(attributes)) { + Boolean conditionEval = conditions.evaluate(attributes); + + if (conditionEval != null && conditionEval){ return true; } } 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 1add5241e..22c3ffeca 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -137,7 +137,7 @@ public static Collection data() throws IOException { private static final String testBucketingId = "bucketingId"; private static final String testBucketingIdKey = ControlAttribute.BUCKETING_ATTRIBUTE.toString(); private static final Map testParams = Collections.singletonMap("test", "params"); - private static final LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, null); + private static final LogEvent logEventToDispatch = new LogEvent(RequestMethod.GET, "test_url", testParams, new EventBatch()); private int datafileVersion; private String validDatafile; @@ -197,10 +197,16 @@ public void activateEndToEnd() throws Exception { when(mockBucketer.bucket(activatedExperiment, bucketingId)) .thenReturn(bucketedVariation); + logbackVerifier.expectMessage(Level.DEBUG, String.format("No variation for experiment \"%s\" mapped to user \"%s\" in the forced variation map", activatedExperiment.getKey(), testUserId)); + + logbackVerifier.expectMessage(Level.DEBUG, "BucketingId is valid: \"bucketingId\""); + + logbackVerifier.expectMessage(Level.INFO, "This decision will not be saved since the UserProfileService is null."); + logbackVerifier.expectMessage(Level.INFO, "Activating user \"userId\" in experiment \"" + activatedExperiment.getKey() + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching impression event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // activate the experiment Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), userId, testUserAttributes); @@ -678,7 +684,7 @@ public void activateWithUnknownAttribute() throws Exception { activatedExperiment.getKey() + "\"."); logbackVerifier.expectMessage(Level.WARN, "Attribute(s) [unknownAttribute] not in the datafile."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching impression event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // Use an immutable map to also check that we're not attempting to change the provided attribute map Variation actualVariation = @@ -982,12 +988,18 @@ public void activateUserNoAttributesWithAudiences() throws Exception { Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) .build(); - logbackVerifier.expectMessage(Level.INFO, - "User \"userId\" does not meet conditions to be in experiment \"" + experiment.getKey() + "\"."); - logbackVerifier.expectMessage(Level.INFO, "Not activating user \"userId\" for experiment \"" + - experiment.getKey() + "\"."); - - assertNull(optimizely.activate(experiment.getKey(), testUserId)); + /** + * TBD: This should be fixed. The v4 datafile does not contain the same condition so + * results are different. We have made a change around 9/7/18 where we evaluate audience + * regardless if you pass in a attribute list or not. In this case there is a not("broswer_type = "firefox") + * This causes the user to be bucketed now because they don't have browser_type set to firefox. + */ + if (datafileVersion == 4) { + assertNull(optimizely.activate(experiment.getKey(), testUserId)); + } + else { + assertNotNull(optimizely.activate(experiment.getKey(), testUserId)); + } } /** @@ -1415,7 +1427,7 @@ public void trackEventWithAttributes() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track optimizely.track(eventType.getKey(), genericUserId, attributes); @@ -1485,7 +1497,7 @@ public void trackEventWithNullAttributes() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track Map attributes = null; @@ -1555,7 +1567,7 @@ public void trackEventWithNullAttributeValues() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track Map attributes = new HashMap(); @@ -1626,7 +1638,7 @@ public void trackEventWithUnknownAttribute() throws Exception { "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.WARN, "Attribute(s) [unknownAttribute] not in the datafile."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track optimizely.track(eventType.getKey(), genericUserId, ImmutableMap.of("unknownAttribute", "attributeValue")); @@ -1699,7 +1711,7 @@ public void trackEventWithEventTags() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track optimizely.track(eventType.getKey(), genericUserId, Collections.emptyMap(), eventTags); @@ -1773,7 +1785,7 @@ public void trackEventWithNullEventTags() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); // call track optimizely.track(eventType.getKey(), genericUserId, Collections.emptyMap(), null); @@ -2247,11 +2259,18 @@ public void getVariationWithAudiencesNoAttributes() throws Exception { .withErrorHandler(mockErrorHandler) .build(); - logbackVerifier.expectMessage(Level.INFO, - "User \"userId\" does not meet conditions to be in experiment \"" + experiment.getKey() + "\"."); - Variation actualVariation = optimizely.getVariation(experiment.getKey(), testUserId); - assertNull(actualVariation); + /** + * This test now passes because the audience is evaludated even if there is no + * attributes passed in. In version 2,3 of the datafile, the audience is a not condition + * which evaluates to true if it is absent. + */ + if (datafileVersion >= 4) { + assertNull(actualVariation); + } + else { + assertNotNull(actualVariation); + } } /** @@ -2630,11 +2649,13 @@ public void removeNotificationListenerNotificationCenter() throws Exception { .build(); Map attributes = new HashMap(); - attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); - when(mockEventFactory.createImpressionEvent(validProjectConfig, activatedExperiment, - bucketedVariation, genericUserId, attributes)) - .thenReturn(logEventToDispatch); + if (datafileVersion >= 4) { + attributes.put(ATTRIBUTE_HOUSE_KEY, AUDIENCE_GRYFFINDOR_VALUE); + } + else { + attributes.put("browser_type", "chrome"); + } when(mockBucketer.bucket(activatedExperiment, genericUserId)) .thenReturn(bucketedVariation); @@ -2827,7 +2848,7 @@ public void trackEventWithListenerAttributes() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); TrackNotificationListener trackNotification = new TrackNotificationListener() { @Override @@ -2911,7 +2932,7 @@ public void trackEventWithListenerNullAttributes() throws Exception { logbackVerifier.expectMessage(Level.INFO, "Tracking event \"" + eventType.getKey() + "\" for user \"" + genericUserId + "\"."); logbackVerifier.expectMessage(Level.DEBUG, "Dispatching conversion event to URL test_url with params " + - testParams + " and payload \"\""); + testParams + " and payload \"{}\""); TrackNotificationListener trackNotification = new TrackNotificationListener() { @Override diff --git a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java index 0b65e9f91..48271759e 100644 --- a/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java +++ b/core-api/src/test/java/com/optimizely/ab/bucketing/DecisionServiceTest.java @@ -148,8 +148,6 @@ public void getForcedVariationBeforeWhitelisting() throws Exception { // user excluded without audiences and whitelisting assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); - logbackVerifier.expectMessage(Level.INFO, "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"etag1\"."); - // set the runtimeForcedVariation validProjectConfig.setForcedVariation(experiment.getKey(), whitelistedUserId, expectedVariation.getKey()); // no attributes provided for a experiment that has an audience @@ -177,8 +175,6 @@ public void getVariationForcedPrecedesAudienceEval() throws Exception { // user excluded without audiences and whitelisting assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); - logbackVerifier.expectMessage(Level.INFO, "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"etag1\"."); - // set the runtimeForcedVariation validProjectConfig.setForcedVariation(experiment.getKey(), genericUserId, expectedVariation.getKey()); // no attributes provided for a experiment that has an audience @@ -210,10 +206,6 @@ public void getVariationForcedBeforeUserProfile() throws Exception { // ensure that normal users still get excluded from the experiment when they fail audience evaluation assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); - logbackVerifier.expectMessage(Level.INFO, - "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"" - + experiment.getKey() + "\"."); - // ensure that a user with a saved user profile, sees the same variation regardless of audience evaluation assertEquals(variation, decisionService.getVariation(experiment, userProfileId, Collections.emptyMap())); @@ -249,10 +241,6 @@ public void getVariationEvaluatesUserProfileBeforeAudienceTargeting() throws Exc // ensure that normal users still get excluded from the experiment when they fail audience evaluation assertNull(decisionService.getVariation(experiment, genericUserId, Collections.emptyMap())); - logbackVerifier.expectMessage(Level.INFO, - "User \"" + genericUserId + "\" does not meet conditions to be in experiment \"" - + experiment.getKey() + "\"."); - // ensure that a user with a saved user profile, sees the same variation regardless of audience evaluation assertEquals(variation, decisionService.getVariation(experiment, userProfileId, Collections.emptyMap())); diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index e7fa1811d..13dc41086 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -65,7 +65,7 @@ public void initialize() { */ @Test public void userAttributeEvaluateTrue() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", null,"chrome"); + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null,"chrome"); assertTrue(testInstance.evaluate(testUserAttributes)); } @@ -74,7 +74,7 @@ public void userAttributeEvaluateTrue() throws Exception { */ @Test public void userAttributeEvaluateFalse() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custom_dimension", null,"firefox"); + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null,"firefox"); assertFalse(testInstance.evaluate(testUserAttributes)); } @@ -83,7 +83,7 @@ public void userAttributeEvaluateFalse() throws Exception { */ @Test public void userAttributeUnknownAttribute() throws Exception { - UserAttribute testInstance = new UserAttribute("unknown_dim", "custom_dimension", null,"unknown"); + UserAttribute testInstance = new UserAttribute("unknown_dim", "custom_attribute", null,"unknown"); assertFalse(testInstance.evaluate(testUserAttributes)); } @@ -102,13 +102,13 @@ public void invalidMatchCondition() throws Exception { */ @Test public void existsMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custome_attribute","exists", "firefox"); + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute","exists", "firefox"); assertTrue(testInstance.evaluate(testUserAttributes)); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exists", false); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exists", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exists", 4.55); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","exists", testUserAttributes); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","exists", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","exists", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","exists", 4.55); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute","exists", testUserAttributes); assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -121,8 +121,8 @@ public void existsMatchConditionEvaluatesTrue() throws Exception { */ @Test public void existsMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstance = new UserAttribute("bad_var", "custome_attribute","exists", "chrome"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","exists", "chrome"); + UserAttribute testInstance = new UserAttribute("bad_var", "custom_attribute","exists", "chrome"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute","exists", "chrome"); assertFalse(testInstance.evaluate(testTypedUserAttributes)); assertFalse(testInstanceNull.evaluate(testTypedUserAttributes)); } @@ -133,10 +133,10 @@ public void existsMatchConditionEvaluatesFalse() throws Exception { */ @Test public void exactMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", "chrome"); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", true); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", 3); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exact", 3.55); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","exact", "chrome"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","exact", true); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","exact", 3); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","exact", 3.55); assertTrue(testInstanceString.evaluate(testUserAttributes)); assertTrue(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -150,10 +150,10 @@ public void exactMatchConditionEvaluatesTrue() throws Exception { */ @Test public void exactMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", "firefox"); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", false); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","exact", 5.55); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","exact", "firefox"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","exact", false); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","exact", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","exact", 5.55); assertFalse(testInstanceString.evaluate(testUserAttributes)); assertFalse(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -167,12 +167,12 @@ public void exactMatchConditionEvaluatesFalse() throws Exception { */ @Test public void exactMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","exact", testUserAttributes); - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","exact", true); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","exact", "true"); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","exact", "3"); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute", "exact", "3.55"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","exact", "null_val"); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute","exact", testUserAttributes); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","exact", true); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","exact", "true"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","exact", "3"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute", "exact", "3.55"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute","exact", "null_val"); assertNull(testInstanceObject.evaluate(testTypedUserAttributes)); assertNull(testInstanceString.evaluate(testUserAttributes)); @@ -189,8 +189,8 @@ public void exactMatchConditionEvaluatesNull() throws Exception { */ @Test public void gtMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","gt", 2); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","gt", 2.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","gt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","gt", 2.55); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -203,8 +203,8 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { */ @Test public void gtMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","gt", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","gt", 5.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","gt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","gt", 5.55); assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -216,10 +216,10 @@ public void gtMatchConditionEvaluatesFalse() throws Exception { */ @Test public void gtMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","gt", 3.5); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","gt", 3.5); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","gt", 3.5); - UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","gt", 3.5); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","gt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","gt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute","gt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute","gt", 3.5); assertNull(testInstanceString.evaluate(testUserAttributes)); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -234,8 +234,8 @@ public void gtMatchConditionEvaluatesNull() throws Exception { */ @Test public void ltMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","lt", 5); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","lt", 5.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","lt", 5); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","lt", 5.55); assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -248,8 +248,8 @@ public void ltMatchConditionEvaluatesTrue() throws Exception { */ @Test public void ltMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","lt", 2); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","lt", 2.55); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","lt", 2); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","lt", 2.55); assertFalse(testInstanceInteger.evaluate(testTypedUserAttributes)); assertFalse(testInstanceDouble.evaluate(testTypedUserAttributes)); @@ -261,10 +261,10 @@ public void ltMatchConditionEvaluatesFalse() throws Exception { */ @Test public void ltMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","lt", 3.5); - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","lt", 3.5); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","lt", 3.5); - UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","lt", 3.5); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","lt", 3.5); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","lt", 3.5); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute","lt", 3.5); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute","lt", 3.5); assertNull(testInstanceString.evaluate(testUserAttributes)); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); @@ -278,7 +278,7 @@ public void ltMatchConditionEvaluatesNull() throws Exception { */ @Test public void substringMatchConditionEvaluatesTrue() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","substring", "chrome1"); assertTrue(testInstanceString.evaluate(testUserAttributes)); } @@ -288,7 +288,7 @@ public void substringMatchConditionEvaluatesTrue() throws Exception { */ @Test public void substringMatchConditionEvaluatesFalse() throws Exception { - UserAttribute testInstanceString = new UserAttribute("browser_type", "custome_attribute","substring", "chr"); + UserAttribute testInstanceString = new UserAttribute("browser_type", "custom_attribute","substring", "chr"); assertFalse(testInstanceString.evaluate(testUserAttributes)); } @@ -298,11 +298,11 @@ public void substringMatchConditionEvaluatesFalse() throws Exception { */ @Test public void substringMatchConditionEvaluatesNull() throws Exception { - UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custome_attribute","substring", "chrome1"); - UserAttribute testInstanceInteger = new UserAttribute("num_size", "custome_attribute","substring", "chrome1"); - UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custome_attribute","substring", "chrome1"); - UserAttribute testInstanceObject = new UserAttribute("meta_data", "custome_attribute","substring", "chrome1"); - UserAttribute testInstanceNull = new UserAttribute("null_val", "custome_attribute","substring", "chrome1"); + UserAttribute testInstanceBoolean = new UserAttribute("is_firefox", "custom_attribute","substring", "chrome1"); + UserAttribute testInstanceInteger = new UserAttribute("num_size", "custom_attribute","substring", "chrome1"); + UserAttribute testInstanceDouble = new UserAttribute("num_counts", "custom_attribute","substring", "chrome1"); + UserAttribute testInstanceObject = new UserAttribute("meta_data", "custom_attribute","substring", "chrome1"); + UserAttribute testInstanceNull = new UserAttribute("null_val", "custom_attribute","substring", "chrome1"); assertNull(testInstanceBoolean.evaluate(testTypedUserAttributes)); assertNull(testInstanceInteger.evaluate(testTypedUserAttributes)); @@ -443,8 +443,8 @@ public void nullValueEvaluate() throws Exception { attributeValue ); - assertTrue(nullValueAttribute.evaluate(Collections.emptyMap())); - assertTrue(nullValueAttribute.evaluate(Collections.singletonMap(attributeName, attributeValue))); - assertFalse(nullValueAttribute.evaluate((Collections.singletonMap(attributeName, "")))); + assertNull(nullValueAttribute.evaluate(Collections.emptyMap())); + assertNull(nullValueAttribute.evaluate(Collections.singletonMap(attributeName, attributeValue))); + assertNull(nullValueAttribute.evaluate((Collections.singletonMap(attributeName, "")))); } } diff --git a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java index bfdcf08df..1954d01fd 100644 --- a/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java +++ b/core-api/src/test/java/com/optimizely/ab/internal/ExperimentUtilsTest.java @@ -128,10 +128,10 @@ public void isUserInExperimentReturnsTrueIfExperimentHasNoAudiences() { * then {@link ExperimentUtils#isUserInExperiment(ProjectConfig, Experiment, Map)} should return false. */ @Test - public void isUserInExperimentReturnsFalseIfExperimentHasAudiencesButUserHasNoAttributes() { + public void isUserInExperimentEvaluatesEvenIfExperimentHasAudiencesButUserHasNoAttributes() { Experiment experiment = projectConfig.getExperiments().get(0); - assertFalse(isUserInExperiment(projectConfig, experiment, Collections.emptyMap())); + assertTrue(isUserInExperiment(projectConfig, experiment, Collections.emptyMap())); } /** @@ -171,7 +171,7 @@ public void isUserInExperimentHandlesNullValue() { Map nonMatchingMap = Collections.singletonMap(ATTRIBUTE_NATIONALITY_KEY, "American"); assertTrue(isUserInExperiment(v4ProjectConfig, experiment, satisfiesFirstCondition)); - assertTrue(isUserInExperiment(v4ProjectConfig, experiment, attributesWithNull)); + assertFalse(isUserInExperiment(v4ProjectConfig, experiment, attributesWithNull)); assertFalse(isUserInExperiment(v4ProjectConfig, experiment, nonMatchingMap)); // It should explicitly be set to null otherwise we will return false on empty maps From c480c8b9e6045781549414ed8285e9899e8d6f0b Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 11 Sep 2018 09:14:46 -0700 Subject: [PATCH 13/32] bring up test coverage --- .../ab/config/audience/MatchType.java | 8 +------- .../AudienceConditionEvaluationTest.java | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+), 7 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java index 4dad2c3be..e1719a385 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java @@ -2,7 +2,6 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; -import javax.annotation.Nonnegative; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -101,12 +100,7 @@ protected ExistsMatch(Object value) { } public @Nullable Boolean eval(Object otherValue) { - try { - return otherValue != null; - } - catch (Exception e) { - return null; - } + return otherValue != null; } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index c1e0f6b2b..63a376989 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -28,6 +28,7 @@ import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.junit.Assert.assertNull; @@ -66,6 +67,10 @@ public void initialize() { @Test public void userAttributeEvaluateTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null,"chrome"); + assertTrue(testInstance.hashCode() != 0); + assertNull(testInstance.getMatch()); + assertEquals(testInstance.getName(), "browser_type"); + assertEquals(testInstance.getType(), "custom_attribute"); assertTrue(testInstance.evaluate(testUserAttributes)); } @@ -96,6 +101,15 @@ public void invalidMatchCondition() throws Exception { assertNull(testInstance.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate returns null on invalid match type. + */ + @Test + public void invalidMatch() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah","chrome"); + assertNull(testInstance.evaluate(testUserAttributes)); + } + /** * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor * attributes with non-null instances @@ -194,6 +208,10 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); + + Map badAttributes = new HashMap<>(); + badAttributes.put("num_size", "bobs burgers"); + assertNull(testInstanceInteger.evaluate(badAttributes)); } /** From 1b4bd6cd3f4982edf017cc0d1a4567c68fe70f63 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 10:55:50 -0700 Subject: [PATCH 14/32] refactor for java standard 1 class per file. --- .../ab/config/audience/MatchType.java | 187 ------------------ .../ab/config/audience/UserAttribute.java | 2 + .../audience/match/CustomDimensionMatch.java | 36 ++++ .../ab/config/audience/match/ExactMatch.java | 32 +++ .../ab/config/audience/match/ExistsMatch.java | 34 ++++ .../ab/config/audience/match/GTMatch.java | 36 ++++ .../ab/config/audience/match/LTMatch.java | 37 ++++ .../ab/config/audience/match/LeafMatch.java | 28 +++ .../ab/config/audience/match/LeafMatcher.java | 21 ++ .../ab/config/audience/match/MatchType.java | 85 ++++++++ .../ab/config/audience/match/NullMatch.java | 34 ++++ .../config/audience/match/SubstringMatch.java | 37 ++++ 12 files changed, 382 insertions(+), 187 deletions(-) delete mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java deleted file mode 100644 index e1719a385..000000000 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/MatchType.java +++ /dev/null @@ -1,187 +0,0 @@ -package com.optimizely.ab.config.audience; - -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -interface LeafMatcher { - public Boolean eval(Object otherValue); -} - -abstract class LeafMatch implements LeafMatcher { - T convert(Object o) { - try { - T rv = (T)o; - return rv; - } catch(java.lang.ClassCastException e) { - return null; - } - } -} - -class NullMatch extends LeafMatch { - @SuppressFBWarnings("URF_UNREAD_FIELD") - Object value; - protected NullMatch() { - this.value = null; - } - - public @Nullable Boolean eval(Object otherValue) { - return null; - } -} - -class ExactMatch extends LeafMatch { - T value; - protected ExactMatch(T value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - if (!value.getClass().isInstance(otherValue)) return null; - return value.equals(convert(otherValue)); - } -} - -/** - * This is a temporary class. It mimics the current behaviour for - * custom dimension. This will be dropped for ExactMatch and the unit tests need to be fixed. - * @param - */ -class CustomDimensionMatch extends LeafMatch { - T value; - protected CustomDimensionMatch(T value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - return value.equals(convert(otherValue)); - } -} - -class GTMatch extends LeafMatch { - Number value; - protected GTMatch(Number value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - try { - return ((Number) convert(otherValue)).doubleValue() > value.doubleValue(); - } - catch (Exception e) { - return null; - } - } -} - -class LTMatch extends LeafMatch { - Number value; - protected LTMatch(Number value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - try { - return ((Number) convert(otherValue)).doubleValue() < value.doubleValue(); - } - catch (Exception e) { - return null; - } - } -} - -class ExistsMatch extends LeafMatch { - @SuppressFBWarnings("URF_UNREAD_FIELD") - Object value; - protected ExistsMatch(Object value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - return otherValue != null; - } -} - -class SubstringMatch extends LeafMatch { - String value; - protected SubstringMatch(String value) { - this.value = value; - } - - public @Nullable Boolean eval(Object otherValue) { - try { - return value.contains(convert(otherValue)); - } - catch (Exception e) { - return null; - } - } -} - -public class MatchType { - - private String type; - private LeafMatcher matcher; - - public static MatchType getMatchType(String type, Object value) { - if (type == null) type = "custom_dimension"; - - switch (type) { - case "exists": - return new MatchType(type, new ExistsMatch(value)); - case "exact": - if (value instanceof String) { - return new MatchType(type, new ExactMatch((String) value)); - } - else if (value instanceof Number) { - return new MatchType(type, new ExactMatch((Number) value)); - } - else if (value instanceof Boolean) { - return new MatchType(type, new ExactMatch((Boolean) value)); - } - break; - case "substring": - if (value instanceof String) { - return new MatchType(type, new SubstringMatch((String) value)); - } - break; - case "gt": - if (value instanceof Number) { - return new MatchType(type, new GTMatch((Number) value)); - } - break; - case "lt": - if (value instanceof Number) { - return new MatchType(type, new LTMatch((Number) value)); - } - break; - case "custom_dimension": - if (value instanceof String) { - return new MatchType(type, new CustomDimensionMatch((String) value)); - } - break; - default: - return new MatchType(type, new NullMatch()); - } - - return new MatchType(type, new NullMatch()); - } - - - private MatchType(String type, LeafMatcher matcher) { - this.type = type; - this.matcher = matcher; - } - - public @Nonnull - LeafMatcher getMatcher() { - return matcher; - } - - @Override - public String toString() { - return type; - } -} \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index b6c7804d9..b8127f959 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -16,6 +16,8 @@ */ package com.optimizely.ab.config.audience; +import com.optimizely.ab.config.audience.match.MatchType; + import javax.annotation.Nonnull; import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java new file mode 100644 index 000000000..c4b023334 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java @@ -0,0 +1,36 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nullable; + +/** + * This is a temporary class. It mimics the current behaviour for + * custom dimension. This will be dropped for ExactMatch and the unit tests need to be fixed. + * @param + */ +class CustomDimensionMatch extends LeafMatch { + T value; + protected CustomDimensionMatch(T value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + return value.equals(convert(otherValue)); + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java new file mode 100644 index 000000000..b23359f6e --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java @@ -0,0 +1,32 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nullable; + +class ExactMatch extends LeafMatch { + T value; + protected ExactMatch(T value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + if (!value.getClass().isInstance(otherValue)) return null; + return value.equals(convert(otherValue)); + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java new file mode 100644 index 000000000..66253789b --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java @@ -0,0 +1,34 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +import javax.annotation.Nullable; + +class ExistsMatch extends LeafMatch { + @SuppressFBWarnings("URF_UNREAD_FIELD") + Object value; + protected ExistsMatch(Object value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + return otherValue != null; + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java new file mode 100644 index 000000000..68b4540cc --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java @@ -0,0 +1,36 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nullable; + +class GTMatch extends LeafMatch { + Number value; + protected GTMatch(Number value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + try { + return ((Number) convert(otherValue)).doubleValue() > value.doubleValue(); + } + catch (Exception e) { + return null; + } + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java new file mode 100644 index 000000000..cfe2054fe --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java @@ -0,0 +1,37 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nullable; + +class LTMatch extends LeafMatch { + Number value; + protected LTMatch(Number value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + try { + return ((Number) convert(otherValue)).doubleValue() < value.doubleValue(); + } + catch (Exception e) { + return null; + } + } +} + diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java new file mode 100644 index 000000000..cfeeb02c5 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java @@ -0,0 +1,28 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +abstract class LeafMatch implements LeafMatcher { + T convert(Object o) { + try { + T rv = (T)o; + return rv; + } catch(java.lang.ClassCastException e) { + return null; + } + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java new file mode 100644 index 000000000..b1a100bfa --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java @@ -0,0 +1,21 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +public interface LeafMatcher { + Boolean eval(Object otherValue); +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java new file mode 100644 index 000000000..d6472879e --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java @@ -0,0 +1,85 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nonnull; + +public class MatchType { + + private String type; + private LeafMatcher matcher; + + public static MatchType getMatchType(String type, Object value) { + if (type == null) type = "custom_dimension"; + + switch (type) { + case "exists": + return new MatchType(type, new ExistsMatch(value)); + case "exact": + if (value instanceof String) { + return new MatchType(type, new ExactMatch((String) value)); + } + else if (value instanceof Number) { + return new MatchType(type, new ExactMatch((Number) value)); + } + else if (value instanceof Boolean) { + return new MatchType(type, new ExactMatch((Boolean) value)); + } + break; + case "substring": + if (value instanceof String) { + return new MatchType(type, new SubstringMatch((String) value)); + } + break; + case "gt": + if (value instanceof Number) { + return new MatchType(type, new GTMatch((Number) value)); + } + break; + case "lt": + if (value instanceof Number) { + return new MatchType(type, new LTMatch((Number) value)); + } + break; + case "custom_dimension": + if (value instanceof String) { + return new MatchType(type, new CustomDimensionMatch((String) value)); + } + break; + default: + return new MatchType(type, new NullMatch()); + } + + return new MatchType(type, new NullMatch()); + } + + + private MatchType(String type, LeafMatcher matcher) { + this.type = type; + this.matcher = matcher; + } + + public @Nonnull + LeafMatcher getMatcher() { + return matcher; + } + + @Override + public String toString() { + return type; + } +} \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java new file mode 100644 index 000000000..8b825cd9f --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java @@ -0,0 +1,34 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; + +import javax.annotation.Nullable; + +class NullMatch extends LeafMatch { + @SuppressFBWarnings("URF_UNREAD_FIELD") + Object value; + protected NullMatch() { + this.value = null; + } + + public @Nullable + Boolean eval(Object otherValue) { + return null; + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java new file mode 100644 index 000000000..6bf3e28e0 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -0,0 +1,37 @@ +/** + * + * Copyright 2018, Optimizely and contributors + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.optimizely.ab.config.audience.match; + +import javax.annotation.Nullable; + +class SubstringMatch extends LeafMatch { + String value; + protected SubstringMatch(String value) { + this.value = value; + } + + public @Nullable + Boolean eval(Object otherValue) { + try { + return value.contains(convert(otherValue)); + } + catch (Exception e) { + return null; + } + } +} + From ac62a995809071513fd33b886dd6ac18d9f1932f Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 11:46:07 -0700 Subject: [PATCH 15/32] trying to get lint to pass java 9 --- .../com/optimizely/ab/config/audience/AndCondition.java | 2 +- .../com/optimizely/ab/config/audience/OrCondition.java | 2 +- .../optimizely/ab/config/audience/match/ExistsMatch.java | 7 ++++++- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 144feff37..441fa0720 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -41,7 +41,7 @@ public List getConditions() { Boolean evaluate(Map attributes) { boolean foundNull = false; // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# - // According to the matix mentioned in the above document. + // According to the matrix mentioned in the above document. for (Condition condition : conditions) { Boolean conditionEval = condition.evaluate(attributes); if (conditionEval == null) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index da8530666..85eae8de9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -38,7 +38,7 @@ public List getConditions() { } // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# - // According to the matix mentioned in the above document. + // According to the matrix mentioned in the above document. public @Nullable Boolean evaluate(Map attributes) { boolean foundNull = false; for (Condition condition : conditions) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java index 66253789b..0ca92d839 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java @@ -29,6 +29,11 @@ protected ExistsMatch(Object value) { public @Nullable Boolean eval(Object otherValue) { - return otherValue != null; + try { + return otherValue != null; + } + catch (Exception e) { + return null; + } } } From 61a1e1f62346ab80812204c4c5c419656b037d6d Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 15:04:31 -0700 Subject: [PATCH 16/32] trying to pass findbugs java 9 --- .../ab/config/audience/match/ExistsMatch.java | 7 +------ .../audience/AudienceConditionEvaluationTest.java | 14 +++++++------- 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java index 0ca92d839..66253789b 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java @@ -29,11 +29,6 @@ protected ExistsMatch(Object value) { public @Nullable Boolean eval(Object otherValue) { - try { - return otherValue != null; - } - catch (Exception e) { - return null; - } + return otherValue != null; } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 63a376989..3cfe0d423 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -68,10 +68,10 @@ public void initialize() { public void userAttributeEvaluateTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null,"chrome"); assertTrue(testInstance.hashCode() != 0); - assertNull(testInstance.getMatch()); - assertEquals(testInstance.getName(), "browser_type"); - assertEquals(testInstance.getType(), "custom_attribute"); - assertTrue(testInstance.evaluate(testUserAttributes)); + //assertNull(testInstance.getMatch()); + //assertEquals(testInstance.getName(), "browser_type"); + //assertEquals(testInstance.getType(), "custom_attribute"); + //assertTrue(testInstance.evaluate(testUserAttributes)); } /** @@ -209,9 +209,9 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); - Map badAttributes = new HashMap<>(); - badAttributes.put("num_size", "bobs burgers"); - assertNull(testInstanceInteger.evaluate(badAttributes)); + //Map badAttributes = new HashMap<>(); + //badAttributes.put("num_size", "bobs burgers"); + //assertNull(testInstanceInteger.evaluate(badAttributes)); } /** From e83698c449b0e0fe561ffc1ccf0fae66cdb7d3f9 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 15:24:08 -0700 Subject: [PATCH 17/32] just seeing if I can get java 9 to pass again --- .../audience/AudienceConditionEvaluationTest.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 3cfe0d423..24ed4b6a9 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -95,11 +95,11 @@ public void userAttributeUnknownAttribute() throws Exception { /** * Verify that UserAttribute.evaluate returns null on invalid match type. */ - @Test - public void invalidMatchCondition() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null,"chrome"); - assertNull(testInstance.evaluate(testUserAttributes)); - } +// @Test +// public void invalidMatchCondition() throws Exception { +// UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null,"chrome"); +// assertNull(testInstance.evaluate(testUserAttributes)); +// } /** * Verify that UserAttribute.evaluate returns null on invalid match type. From 988579d59733089156c9441f32119602031cead8 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 15:25:44 -0700 Subject: [PATCH 18/32] trying to get travis to pass --- .../AudienceConditionEvaluationTest.java | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 24ed4b6a9..66775ac77 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -95,20 +95,20 @@ public void userAttributeUnknownAttribute() throws Exception { /** * Verify that UserAttribute.evaluate returns null on invalid match type. */ -// @Test -// public void invalidMatchCondition() throws Exception { -// UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null,"chrome"); -// assertNull(testInstance.evaluate(testUserAttributes)); -// } + @Test + public void invalidMatchCondition() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "unknown_dimension", null,"chrome"); + assertNull(testInstance.evaluate(testUserAttributes)); + } /** * Verify that UserAttribute.evaluate returns null on invalid match type. */ - @Test - public void invalidMatch() throws Exception { - UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah","chrome"); - assertNull(testInstance.evaluate(testUserAttributes)); - } +// @Test +// public void invalidMatch() throws Exception { +// UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah","chrome"); +// assertNull(testInstance.evaluate(testUserAttributes)); +// } /** * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor From 3bc466b8b4a3bcb358c0175826acd3663654a64d Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 15:38:04 -0700 Subject: [PATCH 19/32] i don't know why findbugs is finding problems in java 9 --- .../AudienceConditionEvaluationTest.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 66775ac77..63a376989 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -68,10 +68,10 @@ public void initialize() { public void userAttributeEvaluateTrue() throws Exception { UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", null,"chrome"); assertTrue(testInstance.hashCode() != 0); - //assertNull(testInstance.getMatch()); - //assertEquals(testInstance.getName(), "browser_type"); - //assertEquals(testInstance.getType(), "custom_attribute"); - //assertTrue(testInstance.evaluate(testUserAttributes)); + assertNull(testInstance.getMatch()); + assertEquals(testInstance.getName(), "browser_type"); + assertEquals(testInstance.getType(), "custom_attribute"); + assertTrue(testInstance.evaluate(testUserAttributes)); } /** @@ -104,11 +104,11 @@ public void invalidMatchCondition() throws Exception { /** * Verify that UserAttribute.evaluate returns null on invalid match type. */ -// @Test -// public void invalidMatch() throws Exception { -// UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah","chrome"); -// assertNull(testInstance.evaluate(testUserAttributes)); -// } + @Test + public void invalidMatch() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute", "blah","chrome"); + assertNull(testInstance.evaluate(testUserAttributes)); + } /** * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor @@ -209,9 +209,9 @@ public void gtMatchConditionEvaluatesTrue() throws Exception { assertTrue(testInstanceInteger.evaluate(testTypedUserAttributes)); assertTrue(testInstanceDouble.evaluate(testTypedUserAttributes)); - //Map badAttributes = new HashMap<>(); - //badAttributes.put("num_size", "bobs burgers"); - //assertNull(testInstanceInteger.evaluate(badAttributes)); + Map badAttributes = new HashMap<>(); + badAttributes.put("num_size", "bobs burgers"); + assertNull(testInstanceInteger.evaluate(badAttributes)); } /** From 7ae84847582deeb0b1e44a486b459b808cefb0b9 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 12 Sep 2018 16:29:53 -0700 Subject: [PATCH 20/32] allow for typedAudiences to exist or not exist in the datafile. --- .../optimizely/ab/config/ProjectConfig.java | 24 ++++++++++++++++++- .../ab/config/parser/JsonConfigParser.java | 6 +++++ .../config/parser/JsonSimpleConfigParser.java | 5 ++++ .../parser/ProjectConfigGsonDeserializer.java | 5 ++++ .../ProjectConfigJacksonDeserializer.java | 9 +++++++ .../ab/config/ValidProjectConfigV4.java | 1 + 6 files changed, 49 insertions(+), 1 deletion(-) 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 25a84f823..992839475 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 @@ -80,6 +80,7 @@ public String toString() { private final Boolean botFiltering; private final List attributes; private final List audiences; + private final List typedAudiences; private final List events; private final List experiments; private final List featureFlags; @@ -136,6 +137,7 @@ public ProjectConfig(String accountId, String projectId, String version, String version, attributes, audiences, + null, eventType, experiments, null, @@ -154,6 +156,7 @@ public ProjectConfig(String accountId, String version, List attributes, List audiences, + List typedAudiences, List events, List experiments, List featureFlags, @@ -170,6 +173,14 @@ public ProjectConfig(String accountId, this.attributes = Collections.unmodifiableList(attributes); this.audiences = Collections.unmodifiableList(audiences); + + if (typedAudiences != null) { + this.typedAudiences = Collections.unmodifiableList(typedAudiences); + } + else { + this.typedAudiences = Collections.emptyList(); + } + this.events = Collections.unmodifiableList(events); if (featureFlags == null) { this.featureFlags = Collections.emptyList(); @@ -206,7 +217,14 @@ public ProjectConfig(String accountId, this.featureKeyMapping = ProjectConfigUtils.generateNameMapping(this.featureFlags); // generate audience id to audience mapping - this.audienceIdMapping = ProjectConfigUtils.generateIdMapping(audiences); + if (typedAudiences == null) { + this.audienceIdMapping = ProjectConfigUtils.generateIdMapping(audiences); + } + else { + List combinedList = new ArrayList<>(audiences); + combinedList.addAll(typedAudiences); + this.audienceIdMapping = ProjectConfigUtils.generateIdMapping(combinedList); + } this.experimentIdMapping = ProjectConfigUtils.generateIdMapping(this.experiments); this.groupIdMapping = ProjectConfigUtils.generateIdMapping(groups); this.rolloutIdMapping = ProjectConfigUtils.generateIdMapping(this.rollouts); @@ -386,6 +404,10 @@ public List getAudiences() { return audiences; } + public List getTypedAudiences() { + return typedAudiences; + } + public Condition getAudienceConditionsFromId(String audienceId) { Audience audience = audienceIdMapping.get(audienceId); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index f3bb40508..176c9c5c9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -69,6 +69,11 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse List events = parseEvents(rootObject.getJSONArray("events")); List audiences = parseAudiences(rootObject.getJSONArray("audiences")); + List typedAudiences = null; + if (rootObject.has("typedAudiences")) { + typedAudiences = parseAudiences(rootObject.getJSONArray("typedAudiences")); + } + List groups = parseGroups(rootObject.getJSONArray("groups")); boolean anonymizeIP = false; @@ -98,6 +103,7 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse version, attributes, audiences, + typedAudiences, events, experiments, featureFlags, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index dee0d7c30..8799ea33d 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -71,6 +71,10 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse List events = parseEvents((JSONArray)rootObject.get("events")); List audiences = parseAudiences((JSONArray)parser.parse(rootObject.get("audiences").toString())); + List typedAudiences =null; + if (rootObject.containsKey("typedAudiences")) { + typedAudiences = parseAudiences((JSONArray)parser.parse(rootObject.get("typedAudiences").toString())); + } List groups = parseGroups((JSONArray)rootObject.get("groups")); boolean anonymizeIP = false; @@ -100,6 +104,7 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse version, attributes, audiences, + typedAudiences, events, experiments, featureFlags, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java index c556b9063..c3305fd5c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java @@ -70,6 +70,10 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa List audiences = context.deserialize(jsonObject.get("audiences").getAsJsonArray(), audienceType); + List typedAudiences = null; + if (jsonObject.has("typedAudiences")) { + typedAudiences = context.deserialize(jsonObject.get("typedAudiences").getAsJsonArray(), audienceType); + } boolean anonymizeIP = false; // live variables should be null if using V2 List liveVariables = null; @@ -101,6 +105,7 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa version, attributes, audiences, + typedAudiences, events, experiments, featureFlags, diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java index 76cac7412..4e28afaa2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java @@ -66,6 +66,14 @@ public ProjectConfig deserialize(JsonParser parser, DeserializationContext conte List audiences = mapper.readValue(node.get("audiences").toString(), new TypeReference>() {}); + List typedAudiences = null; + + if (node.has("typedAudiences")) { + typedAudiences = mapper.readValue(node.get("typedAudiences").toString(), + new TypeReference>() { + }); + } + boolean anonymizeIP = false; List liveVariables = null; if (datafileVersion >= Integer.parseInt(ProjectConfig.Version.V3.toString())) { @@ -95,6 +103,7 @@ public ProjectConfig deserialize(JsonParser parser, DeserializationContext conte version, attributes, audiences, + typedAudiences, events, experiments, featureFlags, diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index 305b21ad7..f6eb65dd1 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -1101,6 +1101,7 @@ public static ProjectConfig generateValidProjectConfigV4() { VERSION, attributes, audiences, + null, events, experiments, featureFlags, From e72aab544dcf13d23ccdb4d4527f89da5a525785 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 13 Sep 2018 13:07:40 -0700 Subject: [PATCH 21/32] added typed attributes to v4 datafile parse and compare them. Need to write more unit tests that use them --- .../optimizely/ab/config/ProjectConfig.java | 1 + .../parser/AudienceGsonDeserializer.java | 22 +++--- .../parser/AudienceJacksonDeserializer.java | 6 +- .../ab/config/parser/JsonConfigParser.java | 4 +- .../config/parser/JsonSimpleConfigParser.java | 2 +- .../ab/config/ProjectConfigTestUtils.java | 2 +- .../ab/config/ValidProjectConfigV4.java | 76 ++++++++++++++++++- .../config/valid-project-config-v4.json | 37 +++++++++ 8 files changed, 133 insertions(+), 17 deletions(-) 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 992839475..5b2f67caa 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 @@ -603,6 +603,7 @@ public String toString() { ", botFiltering=" + botFiltering + ", attributes=" + attributes + ", audiences=" + audiences + + ", typedAudiences=" + typedAudiences + ", events=" + events + ", experiments=" + experiments + ", featureFlags=" + featureFlags + diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java index 621f47ba1..6d39dfdc1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceGsonDeserializer.java @@ -67,19 +67,23 @@ private Condition parseConditions(List rawObjectList) { List objectList = (List)rawObjectList.get(i); conditions.add(parseConditions(objectList)); } else { - LinkedTreeMap conditionMap = (LinkedTreeMap)rawObjectList.get(i); - conditions.add(new UserAttribute(conditionMap.get("name"), conditionMap.get("type"), - conditionMap.get("match"), conditionMap.get("value"))); + LinkedTreeMap conditionMap = (LinkedTreeMap)rawObjectList.get(i); + conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"), + (String)conditionMap.get("match"), conditionMap.get("value"))); } } Condition condition; - if (operand.equals("and")) { - condition = new AndCondition(conditions); - } else if (operand.equals("or")) { - condition = new OrCondition(conditions); - } else { - condition = new NotCondition(conditions.get(0)); + switch (operand) { + case "and": + condition = new AndCondition(conditions); + break; + case "or": + condition = new OrCondition(conditions); + break; + default: + condition = new NotCondition(conditions.get(0)); + break; } return condition; diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java index ae801312e..d2017ae3c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java @@ -59,9 +59,9 @@ private Condition parseConditions(List rawObjectList) { List objectList = (List)rawObjectList.get(i); conditions.add(parseConditions(objectList)); } else { - HashMap conditionMap = (HashMap)rawObjectList.get(i); - conditions.add(new UserAttribute(conditionMap.get("name"), conditionMap.get("type"), - conditionMap.get("match"), conditionMap.get("value"))); + HashMap conditionMap = (HashMap)rawObjectList.get(i); + conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"), + (String)conditionMap.get("match"), conditionMap.get("value"))); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 176c9c5c9..a92316852 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -295,9 +295,9 @@ private Condition parseConditions(JSONArray conditionJson) { conditions.add(parseConditions(conditionJson.getJSONArray(i))); } else { JSONObject conditionMap = (JSONObject)obj; - String value = null; + Object value = null; if (conditionMap.has("value")) { - value = conditionMap.getString("value"); + value = conditionMap.get("value"); } conditions.add(new UserAttribute( (String)conditionMap.get("name"), diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index 8799ea33d..1d64ab582 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -303,7 +303,7 @@ private Condition parseConditions(JSONArray conditionJson) { } else { JSONObject conditionMap = (JSONObject)obj; conditions.add(new UserAttribute((String)conditionMap.get("name"), (String)conditionMap.get("type"), - (String)conditionMap.get("match"), (String)conditionMap.get("value"))); + (String)conditionMap.get("match"), conditionMap.get("value"))); } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java index ce598baf7..57d6d50cd 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java @@ -459,6 +459,7 @@ public static void verifyProjectConfig(@CheckForNull ProjectConfig actual, @Nonn verifyAttributes(actual.getAttributes(), expected.getAttributes()); verifyAudiences(actual.getAudiences(), expected.getAudiences()); + verifyAudiences(actual.getTypedAudiences(), expected.getTypedAudiences()); verifyEvents(actual.getEventTypes(), expected.getEventTypes()); verifyExperiments(actual.getExperiments(), expected.getExperiments()); verifyFeatureFlags(actual.getFeatureFlags(), expected.getFeatureFlags()); @@ -581,7 +582,6 @@ private static void verifyAudiences(List actual, List expect assertThat(actualAudience.getId(), is(expectedAudience.getId())); assertThat(actualAudience.getKey(), is(expectedAudience.getKey())); assertThat(actualAudience.getConditions(), is(expectedAudience.getConditions())); - assertThat(actualAudience.getConditions(), is(expectedAudience.getConditions())); } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index f6eb65dd1..b3a031785 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -63,6 +63,42 @@ public class ValidProjectConfigV4 { // audiences private static final String CUSTOM_ATTRIBUTE_TYPE = "custom_attribute"; + private static final String AUDIENCE_BOOL_ID = "3468206643"; + private static final String AUDIENCE_BOOL_KEY = "BOOL"; + public static final Boolean AUDIENCE_BOOL_VALUE = true; + private static final Audience TYPED_AUDIENCE_BOOL = new Audience( + AUDIENCE_BOOL_ID, + AUDIENCE_BOOL_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_BOOLEAN_KEY, + CUSTOM_ATTRIBUTE_TYPE, "exact", + AUDIENCE_BOOL_VALUE))))))) + ); + private static final String AUDIENCE_INT_ID = "3468206644"; + private static final String AUDIENCE_INT_KEY = "INT"; + public static final Number AUDIENCE_INT_VALUE = 1.0; + private static final Audience TYPED_AUDIENCE_INT = new Audience( + AUDIENCE_INT_ID, + AUDIENCE_INT_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_INTEGER_KEY, + CUSTOM_ATTRIBUTE_TYPE, "gt", + AUDIENCE_INT_VALUE))))))) + ); + private static final String AUDIENCE_DOUBLE_ID = "3468206645"; + private static final String AUDIENCE_DOUBLE_KEY = "DOUBLE"; + public static final Double AUDIENCE_DOUBLE_VALUE = 100.0; + private static final Audience TYPED_AUDIENCE_DOUBLE = new Audience( + AUDIENCE_DOUBLE_ID, + AUDIENCE_DOUBLE_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_DOUBLE_KEY, + CUSTOM_ATTRIBUTE_TYPE, "lt", + AUDIENCE_DOUBLE_VALUE))))))) + ); private static final String AUDIENCE_GRYFFINDOR_ID = "3468206642"; private static final String AUDIENCE_GRYFFINDOR_KEY = "Gryffindors"; public static final String AUDIENCE_GRYFFINDOR_VALUE = "Gryffindor"; @@ -75,6 +111,16 @@ public class ValidProjectConfigV4 { CUSTOM_ATTRIBUTE_TYPE, null, AUDIENCE_GRYFFINDOR_VALUE))))))) ); + private static final Audience TYPED_AUDIENCE_GRYFFINDOR = new Audience( + AUDIENCE_GRYFFINDOR_ID, + AUDIENCE_GRYFFINDOR_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_HOUSE_KEY, + CUSTOM_ATTRIBUTE_TYPE, "exact", + AUDIENCE_GRYFFINDOR_VALUE))))))) + ); + private static final String AUDIENCE_SLYTHERIN_ID = "3988293898"; private static final String AUDIENCE_SLYTHERIN_KEY = "Slytherins"; public static final String AUDIENCE_SLYTHERIN_VALUE = "Slytherin"; @@ -88,6 +134,16 @@ public class ValidProjectConfigV4 { AUDIENCE_SLYTHERIN_VALUE))))))) ); + private static final Audience TYPED_AUDIENCE_SLYTHERIN = new Audience( + AUDIENCE_SLYTHERIN_ID, + AUDIENCE_SLYTHERIN_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_HOUSE_KEY, + CUSTOM_ATTRIBUTE_TYPE, "substring", + AUDIENCE_SLYTHERIN_VALUE))))))) + ); + private static final String AUDIENCE_ENGLISH_CITIZENS_ID = "4194404272"; private static final String AUDIENCE_ENGLISH_CITIZENS_KEY = "english_citizens"; public static final String AUDIENCE_ENGLISH_CITIZENS_VALUE = "English"; @@ -100,6 +156,15 @@ public class ValidProjectConfigV4 { CUSTOM_ATTRIBUTE_TYPE, null, AUDIENCE_ENGLISH_CITIZENS_VALUE))))))) ); + private static final Audience TYPED_AUDIENCE_ENGLISH_CITIZENS = new Audience( + AUDIENCE_ENGLISH_CITIZENS_ID, + AUDIENCE_ENGLISH_CITIZENS_KEY, + new AndCondition(Collections.singletonList( + new OrCondition(Collections.singletonList( + new OrCondition(Collections.singletonList((Condition) new UserAttribute(ATTRIBUTE_NATIONALITY_KEY, + CUSTOM_ATTRIBUTE_TYPE, "exact", + AUDIENCE_ENGLISH_CITIZENS_VALUE))))))) + ); private static final String AUDIENCE_WITH_MISSING_VALUE_ID = "2196265320"; private static final String AUDIENCE_WITH_MISSING_VALUE_KEY = "audience_with_missing_value"; public static final String AUDIENCE_WITH_MISSING_VALUE_VALUE = "English"; @@ -1057,6 +1122,15 @@ public static ProjectConfig generateValidProjectConfigV4() { audiences.add(AUDIENCE_ENGLISH_CITIZENS); audiences.add(AUDIENCE_WITH_MISSING_VALUE); + List typedAudiences = new ArrayList(); + typedAudiences.add(TYPED_AUDIENCE_BOOL); + typedAudiences.add(TYPED_AUDIENCE_INT); + typedAudiences.add(TYPED_AUDIENCE_DOUBLE); + typedAudiences.add(TYPED_AUDIENCE_GRYFFINDOR); + typedAudiences.add(TYPED_AUDIENCE_SLYTHERIN); + typedAudiences.add(TYPED_AUDIENCE_ENGLISH_CITIZENS); + typedAudiences.add(AUDIENCE_WITH_MISSING_VALUE); + // list events List events = new ArrayList(); events.add(EVENT_BASIC_EVENT); @@ -1101,7 +1175,7 @@ public static ProjectConfig generateValidProjectConfigV4() { VERSION, attributes, audiences, - null, + typedAudiences, events, experiments, featureFlags, diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index 48e304205..3ae4aef58 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -27,6 +27,43 @@ "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"nationality\", \"type\": \"custom_attribute\", \"value\": \"English\"}, {\"name\": \"nationality\", \"type\": \"custom_attribute\"}]]]" } ], + "typedAudiences": [ + { + "id": "3468206643", + "name": "BOOL", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"booleanKey\", \"type\": \"custom_attribute\", \"match\":\"exact\", \"value\":true}]]]" + }, + { + "id": "3468206644", + "name": "INT", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"integerKey\", \"type\": \"custom_attribute\", \"match\":\"gt\", \"value\":1.0}]]]" + }, + { + "id": "3468206645", + "name": "DOUBLE", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"doubleKey\", \"type\": \"custom_attribute\", \"match\":\"lt\", \"value\":100.0}]]]" + }, + { + "id": "3468206642", + "name": "Gryffindors", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"house\", \"type\": \"custom_attribute\", \"match\":\"exact\", \"value\":\"Gryffindor\"}]]]" + }, + { + "id": "3988293898", + "name": "Slytherins", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"house\", \"type\": \"custom_attribute\", \"match\":\"substring\", \"value\":\"Slytherin\"}]]]" + }, + { + "id": "4194404272", + "name": "english_citizens", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"nationality\", \"type\": \"custom_attribute\", \"match\":\"exact\", \"value\":\"English\"}]]]" + }, + { + "id": "2196265320", + "name": "audience_with_missing_value", + "conditions": "[\"and\", [\"or\", [\"or\", {\"name\": \"nationality\", \"type\": \"custom_attribute\", \"value\": \"English\"}, {\"name\": \"nationality\", \"type\": \"custom_attribute\"}]]]" + } + ], "attributes": [ { "id": "553339214", From 20dddcf147ae17d714088875c0c22a48494a66cc Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 13 Sep 2018 13:43:14 -0700 Subject: [PATCH 22/32] trying to get travis jdk 9 failures --- .travis.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.travis.yml b/.travis.yml index 74d2f7d2d..469ccd649 100644 --- a/.travis.yml +++ b/.travis.yml @@ -26,3 +26,6 @@ branches: - /^\d+\.\d+\.\d+(-SNAPSHOT|-alpha|-beta)?\d*$/ # trigger builds on tags which are semantically versioned to ship the SDK. after_success: - ./gradlew coveralls uploadArchives --console plain +after_failure: + - cat /home/travis/build/optimizely/java-sdk/core-api/build/reports/findbugs/main.html + From 51e18c34315ddab9fc293d2bc1d40c1143756c41 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 13 Sep 2018 14:00:02 -0700 Subject: [PATCH 23/32] allow null to be returned from evaluate --- .../java/com/optimizely/ab/config/audience/UserAttribute.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index b8127f959..d8f32ab36 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -57,7 +57,7 @@ public Object getValue() { return value; } - public Boolean evaluate(Map attributes) { + public @Nullable Boolean evaluate(Map attributes) { // Valid for primative types, but needs to change when a value is an object or an array Object userAttributeValue = attributes.get(name); From 685982e27883fef31a105954743b525a3382e098 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Thu, 13 Sep 2018 16:35:25 -0700 Subject: [PATCH 24/32] added unit tests to test AND and OR conditions with null, false, and true --- .../AudienceConditionEvaluationTest.java | 108 ++++++++++++++++++ 1 file changed, 108 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 63a376989..343e198a7 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -377,6 +377,72 @@ public void orConditionEvaluateTrue() throws Exception { verify(userAttribute2, times(0)).evaluate(testUserAttributes); } + /** + * Verify that OrCondition.evaluate returns true when at least one of its operand conditions evaluate to true. + */ + @Test + public void orConditionEvaluateTrueWithNullAndTrue() throws Exception { + UserAttribute userAttribute1 = mock(UserAttribute.class); + when(userAttribute1.evaluate(testUserAttributes)).thenReturn(null); + + UserAttribute userAttribute2 = mock(UserAttribute.class); + when(userAttribute2.evaluate(testUserAttributes)).thenReturn(true); + + List conditions = new ArrayList(); + conditions.add(userAttribute1); + conditions.add(userAttribute2); + + OrCondition orCondition = new OrCondition(conditions); + assertTrue(orCondition.evaluate(testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(testUserAttributes); + // shouldn't be called due to short-circuiting in 'Or' evaluation + verify(userAttribute2, times(1)).evaluate(testUserAttributes); + } + + /** + * Verify that OrCondition.evaluate returns true when at least one of its operand conditions evaluate to true. + */ + @Test + public void orConditionEvaluateNullWithNullAndFalse() throws Exception { + UserAttribute userAttribute1 = mock(UserAttribute.class); + when(userAttribute1.evaluate(testUserAttributes)).thenReturn(null); + + UserAttribute userAttribute2 = mock(UserAttribute.class); + when(userAttribute2.evaluate(testUserAttributes)).thenReturn(false); + + List conditions = new ArrayList(); + conditions.add(userAttribute1); + conditions.add(userAttribute2); + + OrCondition orCondition = new OrCondition(conditions); + assertNull(orCondition.evaluate(testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(testUserAttributes); + // shouldn't be called due to short-circuiting in 'Or' evaluation + verify(userAttribute2, times(1)).evaluate(testUserAttributes); + } + + /** + * Verify that OrCondition.evaluate returns true when at least one of its operand conditions evaluate to true. + */ + @Test + public void orConditionEvaluateFalseWithFalseAndFalse() throws Exception { + UserAttribute userAttribute1 = mock(UserAttribute.class); + when(userAttribute1.evaluate(testUserAttributes)).thenReturn(false); + + UserAttribute userAttribute2 = mock(UserAttribute.class); + when(userAttribute2.evaluate(testUserAttributes)).thenReturn(false); + + List conditions = new ArrayList(); + conditions.add(userAttribute1); + conditions.add(userAttribute2); + + OrCondition orCondition = new OrCondition(conditions); + assertFalse(orCondition.evaluate(testUserAttributes)); + verify(userAttribute1, times(1)).evaluate(testUserAttributes); + // shouldn't be called due to short-circuiting in 'Or' evaluation + verify(userAttribute2, times(1)).evaluate(testUserAttributes); + } + /** * Verify that OrCondition.evaluate returns false when all of its operand conditions evaluate to false. */ @@ -419,6 +485,48 @@ public void andConditionEvaluateTrue() throws Exception { verify(orCondition2, times(1)).evaluate(testUserAttributes); } + /** + * Verify that AndCondition.evaluate returns true when all of its operand conditions evaluate to true. + */ + @Test + public void andConditionEvaluateFalseWithNullAndFalse() throws Exception { + OrCondition orCondition1 = mock(OrCondition.class); + when(orCondition1.evaluate(testUserAttributes)).thenReturn(null); + + OrCondition orCondition2 = mock(OrCondition.class); + when(orCondition2.evaluate(testUserAttributes)).thenReturn(false); + + List conditions = new ArrayList(); + conditions.add(orCondition1); + conditions.add(orCondition2); + + AndCondition andCondition = new AndCondition(conditions); + assertFalse(andCondition.evaluate(testUserAttributes)); + verify(orCondition1, times(1)).evaluate(testUserAttributes); + verify(orCondition2, times(1)).evaluate(testUserAttributes); + } + + /** + * Verify that AndCondition.evaluate returns true when all of its operand conditions evaluate to true. + */ + @Test + public void andConditionEvaluateNullWithNullAndTrue() throws Exception { + OrCondition orCondition1 = mock(OrCondition.class); + when(orCondition1.evaluate(testUserAttributes)).thenReturn(null); + + OrCondition orCondition2 = mock(OrCondition.class); + when(orCondition2.evaluate(testUserAttributes)).thenReturn(true); + + List conditions = new ArrayList(); + conditions.add(orCondition1); + conditions.add(orCondition2); + + AndCondition andCondition = new AndCondition(conditions); + assertNull(andCondition.evaluate(testUserAttributes)); + verify(orCondition1, times(1)).evaluate(testUserAttributes); + verify(orCondition2, times(1)).evaluate(testUserAttributes); + } + /** * Verify that AndCondition.evaluate returns false when any one of its operand conditions evaluate to false. */ From 01021d6a3f4be872a0b9e2f1796cf6303b4b158c Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Fri, 14 Sep 2018 17:54:43 -0700 Subject: [PATCH 25/32] add typed_audience_experiment --- .../ab/config/ValidProjectConfigV4.java | 45 +++++++++++++++++++ .../config/valid-project-config-v4.json | 30 +++++++++++++ 2 files changed, 75 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java index b3a031785..d4068a79b 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ValidProjectConfigV4.java @@ -431,6 +431,50 @@ public class ValidProjectConfigV4 { ) ) ); + private static final String LAYER_TYPEDAUDIENCE_EXPERIMENT_ID = "1630555627"; + private static final String EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_ID = "1323241597"; + public static final String EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_KEY = "typed_audience_experiment"; + private static final String VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A_ID = "1423767503"; + private static final String VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A_KEY = "A"; + private static final Variation VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A = new Variation( + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A_ID, + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A_KEY, + Collections.emptyList() + ); + private static final String VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B_ID = "3433458315"; + private static final String VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B_KEY = "B"; + private static final Variation VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B = new Variation( + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B_ID, + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B_KEY, + Collections.emptyList() + ); + + private static final Experiment EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT = new Experiment( + EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_ID, + EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_KEY, + Experiment.ExperimentStatus.RUNNING.toString(), + LAYER_TYPEDAUDIENCE_EXPERIMENT_ID, + ProjectConfigTestUtils.createListOfObjects( + AUDIENCE_BOOL_ID, + AUDIENCE_INT_ID, + AUDIENCE_DOUBLE_ID + ), + ProjectConfigTestUtils.createListOfObjects( + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A, + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B + ), + Collections.EMPTY_MAP, + ProjectConfigTestUtils.createListOfObjects( + new TrafficAllocation( + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_A_ID, + 5000 + ), + new TrafficAllocation( + VARIATION_TYPEDAUDIENCE_EXPERIMENT_VARIATION_B_ID, + 10000 + ) + ) + ); private static final String LAYER_FIRST_GROUPED_EXPERIMENT_ID = "3301900159"; private static final String EXPERIMENT_FIRST_GROUPED_EXPERIMENT_ID = "2738374745"; private static final String EXPERIMENT_FIRST_GROUPED_EXPERIMENT_KEY = "first_grouped_experiment"; @@ -1140,6 +1184,7 @@ public static ProjectConfig generateValidProjectConfigV4() { // list experiments List experiments = new ArrayList(); experiments.add(EXPERIMENT_BASIC_EXPERIMENT); + experiments.add(EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT); experiments.add(EXPERIMENT_MULTIVARIATE_EXPERIMENT); experiments.add(EXPERIMENT_DOUBLE_FEATURE_EXPERIMENT); experiments.add(EXPERIMENT_PAUSED_EXPERIMENT); diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index 3ae4aef58..6318d62d6 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -151,6 +151,36 @@ "Tom Riddle": "B" } }, + { + "id": "1323241597", + "key": "typed_audience_experiment", + "layerId": "1630555627", + "status": "Running", + "variations": [ + { + "id": "1423767503", + "key": "A", + "variables": [] + }, + { + "id": "3433458315", + "key": "B", + "variables": [] + } + ], + "trafficAllocation": [ + { + "entityId": "1423767503", + "endOfRange": 5000 + }, + { + "entityId": "3433458315", + "endOfRange": 10000 + } + ], + "audienceIds": ["3468206643","3468206644","3468206645"], + "forcedVariations": {} + }, { "id": "3262035800", "key": "multivariate_experiment", From 03ab597929c0b7625d7d1fb5465738f98f7d37a8 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Fri, 14 Sep 2018 18:05:47 -0700 Subject: [PATCH 26/32] added tests using datafile with typed audience with experiment using typed audience. --- .../com/optimizely/ab/OptimizelyTest.java | 180 ++++++++++++++++++ 1 file changed, 180 insertions(+) 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 22c3ffeca..11800a4f7 100644 --- a/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java +++ b/core-api/src/test/java/com/optimizely/ab/OptimizelyTest.java @@ -219,6 +219,186 @@ public void activateEndToEnd() throws Exception { verify(mockEventHandler).dispatchEvent(logEventToDispatch); } + /** + * Verify that the {@link Optimizely#activate(Experiment, String, Map)} call correctly builds an endpoint url and + * request params and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. + */ + @Test + public void activateEndToEndWithTypedAudienceInt() throws Exception { + Experiment activatedExperiment; + Map testUserAttributes = new HashMap(); + String bucketingKey = testBucketingIdKey; + String userId = testUserId; + String bucketingId = testBucketingId; + if(datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_KEY); + testUserAttributes.put(ATTRIBUTE_INTEGER_KEY, 2); // should be gt 1. + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + testUserAttributes.put("browser_type", "chrome"); + } + testUserAttributes.put(bucketingKey, bucketingId); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventFactory mockEventFactory = mock(EventFactory.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventFactory) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + + when(mockEventFactory.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, testUserId, + testUserAttributes)) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, bucketingId)) + .thenReturn(bucketedVariation); + + logbackVerifier.expectMessage(Level.DEBUG, String.format("No variation for experiment \"%s\" mapped to user \"%s\" in the forced variation map", activatedExperiment.getKey(), testUserId)); + + logbackVerifier.expectMessage(Level.DEBUG, "BucketingId is valid: \"bucketingId\""); + + logbackVerifier.expectMessage(Level.INFO, "This decision will not be saved since the UserProfileService is null."); + + logbackVerifier.expectMessage(Level.INFO, "Activating user \"userId\" in experiment \"" + + activatedExperiment.getKey() + "\"."); + logbackVerifier.expectMessage(Level.DEBUG, "Dispatching impression event to URL test_url with params " + + testParams + " and payload \"{}\""); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), userId, testUserAttributes); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, bucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + + /** + * Verify that the {@link Optimizely#activate(Experiment, String, Map)} call correctly builds an endpoint url and + * request params and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. + */ + @Test + public void activateEndToEndWithTypedAudienceBool() throws Exception { + Experiment activatedExperiment; + Map testUserAttributes = new HashMap(); + String bucketingKey = testBucketingIdKey; + String userId = testUserId; + String bucketingId = testBucketingId; + if(datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_KEY); + testUserAttributes.put(ATTRIBUTE_BOOLEAN_KEY, true); // should be eq true. + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + testUserAttributes.put("browser_type", "chrome"); + } + testUserAttributes.put(bucketingKey, bucketingId); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventFactory mockEventFactory = mock(EventFactory.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventFactory) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + + when(mockEventFactory.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, testUserId, + testUserAttributes)) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, bucketingId)) + .thenReturn(bucketedVariation); + + logbackVerifier.expectMessage(Level.DEBUG, String.format("No variation for experiment \"%s\" mapped to user \"%s\" in the forced variation map", activatedExperiment.getKey(), testUserId)); + + logbackVerifier.expectMessage(Level.DEBUG, "BucketingId is valid: \"bucketingId\""); + + logbackVerifier.expectMessage(Level.INFO, "This decision will not be saved since the UserProfileService is null."); + + logbackVerifier.expectMessage(Level.INFO, "Activating user \"userId\" in experiment \"" + + activatedExperiment.getKey() + "\"."); + logbackVerifier.expectMessage(Level.DEBUG, "Dispatching impression event to URL test_url with params " + + testParams + " and payload \"{}\""); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), userId, testUserAttributes); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, bucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + + /** + * Verify that the {@link Optimizely#activate(Experiment, String, Map)} call correctly builds an endpoint url and + * request params and passes them through {@link EventHandler#dispatchEvent(LogEvent)}. + */ + @Test + public void activateEndToEndWithTypedAudienceDouble() throws Exception { + Experiment activatedExperiment; + Map testUserAttributes = new HashMap(); + String bucketingKey = testBucketingIdKey; + String userId = testUserId; + String bucketingId = testBucketingId; + if(datafileVersion >= 4) { + activatedExperiment = validProjectConfig.getExperimentKeyMapping().get(EXPERIMENT_TYPEDAUDIENCE_EXPERIMENT_KEY); + testUserAttributes.put(ATTRIBUTE_DOUBLE_KEY, 99.9); // should be lt 100. + } + else { + activatedExperiment = validProjectConfig.getExperiments().get(0); + testUserAttributes.put("browser_type", "chrome"); + } + testUserAttributes.put(bucketingKey, bucketingId); + Variation bucketedVariation = activatedExperiment.getVariations().get(0); + EventFactory mockEventFactory = mock(EventFactory.class); + + Optimizely optimizely = Optimizely.builder(validDatafile, mockEventHandler) + .withBucketing(mockBucketer) + .withEventBuilder(mockEventFactory) + .withConfig(validProjectConfig) + .withErrorHandler(mockErrorHandler) + .build(); + + + when(mockEventFactory.createImpressionEvent(validProjectConfig, activatedExperiment, bucketedVariation, testUserId, + testUserAttributes)) + .thenReturn(logEventToDispatch); + + when(mockBucketer.bucket(activatedExperiment, bucketingId)) + .thenReturn(bucketedVariation); + + logbackVerifier.expectMessage(Level.DEBUG, String.format("No variation for experiment \"%s\" mapped to user \"%s\" in the forced variation map", activatedExperiment.getKey(), testUserId)); + + logbackVerifier.expectMessage(Level.DEBUG, "BucketingId is valid: \"bucketingId\""); + + logbackVerifier.expectMessage(Level.INFO, "This decision will not be saved since the UserProfileService is null."); + + logbackVerifier.expectMessage(Level.INFO, "Activating user \"userId\" in experiment \"" + + activatedExperiment.getKey() + "\"."); + logbackVerifier.expectMessage(Level.DEBUG, "Dispatching impression event to URL test_url with params " + + testParams + " and payload \"{}\""); + + // activate the experiment + Variation actualVariation = optimizely.activate(activatedExperiment.getKey(), userId, testUserAttributes); + + // verify that the bucketing algorithm was called correctly + verify(mockBucketer).bucket(activatedExperiment, bucketingId); + assertThat(actualVariation, is(bucketedVariation)); + + // verify that dispatchEvent was called with the correct LogEvent object + verify(mockEventHandler).dispatchEvent(logEventToDispatch); + } + /** * Verify that the {@link Optimizely#activate(Experiment, String, Map)} DOES NOT dispatch an impression event * when the user isn't bucketed to a variation. From b24db002ad7484df4ad9fb894d8b2bd098e94ebb Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Mon, 17 Sep 2018 16:51:24 -0700 Subject: [PATCH 27/32] add exists test. Cleanup code via mike's comments. --- .../ab/config/audience/AndCondition.java | 9 +++++++-- .../optimizely/ab/config/audience/OrCondition.java | 9 ++++++--- .../ab/config/parser/JsonSimpleConfigParser.java | 4 +++- .../optimizely/ab/config/ProjectConfigTest.java | 2 +- .../ab/config/ProjectConfigTestUtils.java | 2 +- .../audience/AudienceConditionEvaluationTest.java | 14 ++++++++++++++ 6 files changed, 32 insertions(+), 8 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java index 441fa0720..852cf10a2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/AndCondition.java @@ -40,8 +40,13 @@ public List getConditions() { public @Nullable Boolean evaluate(Map attributes) { boolean foundNull = false; - // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# - // According to the matrix mentioned in the above document. + // According to the matrix where: + // false and true is false + // false and null is false + // true and null is null. + // true and false is false + // true and true is true + // null and null is null for (Condition condition : conditions) { Boolean conditionEval = condition.evaluate(attributes); if (conditionEval == null) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index 85eae8de9..9fde4b56e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -37,13 +37,16 @@ public List getConditions() { return conditions; } - // https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit# - // According to the matrix mentioned in the above document. + // According to the matrix: + // true returns true + // false or null is null + // false or false is false + // null or null is null public @Nullable Boolean evaluate(Map attributes) { boolean foundNull = false; for (Condition condition : conditions) { Boolean conditionEval = condition.evaluate(attributes); - if (conditionEval == null) {// true with falses and nulls is still true + if (conditionEval == null) { // true with falses and nulls is still true foundNull = true; } else if (conditionEval) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index 1d64ab582..bdd1380ab 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -71,10 +71,12 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse List events = parseEvents((JSONArray)rootObject.get("events")); List audiences = parseAudiences((JSONArray)parser.parse(rootObject.get("audiences").toString())); - List typedAudiences =null; + + List typedAudiences = null; if (rootObject.containsKey("typedAudiences")) { typedAudiences = parseAudiences((JSONArray)parser.parse(rootObject.get("typedAudiences").toString())); } + List groups = parseGroups((JSONArray)rootObject.get("groups")); boolean anonymizeIP = false; 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 cece09f87..68a96bf5e 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 @@ -106,7 +106,7 @@ public void verifyGetExperimentsForInvalidEvent() throws Exception { @Test public void verifyGetAudienceConditionsFromValidId() throws Exception { List userAttributes = new ArrayList(); - userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null,"firefox")); + userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null, "firefox")); OrCondition orInner = new OrCondition(userAttributes); diff --git a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java index 57d6d50cd..ac8f0a873 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java +++ b/core-api/src/test/java/com/optimizely/ab/config/ProjectConfigTestUtils.java @@ -245,7 +245,7 @@ private static ProjectConfig generateValidProjectConfigV3() { new EventType("100", "no_running_experiments", singletonList("118"))); List userAttributes = new ArrayList(); - userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null,"firefox")); + userAttributes.add(new UserAttribute("browser_type", "custom_attribute", null, "firefox")); OrCondition orInner = new OrCondition(userAttributes); diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java index 343e198a7..d841d40a4 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/AudienceConditionEvaluationTest.java @@ -110,6 +110,20 @@ public void invalidMatch() throws Exception { assertNull(testInstance.evaluate(testUserAttributes)); } + /** + * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor + * attributes with non-null instances and empty string. + */ + @Test + public void existsMatchConditionEmptyStringEvaluatesTrue() throws Exception { + UserAttribute testInstance = new UserAttribute("browser_type", "custom_attribute","exists", "firefox"); + Map attributes = new HashMap<>(); + attributes.put("browser_type", ""); + assertTrue(testInstance.evaluate(attributes)); + attributes.put("browser_type", null); + assertFalse(testInstance.evaluate(attributes)); + } + /** * Verify that UserAttribute.evaluate for EXIST match type returns true for known visitor * attributes with non-null instances From 91d075209e0b2b7505a38580fb3e9bf6bc8553a4 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 18 Sep 2018 10:19:12 -0700 Subject: [PATCH 28/32] add logging, rename custom dimension, clean up code a little --- .../ab/config/audience/UserAttribute.java | 2 ++ ...a => DefaultMatchForLegacyAttributes.java} | 6 ++--- .../ab/config/audience/match/GTMatch.java | 3 ++- .../ab/config/audience/match/LTMatch.java | 3 ++- .../ab/config/audience/match/MatchType.java | 11 +++++--- .../config/audience/match/SubstringMatch.java | 3 +++ .../parser/AudienceJacksonDeserializer.java | 16 +++++++----- .../ab/config/parser/JsonConfigParser.java | 25 ++++++++++--------- 8 files changed, 43 insertions(+), 26 deletions(-) rename core-api/src/main/java/com/optimizely/ab/config/audience/match/{CustomDimensionMatch.java => DefaultMatchForLegacyAttributes.java} (81%) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java index d8f32ab36..6bb457859 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/UserAttribute.java @@ -62,6 +62,7 @@ public Object getValue() { Object userAttributeValue = attributes.get(name); if (!"custom_attribute".equals(type)) { + MatchType.logger.error(String.format("condition type not equal to `custom_attribute` %s", type != null ? type : "")); return null; // unknown type } // check user attribute value is equal @@ -69,6 +70,7 @@ public Object getValue() { return MatchType.getMatchType(match, value).getMatcher().eval(userAttributeValue); } catch (NullPointerException np) { + MatchType.logger.error(String.format("attribute or value null for match %s", match != null ? match : "legacy condition"),np); return null; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java similarity index 81% rename from core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java rename to core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java index c4b023334..72697d45f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/CustomDimensionMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java @@ -20,12 +20,12 @@ /** * This is a temporary class. It mimics the current behaviour for - * custom dimension. This will be dropped for ExactMatch and the unit tests need to be fixed. + * legasy custom attributes. This will be dropped for ExactMatch and the unit tests need to be fixed. * @param */ -class CustomDimensionMatch extends LeafMatch { +class DefaultMatchForLegacyAttributes extends LeafMatch { T value; - protected CustomDimensionMatch(T value) { + protected DefaultMatchForLegacyAttributes(T value) { this.value = value; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java index 68b4540cc..3b98ee307 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java @@ -27,9 +27,10 @@ protected GTMatch(Number value) { public @Nullable Boolean eval(Object otherValue) { try { - return ((Number) convert(otherValue)).doubleValue() > value.doubleValue(); + return convert(otherValue).doubleValue() > value.doubleValue(); } catch (Exception e) { + MatchType.logger.error("Greater than match ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java index cfe2054fe..2f54cf474 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java @@ -27,9 +27,10 @@ protected LTMatch(Number value) { public @Nullable Boolean eval(Object otherValue) { try { - return ((Number) convert(otherValue)).doubleValue() < value.doubleValue(); + return convert(otherValue).doubleValue() < value.doubleValue(); } catch (Exception e) { + MatchType.logger.error("Less than match ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java index d6472879e..5f45d9f65 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java @@ -16,15 +16,20 @@ */ package com.optimizely.ab.config.audience.match; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import javax.annotation.Nonnull; public class MatchType { + public static final Logger logger = LoggerFactory.getLogger(MatchType.class); + private String type; private LeafMatcher matcher; public static MatchType getMatchType(String type, Object value) { - if (type == null) type = "custom_dimension"; + if (type == null) type = "legacy_custom_attribute"; switch (type) { case "exists": @@ -55,9 +60,9 @@ else if (value instanceof Boolean) { return new MatchType(type, new LTMatch((Number) value)); } break; - case "custom_dimension": + case "legacy_custom_attribute": if (value instanceof String) { - return new MatchType(type, new CustomDimensionMatch((String) value)); + return new MatchType(type, new DefaultMatchForLegacyAttributes((String) value)); } break; default: diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java index 6bf3e28e0..7822588f5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -16,6 +16,8 @@ */ package com.optimizely.ab.config.audience.match; +import com.sun.org.apache.xalan.internal.xsltc.compiler.util.MatchGenerator; + import javax.annotation.Nullable; class SubstringMatch extends LeafMatch { @@ -30,6 +32,7 @@ Boolean eval(Object otherValue) { return value.contains(convert(otherValue)); } catch (Exception e) { + MatchType.logger.error("Substring match ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java index d2017ae3c..c487136f6 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/AudienceJacksonDeserializer.java @@ -66,12 +66,16 @@ private Condition parseConditions(List rawObjectList) { } Condition condition; - if (operand.equals("and")) { - condition = new AndCondition(conditions); - } else if (operand.equals("or")) { - condition = new OrCondition(conditions); - } else { - condition = new NotCondition(conditions.get(0)); + switch (operand) { + case "and": + condition = new AndCondition(conditions); + break; + case "or": + condition = new OrCondition(conditions); + break; + default: + condition = new NotCondition(conditions.get(0)); + break; } return condition; diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index a92316852..342cfacb7 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -295,26 +295,27 @@ private Condition parseConditions(JSONArray conditionJson) { conditions.add(parseConditions(conditionJson.getJSONArray(i))); } else { JSONObject conditionMap = (JSONObject)obj; - Object value = null; - if (conditionMap.has("value")) { - value = conditionMap.get("value"); - } + Object value = conditionMap.has("value") ? conditionMap.get("value") : null; + String match = conditionMap.has("match") ? (String) conditionMap.get("match") : null; conditions.add(new UserAttribute( (String)conditionMap.get("name"), (String)conditionMap.get("type"), - conditionMap.has("match")?(String)conditionMap.get("match"):null, - value + match, value )); } } Condition condition; - if (operand.equals("and")) { - condition = new AndCondition(conditions); - } else if (operand.equals("or")) { - condition = new OrCondition(conditions); - } else { - condition = new NotCondition(conditions.get(0)); + switch (operand) { + case "and": + condition = new AndCondition(conditions); + break; + case "or": + condition = new OrCondition(conditions); + break; + default: + condition = new NotCondition(conditions.get(0)); + break; } return condition; From 9cc8dfab4ee35e286d3c7c6ee1965500dc05fff9 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 18 Sep 2018 14:08:35 -0700 Subject: [PATCH 29/32] remove unused import --- .../com/optimizely/ab/config/audience/match/SubstringMatch.java | 2 -- 1 file changed, 2 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java index 7822588f5..af4d4026c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -16,8 +16,6 @@ */ package com.optimizely.ab.config.audience.match; -import com.sun.org.apache.xalan.internal.xsltc.compiler.util.MatchGenerator; - import javax.annotation.Nullable; class SubstringMatch extends LeafMatch { From 4ba45557d860a7e708801150396de617a0979e8f Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 18 Sep 2018 16:33:32 -0700 Subject: [PATCH 30/32] nit cleanup --- .../java/com/optimizely/ab/config/audience/OrCondition.java | 3 ++- .../java/com/optimizely/ab/config/audience/match/GTMatch.java | 2 +- .../java/com/optimizely/ab/config/audience/match/LTMatch.java | 2 +- .../optimizely/ab/config/audience/match/SubstringMatch.java | 2 +- .../main/java/com/optimizely/ab/internal/ExperimentUtils.java | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java index 9fde4b56e..768f6427f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/OrCondition.java @@ -54,7 +54,8 @@ else if (conditionEval) { } } - if (foundNull) {// if found null and false return null. all false return false + // if found null and false return null. all false return false + if (foundNull) { return null; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java index 3b98ee307..64f928f16 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java @@ -30,7 +30,7 @@ Boolean eval(Object otherValue) { return convert(otherValue).doubleValue() > value.doubleValue(); } catch (Exception e) { - MatchType.logger.error("Greater than match ", e); + MatchType.logger.error("Greater than match failed ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java index 2f54cf474..b400333d1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java @@ -30,7 +30,7 @@ Boolean eval(Object otherValue) { return convert(otherValue).doubleValue() < value.doubleValue(); } catch (Exception e) { - MatchType.logger.error("Less than match ", e); + MatchType.logger.error("Less than match failed ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java index af4d4026c..7bd7fb6ed 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -30,7 +30,7 @@ Boolean eval(Object otherValue) { return value.contains(convert(otherValue)); } catch (Exception e) { - MatchType.logger.error("Substring match ", e); + MatchType.logger.error("Substring match failed ", e); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java index f520e546e..1b17b7db3 100644 --- a/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java +++ b/core-api/src/main/java/com/optimizely/ab/internal/ExperimentUtils.java @@ -70,7 +70,7 @@ public static boolean isUserInExperiment(@Nonnull ProjectConfig projectConfig, Condition conditions = projectConfig.getAudienceConditionsFromId(audienceId); Boolean conditionEval = conditions.evaluate(attributes); - if (conditionEval != null && conditionEval){ + if (conditionEval != null && conditionEval) { return true; } } From a159e4cc08f4f02cbf75d6bec2c0dd8004011805 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Wed, 19 Sep 2018 14:11:04 -0700 Subject: [PATCH 31/32] rename match interface and abstract base class. --- .travis.yml | 1 - .../java/com/optimizely/ab/config/audience/Condition.java | 2 +- .../audience/match/{LeafMatch.java => AttributeMatch.java} | 2 +- .../audience/match/DefaultMatchForLegacyAttributes.java | 4 ++-- .../com/optimizely/ab/config/audience/match/ExactMatch.java | 2 +- .../optimizely/ab/config/audience/match/ExistsMatch.java | 2 +- .../com/optimizely/ab/config/audience/match/GTMatch.java | 2 +- .../com/optimizely/ab/config/audience/match/LTMatch.java | 2 +- .../config/audience/match/{LeafMatcher.java => Match.java} | 2 +- .../com/optimizely/ab/config/audience/match/MatchType.java | 6 +++--- .../com/optimizely/ab/config/audience/match/NullMatch.java | 2 +- .../optimizely/ab/config/audience/match/SubstringMatch.java | 2 +- .../src/test/resources/config/valid-project-config-v4.json | 2 +- 13 files changed, 15 insertions(+), 16 deletions(-) rename core-api/src/main/java/com/optimizely/ab/config/audience/match/{LeafMatch.java => AttributeMatch.java} (94%) rename core-api/src/main/java/com/optimizely/ab/config/audience/match/{LeafMatcher.java => Match.java} (95%) diff --git a/.travis.yml b/.travis.yml index 469ccd649..512de767b 100644 --- a/.travis.yml +++ b/.travis.yml @@ -28,4 +28,3 @@ after_success: - ./gradlew coveralls uploadArchives --console plain after_failure: - cat /home/travis/build/optimizely/java-sdk/core-api/build/reports/findbugs/main.html - diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java index bd5c1503e..997e6922f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/Condition.java @@ -26,4 +26,4 @@ public interface Condition { @Nullable Boolean evaluate(Map attributes); -} \ No newline at end of file +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java similarity index 94% rename from core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java rename to core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java index cfeeb02c5..fa1029cec 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java @@ -16,7 +16,7 @@ */ package com.optimizely.ab.config.audience.match; -abstract class LeafMatch implements LeafMatcher { +abstract class AttributeMatch implements Match { T convert(Object o) { try { T rv = (T)o; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java index 72697d45f..dba84526e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java @@ -20,10 +20,10 @@ /** * This is a temporary class. It mimics the current behaviour for - * legasy custom attributes. This will be dropped for ExactMatch and the unit tests need to be fixed. + * legacy custom attributes. This will be dropped for ExactMatch and the unit tests need to be fixed. * @param */ -class DefaultMatchForLegacyAttributes extends LeafMatch { +class DefaultMatchForLegacyAttributes extends AttributeMatch { T value; protected DefaultMatchForLegacyAttributes(T value) { this.value = value; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java index b23359f6e..38da59bd3 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java @@ -18,7 +18,7 @@ import javax.annotation.Nullable; -class ExactMatch extends LeafMatch { +class ExactMatch extends AttributeMatch { T value; protected ExactMatch(T value) { this.value = value; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java index 66253789b..281bcec01 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java @@ -20,7 +20,7 @@ import javax.annotation.Nullable; -class ExistsMatch extends LeafMatch { +class ExistsMatch extends AttributeMatch { @SuppressFBWarnings("URF_UNREAD_FIELD") Object value; protected ExistsMatch(Object value) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java index 64f928f16..24f069693 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java @@ -18,7 +18,7 @@ import javax.annotation.Nullable; -class GTMatch extends LeafMatch { +class GTMatch extends AttributeMatch { Number value; protected GTMatch(Number value) { this.value = value; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java index b400333d1..74e5cc934 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java @@ -18,7 +18,7 @@ import javax.annotation.Nullable; -class LTMatch extends LeafMatch { +class LTMatch extends AttributeMatch { Number value; protected LTMatch(Number value) { this.value = value; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java similarity index 95% rename from core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java rename to core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java index b1a100bfa..7818986d5 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LeafMatcher.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java @@ -16,6 +16,6 @@ */ package com.optimizely.ab.config.audience.match; -public interface LeafMatcher { +public interface Match { Boolean eval(Object otherValue); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java index 5f45d9f65..a429c9366 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java @@ -26,7 +26,7 @@ public class MatchType { public static final Logger logger = LoggerFactory.getLogger(MatchType.class); private String type; - private LeafMatcher matcher; + private Match matcher; public static MatchType getMatchType(String type, Object value) { if (type == null) type = "legacy_custom_attribute"; @@ -73,13 +73,13 @@ else if (value instanceof Boolean) { } - private MatchType(String type, LeafMatcher matcher) { + private MatchType(String type, Match matcher) { this.type = type; this.matcher = matcher; } public @Nonnull - LeafMatcher getMatcher() { + Match getMatcher() { return matcher; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java index 8b825cd9f..6d2bb9889 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java @@ -20,7 +20,7 @@ import javax.annotation.Nullable; -class NullMatch extends LeafMatch { +class NullMatch extends AttributeMatch { @SuppressFBWarnings("URF_UNREAD_FIELD") Object value; protected NullMatch() { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java index 7bd7fb6ed..1ebcac2b2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -18,7 +18,7 @@ import javax.annotation.Nullable; -class SubstringMatch extends LeafMatch { +class SubstringMatch extends AttributeMatch { String value; protected SubstringMatch(String value) { this.value = value; diff --git a/core-api/src/test/resources/config/valid-project-config-v4.json b/core-api/src/test/resources/config/valid-project-config-v4.json index 6318d62d6..4abcd69f8 100644 --- a/core-api/src/test/resources/config/valid-project-config-v4.json +++ b/core-api/src/test/resources/config/valid-project-config-v4.json @@ -178,7 +178,7 @@ "endOfRange": 10000 } ], - "audienceIds": ["3468206643","3468206644","3468206645"], + "audienceIds": ["3468206643", "3468206644", "3468206645"], "forcedVariations": {} }, { From 7c633389d611f2c927595442bfdaa4704de5a8d8 Mon Sep 17 00:00:00 2001 From: Thomas Zurkan Date: Tue, 2 Oct 2018 12:50:41 -0700 Subject: [PATCH 32/32] refactor to incorporate Nikhil's comments --- .../config/audience/match/AttributeMatch.java | 4 ++ .../DefaultMatchForLegacyAttributes.java | 4 +- .../ab/config/audience/match/ExactMatch.java | 6 +-- .../ab/config/audience/match/ExistsMatch.java | 4 +- .../ab/config/audience/match/GTMatch.java | 4 +- .../ab/config/audience/match/LTMatch.java | 4 +- .../ab/config/audience/match/Match.java | 4 +- .../ab/config/audience/match/MatchType.java | 46 +++++++++---------- .../ab/config/audience/match/NullMatch.java | 2 +- .../config/audience/match/SubstringMatch.java | 4 +- .../ab/config/parser/JsonConfigParser.java | 8 +++- .../config/parser/JsonSimpleConfigParser.java | 7 ++- .../parser/ProjectConfigGsonDeserializer.java | 7 ++- .../ProjectConfigJacksonDeserializer.java | 10 +++- 14 files changed, 70 insertions(+), 44 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java index fa1029cec..c0fd6dd07 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java @@ -22,6 +22,10 @@ T convert(Object o) { T rv = (T)o; return rv; } catch(java.lang.ClassCastException e) { + MatchType.logger.error( + "Cannot evaluate targeting condition since the value for attribute is an incompatible type", + e + ); return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java index dba84526e..493d3551c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.java @@ -30,7 +30,7 @@ protected DefaultMatchForLegacyAttributes(T value) { } public @Nullable - Boolean eval(Object otherValue) { - return value.equals(convert(otherValue)); + Boolean eval(Object attributeValue) { + return value.equals(convert(attributeValue)); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java index 38da59bd3..0f7e51a48 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactMatch.java @@ -25,8 +25,8 @@ protected ExactMatch(T value) { } public @Nullable - Boolean eval(Object otherValue) { - if (!value.getClass().isInstance(otherValue)) return null; - return value.equals(convert(otherValue)); + Boolean eval(Object attributeValue) { + if (!value.getClass().isInstance(attributeValue)) return null; + return value.equals(convert(attributeValue)); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java index 281bcec01..8434188cd 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExistsMatch.java @@ -28,7 +28,7 @@ protected ExistsMatch(Object value) { } public @Nullable - Boolean eval(Object otherValue) { - return otherValue != null; + Boolean eval(Object attributeValue) { + return attributeValue != null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java index 24f069693..354a5e04f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GTMatch.java @@ -25,9 +25,9 @@ protected GTMatch(Number value) { } public @Nullable - Boolean eval(Object otherValue) { + Boolean eval(Object attributeValue) { try { - return convert(otherValue).doubleValue() > value.doubleValue(); + return convert(attributeValue).doubleValue() > value.doubleValue(); } catch (Exception e) { MatchType.logger.error("Greater than match failed ", e); diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java index 74e5cc934..ee71694dc 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LTMatch.java @@ -25,9 +25,9 @@ protected LTMatch(Number value) { } public @Nullable - Boolean eval(Object otherValue) { + Boolean eval(Object attributeValue) { try { - return convert(otherValue).doubleValue() < value.doubleValue(); + return convert(attributeValue).doubleValue() < value.doubleValue(); } catch (Exception e) { MatchType.logger.error("Less than match failed ", e); diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java index 7818986d5..c0506ee4f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/Match.java @@ -16,6 +16,8 @@ */ package com.optimizely.ab.config.audience.match; +import javax.annotation.Nullable; + public interface Match { - Boolean eval(Object otherValue); + @Nullable Boolean eval(Object attributeValue); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java index a429c9366..064fc543c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java @@ -25,56 +25,56 @@ public class MatchType { public static final Logger logger = LoggerFactory.getLogger(MatchType.class); - private String type; + private String matchType; private Match matcher; - public static MatchType getMatchType(String type, Object value) { - if (type == null) type = "legacy_custom_attribute"; + public static MatchType getMatchType(String matchType, Object conditionValue) { + if (matchType == null) matchType = "legacy_custom_attribute"; - switch (type) { + switch (matchType) { case "exists": - return new MatchType(type, new ExistsMatch(value)); + return new MatchType(matchType, new ExistsMatch(conditionValue)); case "exact": - if (value instanceof String) { - return new MatchType(type, new ExactMatch((String) value)); + if (conditionValue instanceof String) { + return new MatchType(matchType, new ExactMatch((String) conditionValue)); } - else if (value instanceof Number) { - return new MatchType(type, new ExactMatch((Number) value)); + else if (conditionValue instanceof Integer || conditionValue instanceof Double) { + return new MatchType(matchType, new ExactMatch((Number) conditionValue)); } - else if (value instanceof Boolean) { - return new MatchType(type, new ExactMatch((Boolean) value)); + else if (conditionValue instanceof Boolean) { + return new MatchType(matchType, new ExactMatch((Boolean) conditionValue)); } break; case "substring": - if (value instanceof String) { - return new MatchType(type, new SubstringMatch((String) value)); + if (conditionValue instanceof String) { + return new MatchType(matchType, new SubstringMatch((String) conditionValue)); } break; case "gt": - if (value instanceof Number) { - return new MatchType(type, new GTMatch((Number) value)); + if (conditionValue instanceof Integer || conditionValue instanceof Double) { + return new MatchType(matchType, new GTMatch((Number) conditionValue)); } break; case "lt": - if (value instanceof Number) { - return new MatchType(type, new LTMatch((Number) value)); + if (conditionValue instanceof Integer || conditionValue instanceof Double) { + return new MatchType(matchType, new LTMatch((Number) conditionValue)); } break; case "legacy_custom_attribute": - if (value instanceof String) { - return new MatchType(type, new DefaultMatchForLegacyAttributes((String) value)); + if (conditionValue instanceof String) { + return new MatchType(matchType, new DefaultMatchForLegacyAttributes((String) conditionValue)); } break; default: - return new MatchType(type, new NullMatch()); + return new MatchType(matchType, new NullMatch()); } - return new MatchType(type, new NullMatch()); + return new MatchType(matchType, new NullMatch()); } private MatchType(String type, Match matcher) { - this.type = type; + this.matchType = type; this.matcher = matcher; } @@ -85,6 +85,6 @@ Match getMatcher() { @Override public String toString() { - return type; + return matchType; } } \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java index 6d2bb9889..3da47a8fc 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NullMatch.java @@ -28,7 +28,7 @@ protected NullMatch() { } public @Nullable - Boolean eval(Object otherValue) { + Boolean eval(Object attributeValue) { return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java index 1ebcac2b2..69d8c91d9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SubstringMatch.java @@ -25,9 +25,9 @@ protected SubstringMatch(String value) { } public @Nullable - Boolean eval(Object otherValue) { + Boolean eval(Object attributeValue) { try { - return value.contains(convert(otherValue)); + return value.contains(convert(attributeValue)); } catch (Exception e) { MatchType.logger.error("Substring match failed ", e); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java index 342cfacb7..2394698d1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonConfigParser.java @@ -41,6 +41,7 @@ import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -68,7 +69,12 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse attributes = parseAttributes(rootObject.getJSONArray("attributes")); List events = parseEvents(rootObject.getJSONArray("events")); - List audiences = parseAudiences(rootObject.getJSONArray("audiences")); + List audiences = Collections.emptyList(); + + if (rootObject.has("audiences")) { + audiences = parseAudiences(rootObject.getJSONArray("audiences")); + } + List typedAudiences = null; if (rootObject.has("typedAudiences")) { typedAudiences = parseAudiences(rootObject.getJSONArray("typedAudiences")); diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java index bdd1380ab..fbbdab578 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/JsonSimpleConfigParser.java @@ -43,6 +43,7 @@ import javax.annotation.Nonnull; import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.List; import java.util.Map; @@ -70,7 +71,11 @@ public ProjectConfig parseProjectConfig(@Nonnull String json) throws ConfigParse attributes = parseAttributes((JSONArray)rootObject.get("attributes")); List events = parseEvents((JSONArray)rootObject.get("events")); - List audiences = parseAudiences((JSONArray)parser.parse(rootObject.get("audiences").toString())); + List audiences = Collections.emptyList(); + + if (rootObject.containsKey("audiences")) { + audiences = parseAudiences((JSONArray)parser.parse(rootObject.get("audiences").toString())); + } List typedAudiences = null; if (rootObject.containsKey("typedAudiences")) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java index c3305fd5c..b2e848137 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigGsonDeserializer.java @@ -33,6 +33,7 @@ import com.optimizely.ab.config.audience.Audience; import java.lang.reflect.Type; +import java.util.Collections; import java.util.List; /** @@ -67,8 +68,10 @@ public ProjectConfig deserialize(JsonElement json, Type typeOfT, JsonDeserializa List events = context.deserialize(jsonObject.get("events").getAsJsonArray(), eventsType); - List audiences = - context.deserialize(jsonObject.get("audiences").getAsJsonArray(), audienceType); + List audiences = Collections.emptyList(); + if (jsonObject.has("audiences")) { + audiences = context.deserialize(jsonObject.get("audiences").getAsJsonArray(), audienceType); + } List typedAudiences = null; if (jsonObject.has("typedAudiences")) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java index 4e28afaa2..164876b39 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java +++ b/core-api/src/main/java/com/optimizely/ab/config/parser/ProjectConfigJacksonDeserializer.java @@ -34,6 +34,7 @@ import com.optimizely.ab.config.audience.Audience; import java.io.IOException; +import java.util.Collections; import java.util.List; class ProjectConfigJacksonDeserializer extends JsonDeserializer { @@ -63,8 +64,13 @@ public ProjectConfig deserialize(JsonParser parser, DeserializationContext conte List events = mapper.readValue(node.get("events").toString(), new TypeReference>() {}); - List audiences = mapper.readValue(node.get("audiences").toString(), - new TypeReference>() {}); + + List audiences = Collections.emptyList(); + + if (node.has("audiences")) { + audiences = mapper.readValue(node.get("audiences").toString(), + new TypeReference>() {}); + } List typedAudiences = null;