New audience match types (#213)

* update with some thoughts on how to implement new match types.

* add exists

* get the lt gt logic correct

* suppress unread field in 'exists' match type

* nullmatch, substringmatch implementaiton

* unit tests

* 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.

* unit test descriptions

* update condition evaluation to return null under matrix given here https://docs.google.com/document/d/158_83difXVXF0nb91rxzrfHZwnhsybH21ImRA_si7sg/edit#

* some unit tests (and)

* add new match type?

* update tests after evaluation of audience with no attributes

* bring up test coverage

* refactor for java standard 1 class per file.

* trying to get lint to pass java 9

* trying to pass findbugs java 9

* just seeing if I can get java 9 to pass again

* trying to get travis to pass

* i don't know why findbugs is finding problems in java 9

* allow for typedAudiences to exist or not exist in the datafile.

* added typed attributes to v4 datafile parse and compare them.  Need to write more unit tests that use them

* trying to get travis jdk 9 failures

* allow null to be returned from evaluate

* added unit tests to test AND and OR conditions with null, false, and true

* add typed_audience_experiment

* added tests using datafile with typed audience with experiment using typed audience.

* add exists test. Cleanup code via mike's comments.

* add logging, rename custom dimension, clean up code a little

* remove unused import

* nit cleanup

* rename match interface and abstract base class.

* refactor to incorporate Nikhil's comments

Author: thomaszurkan-optimizely

.travis.yml

@@ -26,3 +26,5 @@ branches:
   - /^\d+\.\d+\.(\d|[x])+(-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

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<Attribute> attributes;
     private final List<Audience> audiences;
+    private final List<Audience> typedAudiences;
     private final List<EventType> events;
     private final List<Experiment> experiments;
     private final List<FeatureFlag> 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<Attribute> attributes,
                          List<Audience> audiences,
+                         List<Audience> typedAudiences,
                          List<EventType> events,
                          List<Experiment> experiments,
                          List<FeatureFlag> 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<Audience> 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<Audience> getAudiences() {
         return audiences;
     }
 
+    public List<Audience> getTypedAudiences() {
+        return typedAudiences;
+    }
+
     public Condition getAudienceConditionsFromId(String audienceId) {
         Audience audience = audienceIdMapping.get(audienceId);
 
@@ -581,6 +603,7 @@ public String toString() {
                 ", botFiltering=" + botFiltering +
                 ", attributes=" + attributes +
                 ", audiences=" + audiences +
+                ", typedAudiences=" + typedAudiences +
                 ", events=" + events +
                 ", experiments=" + experiments +
                 ", featureFlags=" + featureFlags +

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,32 @@ public List<Condition> getConditions() {
         return conditions;
     }
 
-    public boolean evaluate(Map<String, ?> attributes) {
+    public @Nullable
+    Boolean evaluate(Map<String, ?> attributes) {
+        boolean foundNull = false;
+        // 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) {
-            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

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

@@ -16,12 +16,14 @@
  */
 package com.optimizely.ab.config.audience;
 
+import javax.annotation.Nullable;
 import java.util.Map;
 
 /**
  * Interface implemented by all conditions condition objects to aid in condition evaluation.
  */
 public interface Condition {
 
-    boolean evaluate(Map<String, ?> attributes);
-}
+    @Nullable
+    Boolean evaluate(Map<String, ?> attributes);
+}

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<String, ?> attributes) {
-        return !condition.evaluate(attributes);
+    public @Nullable Boolean evaluate(Map<String, ?> attributes) {
+        Boolean conditionEval = condition.evaluate(attributes);
+        return (conditionEval == null ? null : !conditionEval);
     }
 
     @Override

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,26 @@ public List<Condition> getConditions() {
         return conditions;
     }
 
-    public boolean evaluate(Map<String, ?> attributes) {
+    // 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<String, ?> 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 found null and false return null.  all false return false
+        if (foundNull) {
+            return null;
         }
 
         return false;

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;
@@ -29,11 +31,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,25 +49,31 @@ public String getType() {
         return type;
     }
 
+    public String getMatch() {
+        return match;
+    }
+
     public Object getValue() {
         return value;
     }
 
-    public boolean evaluate(Map<String, ?> attributes) {
+    public @Nullable Boolean evaluate(Map<String, ?> 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);
+        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
         }
-        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;
+        // check user attribute value is equal
+        try {
+            return MatchType.getMatchType(match, value).getMatcher().eval(userAttributeValue);
         }
-        else { // both are null
-            return true;
+        catch (NullPointerException np) {
+            MatchType.logger.error(String.format("attribute or value null for match %s", match != null ? match : "legacy condition"),np);
+            return null;
         }
+
     }
 
     @Override

core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.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;
+
+abstract class AttributeMatch<T> implements Match {
+    T convert(Object o) {
+        try {
+            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;
+        }
+    }
+}

core-api/src/main/java/com/optimizely/ab/config/audience/match/DefaultMatchForLegacyAttributes.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
+ * legacy custom attributes.  This will be dropped for ExactMatch and the unit tests need to be fixed.
+ * @param <T>
+ */
+class DefaultMatchForLegacyAttributes<T> extends AttributeMatch<T> {
+    T value;
+    protected DefaultMatchForLegacyAttributes(T value) {
+        this.value = value;
+    }
+
+    public @Nullable
+    Boolean eval(Object attributeValue) {
+        return value.equals(convert(attributeValue));
+    }
+}

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<T> extends AttributeMatch<T> {
+    T value;
+    protected ExactMatch(T value) {
+        this.value = value;
+    }
+
+    public @Nullable
+    Boolean eval(Object attributeValue) {
+        if (!value.getClass().isInstance(attributeValue)) return null;
+        return value.equals(convert(attributeValue));
+    }
+}