From 7aeb262539abd0126c89cd98f0be1ee42925ac30 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Thu, 6 Aug 2020 18:08:42 -0700 Subject: [PATCH 01/15] Add registry. --- .../ab/config/audience/UserAttribute.java | 17 +-- .../config/audience/match/AttributeMatch.java | 33 ----- .../DefaultMatchForLegacyAttributes.java | 14 +- .../ab/config/audience/match/ExactMatch.java | 44 +++++-- .../audience/match/ExactNumberMatch.java | 47 ------- .../ab/config/audience/match/ExistsMatch.java | 9 +- .../ab/config/audience/match/GTMatch.java | 12 +- .../ab/config/audience/match/LTMatch.java | 12 +- .../ab/config/audience/match/Match.java | 2 +- .../config/audience/match/MatchRegistry.java | 42 ++++++ .../ab/config/audience/match/MatchType.java | 124 ------------------ .../config/audience/match/SubstringMatch.java | 18 ++- .../AudienceConditionEvaluationTest.java | 4 +- 13 files changed, 119 insertions(+), 259 deletions(-) delete mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java delete mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactNumberMatch.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java delete mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java 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 be1e11169..e2e4a0e66 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 @@ -20,10 +20,7 @@ import com.fasterxml.jackson.annotation.JsonIgnoreProperties; import com.fasterxml.jackson.annotation.JsonProperty; import com.optimizely.ab.config.ProjectConfig; -import com.optimizely.ab.config.audience.match.Match; -import com.optimizely.ab.config.audience.match.MatchType; -import com.optimizely.ab.config.audience.match.UnexpectedValueTypeException; -import com.optimizely.ab.config.audience.match.UnknownMatchTypeException; +import com.optimizely.ab.config.audience.match.*; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -87,8 +84,8 @@ public Boolean evaluate(ProjectConfig config, Map attributes) { } // check user attribute value is equal try { - Match matchType = MatchType.getMatchType(match, value).getMatcher(); - Boolean result = matchType.eval(userAttributeValue); + Match matcher = MatchRegistry.getMatch(match); + Boolean result = matcher.eval(value, userAttributeValue); if (result == null) { if (!attributes.containsKey(name)) { @@ -111,11 +108,11 @@ public Boolean evaluate(ProjectConfig config, Map attributes) { } } return result; - } catch (UnknownMatchTypeException | UnexpectedValueTypeException ex) { - logger.warn("Audience condition \"{}\" " + ex.getMessage(), + } catch (UnknownMatchTypeException e) { + logger.warn("Audience condition \"{}\" " + e.getMessage(), this); - } catch (NullPointerException np) { - logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", np); + } catch (NullPointerException e) { + logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", e); } return null; } 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 deleted file mode 100644 index e2f413c4e..000000000 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/AttributeMatch.java +++ /dev/null @@ -1,33 +0,0 @@ -/** - * - * Copyright 2018-2019, 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 implements Match { - T castToValueType(Object o, Object value) { - try { - if (!o.getClass().isInstance(value) && !(o instanceof Number && value instanceof Number)) { - return null; - } - - T rv = (T) o; - - return rv; - } catch (Exception 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 cb2dfa671..9ad7f7a19 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 @@ -21,17 +21,17 @@ /** * 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 */ -class DefaultMatchForLegacyAttributes extends AttributeMatch { - T value; +class DefaultMatchForLegacyAttributes implements Match { - protected DefaultMatchForLegacyAttributes(T value) { - this.value = value; + protected DefaultMatchForLegacyAttributes() { } @Nullable - public Boolean eval(Object attributeValue) { - return value.equals(castToValueType(attributeValue, value)); + public Boolean eval(Object conditionValue, Object attributeValue) { + if (attributeValue == null) { + return false; + } + return conditionValue.toString().equals(attributeValue.toString()); } } 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 9d8d4e8c3..ed12bfa90 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,18 +18,46 @@ import javax.annotation.Nullable; -class ExactMatch extends AttributeMatch { - T value; +import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - protected ExactMatch(T value) { - this.value = value; +class ExactMatch implements Match { + + protected ExactMatch() { + } + + @Nullable + public Boolean eval(Object conditionValue, Object attributeValue) { + if (isValidNumber(attributeValue)) { + if (isValidNumber(conditionValue)) { + return evalNumber((Number) conditionValue, (Number) attributeValue); + } + return null; + } + + if (conditionValue == null) { + return attributeValue == null; + } + + if (!(conditionValue instanceof String || conditionValue instanceof Boolean)) { + return null; + } + + if (attributeValue.getClass() != conditionValue.getClass()) { + return null; + } + + return conditionValue.equals(attributeValue); } @Nullable - public Boolean eval(Object attributeValue) { - T converted = castToValueType(attributeValue, value); - if (value != null && converted == null) return null; + public Boolean evalNumber(Number conditionValue, Number attributeValue) { + try { + if(isValidNumber(attributeValue)) { + return conditionValue.doubleValue() == attributeValue.doubleValue(); + } + } catch (Exception e) { + } - return value == null ? attributeValue == null : value.equals(converted); + return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactNumberMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactNumberMatch.java deleted file mode 100644 index 56984537a..000000000 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/ExactNumberMatch.java +++ /dev/null @@ -1,47 +0,0 @@ -/** - * - * Copyright 2018-2019, 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 com.optimizely.ab.config.ProjectConfig; - -import javax.annotation.Nullable; - -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - -// Because json number is a double in most java json parsers. at this -// point we allow comparision of Integer and Double. The instance class is Double and -// Integer which would fail in our normal exact match. So, we are special casing for now. We have already filtered -// out other Number types. -public class ExactNumberMatch extends AttributeMatch { - Number value; - - protected ExactNumberMatch(Number value) { - this.value = value; - } - - @Nullable - public Boolean eval(Object attributeValue) { - try { - if(isValidNumber(attributeValue)) { - return value.doubleValue() == castToValueType(attributeValue, value).doubleValue(); - } - } catch (Exception e) { - } - - return null; - } -} 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 594ea6fc4..1ffbc6fd0 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 @@ -16,20 +16,15 @@ */ package com.optimizely.ab.config.audience.match; -import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; - import javax.annotation.Nullable; class ExistsMatch implements Match { - @SuppressFBWarnings("URF_UNREAD_FIELD") - Object value; - protected ExistsMatch(Object value) { - this.value = value; + protected ExistsMatch() { } @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, 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 8b9e9dd7b..c7f95aa31 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 @@ -20,18 +20,16 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; -class GTMatch extends AttributeMatch { - Number value; +class GTMatch implements Match { - protected GTMatch(Number value) { - this.value = value; + protected GTMatch() { } @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if(isValidNumber(attributeValue)) { - return castToValueType(attributeValue, value).doubleValue() > value.doubleValue(); + if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { + return ((Number) attributeValue).doubleValue() > ((Number) conditionValue).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 index 951adbffb..84cfb49f7 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 @@ -20,18 +20,16 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; -class LTMatch extends AttributeMatch { - Number value; +class LTMatch implements Match { - protected LTMatch(Number value) { - this.value = value; + protected LTMatch() { } @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if(isValidNumber(attributeValue)) { - return castToValueType(attributeValue, value).doubleValue() < value.doubleValue(); + if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { + return ((Number) attributeValue).doubleValue() < ((Number) conditionValue).doubleValue(); } } catch (Exception e) { return null; 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 2f0d3a2a1..ae82bc112 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 @@ -20,5 +20,5 @@ public interface Match { @Nullable - Boolean eval(Object attributeValue); + Boolean eval(Object conditionValue, Object attributeValue); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java new file mode 100644 index 000000000..dcadf6114 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java @@ -0,0 +1,42 @@ +package com.optimizely.ab.config.audience.match; + +import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; + +public class MatchRegistry { + + private static final Map registry = new ConcurrentHashMap<>(); + private static final String EXISTS = "exists"; + private static final String EXACT = "exact"; + private static final String GREATER_THAN = "gt"; + private static final String LESS_THAN = "lt"; + private static final String SUBSTRING = "substring"; + private static final Match LEGACY = new DefaultMatchForLegacyAttributes(); + + static { + registry.put(EXISTS, new ExistsMatch()); + registry.put(EXACT, new ExactMatch()); + registry.put(GREATER_THAN, new GTMatch()); + registry.put(LESS_THAN, new LTMatch()); + registry.put(SUBSTRING, new SubstringMatch()); + } + + // TODO rename Match to Matcher + public static Match getMatch(String name) throws UnknownMatchTypeException { + if (name == null) { + return LEGACY; + } + + Match match = registry.get(name); + if (match == null) { + throw new UnknownMatchTypeException(); + } + + return match; + } + + public static void register(String name, Match match) { + registry.put(name, match); + } + +} 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 deleted file mode 100644 index 7455f1270..000000000 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchType.java +++ /dev/null @@ -1,124 +0,0 @@ -/** - * - * Copyright 2018-2020, 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 org.slf4j.Logger; -import org.slf4j.LoggerFactory; - -import javax.annotation.Nonnull; - -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - -public class MatchType { - - public static final Logger logger = LoggerFactory.getLogger(MatchType.class); - - private String matchType; - private Match matcher; - - public static MatchType getMatchType(String matchType, Object conditionValue) throws UnexpectedValueTypeException, UnknownMatchTypeException { - if (matchType == null) matchType = "legacy_custom_attribute"; - - switch (matchType) { - case "exists": - return new MatchType(matchType, new ExistsMatch(conditionValue)); - case "exact": - if (conditionValue instanceof String) { - return new MatchType(matchType, new ExactMatch((String) conditionValue)); - } else if (isValidNumber(conditionValue)) { - return new MatchType(matchType, new ExactNumberMatch((Number) conditionValue)); - } else if (conditionValue instanceof Boolean) { - return new MatchType(matchType, new ExactMatch((Boolean) conditionValue)); - } - break; - case "substring": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SubstringMatch((String) conditionValue)); - } - break; - case "ge": - if (isValidNumber(conditionValue)) { - return new MatchType(matchType, new GEMatch((Number) conditionValue)); - } - break; - case "gt": - if (isValidNumber(conditionValue)) { - return new MatchType(matchType, new GTMatch((Number) conditionValue)); - } - break; - case "le": - if (isValidNumber(conditionValue)) { - return new MatchType(matchType, new LEMatch((Number) conditionValue)); - } - break; - case "lt": - if (isValidNumber(conditionValue)) { - return new MatchType(matchType, new LTMatch((Number) conditionValue)); - } - break; - case "legacy_custom_attribute": - if (conditionValue instanceof String) { - return new MatchType(matchType, new DefaultMatchForLegacyAttributes((String) conditionValue)); - } - break; - case "semver_eq": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SemanticVersionEqualsMatch((String) conditionValue)); - } - break; - case "semver_ge": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SemanticVersionGEMatch((String) conditionValue)); - } - break; - case "semver_gt": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SemanticVersionGTMatch((String) conditionValue)); - } - break; - case "semver_le": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SemanticVersionLEMatch((String) conditionValue)); - } - break; - case "semver_lt": - if (conditionValue instanceof String) { - return new MatchType(matchType, new SemanticVersionLTMatch((String) conditionValue)); - } - break; - default: - throw new UnknownMatchTypeException(); - } - - throw new UnexpectedValueTypeException(); - } - - private MatchType(String type, Match matcher) { - this.matchType = type; - this.matcher = matcher; - } - - @Nonnull - public Match getMatcher() { - return matcher; - } - - @Override - public String toString() { - return matchType; - } -} 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 946ebad99..a1c10d645 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,11 +18,9 @@ import javax.annotation.Nullable; -class SubstringMatch extends AttributeMatch { - String value; +class SubstringMatch implements Match { - protected SubstringMatch(String value) { - this.value = value; + protected SubstringMatch() { } /** @@ -32,9 +30,17 @@ protected SubstringMatch(String value) { * @return true/false if the user attribute string value contains the condition string value */ @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { + if (!(conditionValue instanceof String)) { + return null; + } + + if (!(attributeValue instanceof String)) { + return null; + } + try { - return castToValueType(attributeValue, value).contains(value); + return attributeValue.toString().contains(conditionValue.toString()); } catch (Exception e) { return 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 645025476..772d22ef7 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 @@ -55,11 +55,11 @@ public class AudienceConditionEvaluationTest { @Before public void initialize() { - testUserAttributes = new HashMap(); + testUserAttributes = new HashMap<>(); testUserAttributes.put("browser_type", "chrome"); testUserAttributes.put("device_type", "Android"); - testTypedUserAttributes = new HashMap(); + testTypedUserAttributes = new HashMap<>(); testTypedUserAttributes.put("is_firefox", true); testTypedUserAttributes.put("num_counts", 3.55); testTypedUserAttributes.put("num_size", 3); From 5a76aaee7d4006b985a274bc5608b65f2fdc3373 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Thu, 6 Aug 2020 18:23:39 -0700 Subject: [PATCH 02/15] Fix remaining tests. --- .../com/optimizely/ab/config/audience/UserAttribute.java | 2 +- .../audience/match/DefaultMatchForLegacyAttributes.java | 5 ++++- .../com/optimizely/ab/config/audience/match/ExactMatch.java | 4 ++-- .../java/com/optimizely/ab/config/audience/match/Match.java | 2 +- .../optimizely/ab/config/audience/match/SubstringMatch.java | 4 ++-- 5 files changed, 10 insertions(+), 7 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 e2e4a0e66..474b151d6 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 @@ -108,7 +108,7 @@ public Boolean evaluate(ProjectConfig config, Map attributes) { } } return result; - } catch (UnknownMatchTypeException e) { + } catch (UnknownMatchTypeException | UnexpectedValueTypeException e) { logger.warn("Audience condition \"{}\" " + e.getMessage(), this); } catch (NullPointerException e) { 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 9ad7f7a19..e68f260c3 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 @@ -28,7 +28,10 @@ protected DefaultMatchForLegacyAttributes() { } @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + if (!(conditionValue instanceof String)) { + throw new UnexpectedValueTypeException(); + } if (attributeValue == null) { return false; } 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 ed12bfa90..be451b800 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 @@ -26,7 +26,7 @@ protected ExactMatch() { } @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (isValidNumber(attributeValue)) { if (isValidNumber(conditionValue)) { return evalNumber((Number) conditionValue, (Number) attributeValue); @@ -39,7 +39,7 @@ public Boolean eval(Object conditionValue, Object attributeValue) { } if (!(conditionValue instanceof String || conditionValue instanceof Boolean)) { - return null; + throw new UnexpectedValueTypeException(); } if (attributeValue.getClass() != conditionValue.getClass()) { 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 ae82bc112..cfc1e048f 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 @@ -20,5 +20,5 @@ public interface Match { @Nullable - Boolean eval(Object conditionValue, Object attributeValue); + Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException; } 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 a1c10d645..7a4427993 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,9 +30,9 @@ protected SubstringMatch() { * @return true/false if the user attribute string value contains the condition string value */ @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (!(conditionValue instanceof String)) { - return null; + throw new UnexpectedValueTypeException(); } if (!(attributeValue instanceof String)) { From 1ad8d43f8a6b315221bb897d1e80b58064133a3e Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Fri, 7 Aug 2020 11:58:55 -0700 Subject: [PATCH 03/15] Update doc string. --- .../config/audience/match/MatchRegistry.java | 58 ++++++++++++++----- 1 file changed, 42 insertions(+), 16 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java index dcadf6114..60bf9223f 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java @@ -1,33 +1,52 @@ +/** + * + * Copyright 2020, 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 java.util.Map; import java.util.concurrent.ConcurrentHashMap; +/** + * MatchRegistry maps a string match "type" to a match implementation. + * All supported Match implementations must be registed with this registry. + * Third-party {@link Match} implementations may also be registered to provide + * additional functionality. + */ public class MatchRegistry { private static final Map registry = new ConcurrentHashMap<>(); - private static final String EXISTS = "exists"; - private static final String EXACT = "exact"; - private static final String GREATER_THAN = "gt"; - private static final String LESS_THAN = "lt"; - private static final String SUBSTRING = "substring"; - private static final Match LEGACY = new DefaultMatchForLegacyAttributes(); + public static final String EXISTS = "exists"; + public static final String EXACT = "exact"; + public static final String GREATER_THAN = "gt"; + public static final String LESS_THAN = "lt"; + public static final String SUBSTRING = "substring"; + public static final String LEGACY = "legacy"; static { - registry.put(EXISTS, new ExistsMatch()); - registry.put(EXACT, new ExactMatch()); - registry.put(GREATER_THAN, new GTMatch()); - registry.put(LESS_THAN, new LTMatch()); - registry.put(SUBSTRING, new SubstringMatch()); + register(EXISTS, new ExistsMatch()); + register(EXACT, new ExactMatch()); + register(GREATER_THAN, new GTMatch()); + register(LESS_THAN, new LTMatch()); + register(SUBSTRING, new SubstringMatch()); + register(LEGACY, new DefaultMatchForLegacyAttributes()); } // TODO rename Match to Matcher public static Match getMatch(String name) throws UnknownMatchTypeException { - if (name == null) { - return LEGACY; - } - - Match match = registry.get(name); + Match match = registry.get(name == null ? LEGACY : name); if (match == null) { throw new UnknownMatchTypeException(); } @@ -35,6 +54,13 @@ public static Match getMatch(String name) throws UnknownMatchTypeException { return match; } + /** + * register registers a Match implementation with it's name. + * NOTE: This does not check for existence so default implementations can + * be overridden. + * @param name + * @param match + */ public static void register(String name, Match match) { registry.put(name, match); } From fc73e8e9ca1b67cac96a83f5e68123681a8b6740 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Fri, 4 Sep 2020 12:29:30 -0700 Subject: [PATCH 04/15] Refactor semver. --- .../ab/config/audience/match/GEMatch.java | 13 ++++--------- .../ab/config/audience/match/LEMatch.java | 14 +++++--------- .../ab/config/audience/match/MatchRegistry.java | 15 +++++++++++++++ .../ab/config/audience/match/SemanticVersion.java | 10 ++++++++++ .../match/SemanticVersionEqualsMatch.java | 15 ++------------- .../audience/match/SemanticVersionGEMatch.java | 15 ++------------- .../audience/match/SemanticVersionGTMatch.java | 15 ++------------- .../audience/match/SemanticVersionLEMatch.java | 15 ++------------- .../audience/match/SemanticVersionLTMatch.java | 15 ++------------- 9 files changed, 44 insertions(+), 83 deletions(-) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java index 8724cfcb0..0681b264b 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java @@ -20,18 +20,13 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; -class GEMatch extends AttributeMatch { - Number value; - - protected GEMatch(Number value) { - this.value = value; - } +class GEMatch implements Match { @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if(isValidNumber(attributeValue)) { - return castToValueType(attributeValue, value).doubleValue() >= value.doubleValue(); + if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { + return ((Number) attributeValue).doubleValue() >= ((Number) conditionValue).doubleValue(); } } catch (Exception e) { return null; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java index 23d1c03fc..f6c75635c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java @@ -20,23 +20,19 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; -class LEMatch extends AttributeMatch { - Number value; - - protected LEMatch(Number value) { - this.value = value; - } +class LEMatch implements Match { @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if(isValidNumber(attributeValue)) { - return castToValueType(attributeValue, value).doubleValue() <= value.doubleValue(); + if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { + return ((Number) attributeValue).doubleValue() <= ((Number) conditionValue).doubleValue(); } } catch (Exception e) { return null; } return null; + } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java index 60bf9223f..bb0458280 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java @@ -32,7 +32,15 @@ public class MatchRegistry { public static final String EXACT = "exact"; public static final String GREATER_THAN = "gt"; public static final String LESS_THAN = "lt"; + public static final String LESS_THAN_EQ = "le"; + public static final String GREATER_THAN_EQ = "ge"; public static final String SUBSTRING = "substring"; + public static final String SEMVER_EQ = "semver_eq"; + public static final String SEMVER_GE = "semver_ge"; + public static final String SEMVER_GT = "semver_gt"; + public static final String SEMVER_LE = "semver_le"; + public static final String SEMVER_LT = "semver_lt"; + public static final String LEGACY = "legacy"; static { @@ -40,7 +48,14 @@ public class MatchRegistry { register(EXACT, new ExactMatch()); register(GREATER_THAN, new GTMatch()); register(LESS_THAN, new LTMatch()); + register(LESS_THAN_EQ, new LEMatch()); + register(GREATER_THAN_EQ, new GEMatch()); register(SUBSTRING, new SubstringMatch()); + register(SEMVER_EQ, new SemanticVersionEqualsMatch()); + register(SEMVER_GE, new SemanticVersionGEMatch()); + register(SEMVER_GT, new SemanticVersionGTMatch()); + register(SEMVER_LE, new SemanticVersionLEMatch()); + register(SEMVER_LT, new SemanticVersionLTMatch()); register(LEGACY, new DefaultMatchForLegacyAttributes()); } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java index 9e37ac02b..afb7ca497 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java @@ -34,6 +34,16 @@ public SemanticVersion(String version) { this.version = version; } + public static int compare(Object o1, Object o2) throws Exception { + if (o1 instanceof String && o2 instanceof String) { + SemanticVersion v1 = new SemanticVersion((String) o1); + SemanticVersion v2 = new SemanticVersion((String) o2); + return v1.compare(v2); + } + + throw new UnexpectedValueTypeException(); + } + public int compare(SemanticVersion targetedVersion) throws Exception { if (targetedVersion == null || stringIsNullOrEmpty(targetedVersion.version)) { diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java index b727d88cf..f79f4d8a4 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java @@ -19,23 +19,12 @@ import javax.annotation.Nullable; class SemanticVersionEqualsMatch implements Match { - String value; - - protected SemanticVersionEqualsMatch(String value) { - this.value = value; - } - @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if (this.value != null && attributeValue instanceof String) { - SemanticVersion conditionalVersion = new SemanticVersion(value); - SemanticVersion userSemanticVersion = new SemanticVersion((String) attributeValue); - return userSemanticVersion.compare(conditionalVersion) == 0; - } + return SemanticVersion.compare(attributeValue, conditionValue) == 0; } catch (Exception e) { return null; } - return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java index fd31e1ab2..3737fefb9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java @@ -19,23 +19,12 @@ import javax.annotation.Nullable; class SemanticVersionGEMatch implements Match { - String value; - - protected SemanticVersionGEMatch(String value) { - this.value = value; - } - @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if (this.value != null && attributeValue instanceof String) { - SemanticVersion conditionalVersion = new SemanticVersion(value); - SemanticVersion userSemanticVersion = new SemanticVersion((String) attributeValue); - return userSemanticVersion.compare(conditionalVersion) >= 0; - } + return SemanticVersion.compare(attributeValue, conditionValue) >= 0; } catch (Exception e) { return null; } - return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java index 7ca0f31b1..0dc823869 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java @@ -19,23 +19,12 @@ import javax.annotation.Nullable; class SemanticVersionGTMatch implements Match { - String value; - - protected SemanticVersionGTMatch(String target) { - this.value = target; - } - @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if (this.value != null && attributeValue instanceof String) { - SemanticVersion conditionalVersion = new SemanticVersion(value); - SemanticVersion userSemanticVersion = new SemanticVersion((String) attributeValue); - return userSemanticVersion.compare(conditionalVersion) > 0; - } + return SemanticVersion.compare(attributeValue, conditionValue) > 0; } catch (Exception e) { return null; } - return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java index 6c7629672..ca0c08733 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java @@ -19,23 +19,12 @@ import javax.annotation.Nullable; class SemanticVersionLEMatch implements Match { - String value; - - protected SemanticVersionLEMatch(String target) { - this.value = target; - } - @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if (this.value != null && attributeValue instanceof String) { - SemanticVersion conditionalVersion = new SemanticVersion(value); - SemanticVersion userSemanticVersion = new SemanticVersion((String) attributeValue); - return userSemanticVersion.compare(conditionalVersion) <= 0; - } + return SemanticVersion.compare(attributeValue, conditionValue) <= 0; } catch (Exception e) { return null; } - return null; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java index 6f67863a1..f12a6ba89 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java @@ -19,23 +19,12 @@ import javax.annotation.Nullable; class SemanticVersionLTMatch implements Match { - String value; - - protected SemanticVersionLTMatch(String target) { - this.value = target; - } - @Nullable - public Boolean eval(Object attributeValue) { + public Boolean eval(Object conditionValue, Object attributeValue) { try { - if (this.value != null && attributeValue instanceof String) { - SemanticVersion conditionalVersion = new SemanticVersion(value); - SemanticVersion userSemanticVersion = new SemanticVersion((String) attributeValue); - return userSemanticVersion.compare(conditionalVersion) < 0; - } + return SemanticVersion.compare(attributeValue, conditionValue) < 0; } catch (Exception e) { return null; } - return null; } } From 26557fdd0a472156a2deb471160eb6d7d193da17 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 8 Sep 2020 11:06:53 -0700 Subject: [PATCH 05/15] Add UnknownValueTypeExceptioni. --- .../ab/config/audience/UserAttribute.java | 41 ++++++++++--------- .../ab/config/audience/match/ExactMatch.java | 14 +------ .../ab/config/audience/match/GEMatch.java | 16 +++----- .../ab/config/audience/match/GTMatch.java | 15 ++----- .../ab/config/audience/match/LEMatch.java | 17 +++----- .../ab/config/audience/match/LTMatch.java | 14 +------ .../ab/config/audience/match/Match.java | 2 +- .../audience/match/NumberComparator.java | 37 +++++++++++++++++ .../audience/match/SemanticVersion.java | 12 +++++- .../match/SemanticVersionEqualsMatch.java | 8 +--- .../match/SemanticVersionGEMatch.java | 8 +--- .../match/SemanticVersionGTMatch.java | 8 +--- .../match/SemanticVersionLEMatch.java | 8 +--- .../match/SemanticVersionLTMatch.java | 8 +--- .../match/UnknownValueTypeException.java | 26 ++++++++++++ 15 files changed, 121 insertions(+), 113 deletions(-) create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java create mode 100644 core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java 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 474b151d6..277f2f184 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 @@ -86,31 +86,32 @@ public Boolean evaluate(ProjectConfig config, Map attributes) { try { Match matcher = MatchRegistry.getMatch(match); Boolean result = matcher.eval(value, userAttributeValue); - if (result == null) { - if (!attributes.containsKey(name)) { - //Missing attribute value - logger.debug("Audience condition \"{}\" evaluated to UNKNOWN because no value was passed for user attribute \"{}\"", this, name); + throw new UnknownValueTypeException(); + } + + return result; + } catch(UnknownValueTypeException e) { + if (!attributes.containsKey(name)) { + //Missing attribute value + logger.debug("Audience condition \"{}\" evaluated to UNKNOWN because no value was passed for user attribute \"{}\"", this, name); + } else { + //if attribute value is not valid + if (userAttributeValue != null) { + logger.warn( + "Audience condition \"{}\" evaluated to UNKNOWN because a value of type \"{}\" was passed for user attribute \"{}\"", + this, + userAttributeValue.getClass().getCanonicalName(), + name); } else { - //if attribute value is not valid - if (userAttributeValue != null) { - logger.warn( - "Audience condition \"{}\" evaluated to UNKNOWN because a value of type \"{}\" was passed for user attribute \"{}\"", - this, - userAttributeValue.getClass().getCanonicalName(), - name); - } else { - logger.debug( - "Audience condition \"{}\" evaluated to UNKNOWN because a null value was passed for user attribute \"{}\"", - this, - name); - } + logger.debug( + "Audience condition \"{}\" evaluated to UNKNOWN because a null value was passed for user attribute \"{}\"", + this, + name); } } - return result; } catch (UnknownMatchTypeException | UnexpectedValueTypeException e) { - logger.warn("Audience condition \"{}\" " + e.getMessage(), - this); + logger.warn("Audience condition \"{}\" " + e.getMessage(), this); } catch (NullPointerException e) { logger.error("attribute or value null for match {}", match != null ? match : "legacy condition", e); } 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 be451b800..738f6b0a1 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 @@ -29,7 +29,7 @@ protected ExactMatch() { public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (isValidNumber(attributeValue)) { if (isValidNumber(conditionValue)) { - return evalNumber((Number) conditionValue, (Number) attributeValue); + return NumberComparator.compareUnsafe(attributeValue, conditionValue) == 0; } return null; } @@ -48,16 +48,4 @@ public Boolean eval(Object conditionValue, Object attributeValue) throws Unexpec return conditionValue.equals(attributeValue); } - - @Nullable - public Boolean evalNumber(Number conditionValue, Number attributeValue) { - try { - if(isValidNumber(attributeValue)) { - return conditionValue.doubleValue() == attributeValue.doubleValue(); - } - } catch (Exception e) { - } - - return null; - } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java index 0681b264b..ec8a4296c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java @@ -18,19 +18,13 @@ import javax.annotation.Nullable; -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - class GEMatch implements Match { + protected GEMatch() { + } + @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { - return ((Number) attributeValue).doubleValue() >= ((Number) conditionValue).doubleValue(); - } - } catch (Exception e) { - return null; - } - return null; + public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { + return NumberComparator.compare(attributeValue, conditionValue) >= 0; } } 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 c7f95aa31..422f71768 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,22 +18,13 @@ import javax.annotation.Nullable; -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - class GTMatch implements Match { protected GTMatch() { } @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { - return ((Number) attributeValue).doubleValue() > ((Number) conditionValue).doubleValue(); - } - } catch (Exception e) { - return null; - } - return null; + public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { + return NumberComparator.compare(attributeValue, conditionValue) > 0; } -} +} \ No newline at end of file diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java index f6c75635c..caac2872e 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java @@ -18,21 +18,14 @@ import javax.annotation.Nullable; -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - class LEMatch implements Match { - @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { - return ((Number) attributeValue).doubleValue() <= ((Number) conditionValue).doubleValue(); - } - } catch (Exception e) { - return null; - } - return null; + protected LEMatch() { + } + @Nullable + public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { + return NumberComparator.compare(attributeValue, conditionValue) <= 0; } } 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 84cfb49f7..2cab6ddff 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,23 +18,13 @@ import javax.annotation.Nullable; -import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; - class LTMatch implements Match { protected LTMatch() { } @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - if(isValidNumber(attributeValue) && isValidNumber(conditionValue) ) { - return ((Number) attributeValue).doubleValue() < ((Number) conditionValue).doubleValue(); - } - } catch (Exception e) { - return null; - } - return null; + public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { + return NumberComparator.compare(attributeValue, conditionValue) < 0; } } - 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 cfc1e048f..ff3516470 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 @@ -20,5 +20,5 @@ public interface Match { @Nullable - Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException; + Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException, UnknownValueTypeException; } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java new file mode 100644 index 000000000..ba5e08213 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java @@ -0,0 +1,37 @@ +/** + * + * Copyright 2020, 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 static com.optimizely.ab.internal.AttributesUtil.isValidNumber; + +public class NumberComparator { + public static int compare(Object o1, Object o2) throws UnknownValueTypeException { + if (!isValidNumber(o1)) { + throw new UnknownValueTypeException(); + } + + if (!isValidNumber(o2)) { + throw new UnknownValueTypeException(); + } + + return compareUnsafe(o1, o2); + } + + public static int compareUnsafe(Object o1, Object o2) { + return Double.compare(((Number) o1).doubleValue(), ((Number) o2).doubleValue()); + } +} diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java index afb7ca497..972f2b0f6 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java @@ -16,6 +16,9 @@ */ package com.optimizely.ab.config.audience.match; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -25,6 +28,7 @@ public final class SemanticVersion { + private static final Logger logger = LoggerFactory.getLogger(SemanticVersion.class); private static final String BUILD_SEPERATOR = "\\+"; private static final String PRE_RELEASE_SEPERATOR = "-"; @@ -34,11 +38,15 @@ public SemanticVersion(String version) { this.version = version; } - public static int compare(Object o1, Object o2) throws Exception { + public static int compare(Object o1, Object o2) throws UnexpectedValueTypeException { if (o1 instanceof String && o2 instanceof String) { SemanticVersion v1 = new SemanticVersion((String) o1); SemanticVersion v2 = new SemanticVersion((String) o2); - return v1.compare(v2); + try { + return v1.compare(v2); + } catch (Exception e) { + logger.warn("Error comparing semantic versions", e); + } } throw new UnexpectedValueTypeException(); diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java index f79f4d8a4..b66710a31 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java @@ -20,11 +20,7 @@ class SemanticVersionEqualsMatch implements Match { @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - return SemanticVersion.compare(attributeValue, conditionValue) == 0; - } catch (Exception e) { - return null; - } + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + return SemanticVersion.compare(attributeValue, conditionValue) == 0; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java index 3737fefb9..883ed5d87 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java @@ -20,11 +20,7 @@ class SemanticVersionGEMatch implements Match { @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - return SemanticVersion.compare(attributeValue, conditionValue) >= 0; - } catch (Exception e) { - return null; - } + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + return SemanticVersion.compare(attributeValue, conditionValue) >= 0; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java index 0dc823869..777b883f9 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java @@ -20,11 +20,7 @@ class SemanticVersionGTMatch implements Match { @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - return SemanticVersion.compare(attributeValue, conditionValue) > 0; - } catch (Exception e) { - return null; - } + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + return SemanticVersion.compare(attributeValue, conditionValue) > 0; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java index ca0c08733..736222877 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java @@ -20,11 +20,7 @@ class SemanticVersionLEMatch implements Match { @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - return SemanticVersion.compare(attributeValue, conditionValue) <= 0; - } catch (Exception e) { - return null; - } + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + return SemanticVersion.compare(attributeValue, conditionValue) <= 0; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java index f12a6ba89..af71206bc 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java @@ -20,11 +20,7 @@ class SemanticVersionLTMatch implements Match { @Nullable - public Boolean eval(Object conditionValue, Object attributeValue) { - try { - return SemanticVersion.compare(attributeValue, conditionValue) < 0; - } catch (Exception e) { - return null; - } + public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { + return SemanticVersion.compare(attributeValue, conditionValue) < 0; } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java new file mode 100644 index 000000000..3fc93b8d8 --- /dev/null +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java @@ -0,0 +1,26 @@ +/** + * + * Copyright 2020, 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 class UnknownValueTypeException extends Exception { + private static String message = "has an unsupported attribute value."; + + public UnknownValueTypeException() { + super(message); + } +} From df8ea55acbbe145c2d208895469cf3d10a79c1f9 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 13:33:28 -0700 Subject: [PATCH 06/15] Remove null check. --- .../com/optimizely/ab/config/audience/match/ExactMatch.java | 4 ---- 1 file changed, 4 deletions(-) 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 738f6b0a1..150a4f9ef 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 @@ -34,10 +34,6 @@ public Boolean eval(Object conditionValue, Object attributeValue) throws Unexpec return null; } - if (conditionValue == null) { - return attributeValue == null; - } - if (!(conditionValue instanceof String || conditionValue instanceof Boolean)) { throw new UnexpectedValueTypeException(); } From 0d3eea60f2c4faa7a8245acbe350e5410c0e727e Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 14:40:24 -0700 Subject: [PATCH 07/15] Add and refactor tests. --- .../config/audience/SemanticVersionTest.java | 116 +++--------------- .../audience/match/NumberComparatorTest.java | 75 +++++++++++ 2 files changed, 94 insertions(+), 97 deletions(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/config/audience/match/NumberComparatorTest.java diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java index 6d4605e54..11761618c 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java @@ -17,6 +17,7 @@ package com.optimizely.ab.config.audience; import com.optimizely.ab.config.audience.match.SemanticVersion; +import com.optimizely.ab.config.audience.match.UnexpectedValueTypeException; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; @@ -141,108 +142,29 @@ public void semanticVersionInvalidShouldBeOfSizeLessThan3() throws Exception { } @Test - public void semanticVersionCompareTo() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1"); - SemanticVersion actualSV = new SemanticVersion("3.7.1"); - assertTrue(actualSV.compare(targetSV) == 0); + public void testEquals() throws Exception { + assertEquals(0, SemanticVersion.compare("3.7.1", "3.7.1")); + assertEquals(0, SemanticVersion.compare("3.7.1", "3.7")); + assertEquals(0, SemanticVersion.compare("2.1.3+build", "2.1.3")); + assertEquals(0, SemanticVersion.compare("3.7.1-beta.1+2.3", "3.7.1-beta.1+2.3")); } @Test - public void semanticVersionCompareToActualLess() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1"); - SemanticVersion actualSV = new SemanticVersion("3.7.0"); - assertTrue(actualSV.compare(targetSV) < 0); + public void testLessThan() throws Exception { + assertTrue(SemanticVersion.compare("3.7.0", "3.7.1") < 0); + assertTrue(SemanticVersion.compare("3.7", "3.7.1") < 0); + assertTrue(SemanticVersion.compare("2.1.3-beta+1", "2.1.3-beta+1.2.3") < 0); + assertTrue(SemanticVersion.compare("2.1.3-beta-1", "2.1.3-beta-1.2.3") < 0); } @Test - public void semanticVersionCompareToActualGreater() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1"); - SemanticVersion actualSV = new SemanticVersion("3.7.2"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - @Test - public void semanticVersionCompareToPatchMissing() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7"); - SemanticVersion actualSV = new SemanticVersion("3.7.1"); - assertTrue(actualSV.compare(targetSV) == 0); - } - - @Test - public void semanticVersionCompareToActualPatchMissing() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1"); - SemanticVersion actualSV = new SemanticVersion("3.7"); - assertTrue(actualSV.compare(targetSV) < 0); - } - - @Test - public void semanticVersionCompareToActualPreReleaseMissing() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-beta"); - SemanticVersion actualSV = new SemanticVersion("3.7.1"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - @Test - public void semanticVersionCompareTargetBetaComplex() throws Exception { - SemanticVersion targetSV = new SemanticVersion("2.1.3-beta+1"); - SemanticVersion actualSV = new SemanticVersion("2.1.3-beta+1.2.3"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - @Test - public void semanticVersionCompareTargetBuildIgnores() throws Exception { - SemanticVersion targetSV = new SemanticVersion("2.1.3"); - SemanticVersion actualSV = new SemanticVersion("2.1.3+build"); - assertTrue(actualSV.compare(targetSV) == 0); - } - - @Test - public void semanticVersionCompareTargetBuildComplex() throws Exception { - SemanticVersion targetSV = new SemanticVersion("2.1.3-beta+1.2.3"); - SemanticVersion actualSV = new SemanticVersion("2.1.3-beta+1"); - assertTrue(actualSV.compare(targetSV) < 0); - } - - @Test - public void semanticVersionCompareMultipleDash() throws Exception { - SemanticVersion targetSV = new SemanticVersion("2.1.3-beta-1.2.3"); - SemanticVersion actualSV = new SemanticVersion("2.1.3-beta-1"); - assertTrue(actualSV.compare(targetSV) < 0); - } - - @Test - public void semanticVersionCompareToAlphaBetaAsciiComparision() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-alpha"); - SemanticVersion actualSV = new SemanticVersion("3.7.1-beta"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - @Test - public void semanticVersionComparePrereleaseSmallerThanBuild() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-prerelease"); - SemanticVersion actualSV = new SemanticVersion("3.7.1+build"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - - @Test - public void semanticVersionCompareAgainstPreReleaseToPreRelease() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-prerelease+build"); - SemanticVersion actualSV = new SemanticVersion("3.7.1-prerelease-prerelease+rc"); - assertTrue(actualSV.compare(targetSV) > 0); - } - - @Test - public void semanticVersionCompareToIgnoreMetaComparision() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-beta.1+2.3"); - SemanticVersion actualSV = new SemanticVersion("3.7.1-beta.1+2.3"); - assertTrue(actualSV.compare(targetSV) == 0); - } - - @Test - public void semanticVersionCompareToPreReleaseComparision() throws Exception { - SemanticVersion targetSV = new SemanticVersion("3.7.1-beta.1"); - SemanticVersion actualSV = new SemanticVersion("3.7.1-beta.2"); - assertTrue(actualSV.compare(targetSV) > 0); + public void testGreaterThan() throws Exception { + assertTrue(SemanticVersion.compare("3.7.2", "3.7.1") > 0); + assertTrue(SemanticVersion.compare("3.7.1", "3.7.1-beta") > 0); + assertTrue(SemanticVersion.compare("2.1.3-beta+1.2.3", "2.1.3-beta+1") > 0); + assertTrue(SemanticVersion.compare("3.7.1-beta", "3.7.1-alpha") > 0); + assertTrue(SemanticVersion.compare("3.7.1+build", "3.7.1-prerelease") > 0); + assertTrue(SemanticVersion.compare("3.7.1-prerelease-prerelease+rc", "3.7.1-prerelease+build") > 0); + assertTrue(SemanticVersion.compare("3.7.1-beta.2", "3.7.1-beta.1") > 0); } } diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/NumberComparatorTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/NumberComparatorTest.java new file mode 100644 index 000000000..19d67dd33 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/NumberComparatorTest.java @@ -0,0 +1,75 @@ +/** + * + * Copyright 2020, 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 org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.*; + +public class NumberComparatorTest { + + private static final List INVALIDS = Collections.unmodifiableList(Arrays.asList(null, "test", "", true)); + + @Test + public void testLessThan() throws UnknownValueTypeException { + assertTrue(NumberComparator.compare(0,1) < 0); + assertTrue(NumberComparator.compare(0,1.0) < 0); + assertTrue(NumberComparator.compare(0,1L) < 0); + } + + @Test + public void testGreaterThan() throws UnknownValueTypeException { + assertTrue(NumberComparator.compare(1,0) > 0); + assertTrue(NumberComparator.compare(1.0,0) > 0); + assertTrue(NumberComparator.compare(1L,0) > 0); + } + + @Test + public void testEquals() throws UnknownValueTypeException { + assertEquals(0, NumberComparator.compare(1, 1)); + assertEquals(0, NumberComparator.compare(1, 1.0)); + assertEquals(0, NumberComparator.compare(1L, 1)); + } + + @Test + public void testInvalidRight() { + for (Object invalid: INVALIDS) { + try { + NumberComparator.compare(0, invalid); + fail("should have failed for invalid object"); + } catch (UnknownValueTypeException e) { + // pass + } + } + } + + @Test + public void testInvalidLeft() { + for (Object invalid: INVALIDS) { + try { + NumberComparator.compare(invalid, 0); + fail("should have failed for invalid object"); + } catch (UnknownValueTypeException e) { + // pass + } + } + } +} From d51cc8be87f430635824eb5e318126701bb24f30 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 15:04:42 -0700 Subject: [PATCH 08/15] Move SemanticVersionTest.java. --- .../ab/config/audience/{ => match}/SemanticVersionTest.java | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) rename core-api/src/test/java/com/optimizely/ab/config/audience/{ => match}/SemanticVersionTest.java (97%) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/SemanticVersionTest.java similarity index 97% rename from core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java rename to core-api/src/test/java/com/optimizely/ab/config/audience/match/SemanticVersionTest.java index 11761618c..1b819d418 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/SemanticVersionTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/SemanticVersionTest.java @@ -14,10 +14,8 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -package com.optimizely.ab.config.audience; +package com.optimizely.ab.config.audience.match; -import com.optimizely.ab.config.audience.match.SemanticVersion; -import com.optimizely.ab.config.audience.match.UnexpectedValueTypeException; import org.junit.Rule; import org.junit.Test; import org.junit.rules.ExpectedException; From 49fabdae8e1c452519257a4c867b8d3fab227391 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 16:05:23 -0700 Subject: [PATCH 09/15] Add missing unit tests. --- .../ab/config/audience/match/ExactMatch.java | 2 +- .../config/audience/match/ExactMatchTest.java | 84 +++++++++++++++++++ .../audience/match/SubstringMatchTest.java | 65 ++++++++++++++ 3 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 core-api/src/test/java/com/optimizely/ab/config/audience/match/ExactMatchTest.java create mode 100644 core-api/src/test/java/com/optimizely/ab/config/audience/match/SubstringMatchTest.java 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 150a4f9ef..119cbaa4b 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 @@ -38,7 +38,7 @@ public Boolean eval(Object conditionValue, Object attributeValue) throws Unexpec throw new UnexpectedValueTypeException(); } - if (attributeValue.getClass() != conditionValue.getClass()) { + if (attributeValue == null || attributeValue.getClass() != conditionValue.getClass()) { return null; } diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/ExactMatchTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/ExactMatchTest.java new file mode 100644 index 000000000..5f2d1d62e --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/ExactMatchTest.java @@ -0,0 +1,84 @@ +/** + * + * Copyright 2020, 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 org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.*; + +public class ExactMatchTest { + + private ExactMatch match; + private static final List INVALIDS = Collections.unmodifiableList(Arrays.asList(new byte[0], new Object(), null)); + + @Before + public void setUp() { + match = new ExactMatch(); + } + + @Test + public void testInvalidConditionValues() { + for (Object invalid : INVALIDS) { + try { + match.eval(invalid, "valid"); + fail("should have raised exception"); + } catch (UnexpectedValueTypeException e) { + //pass + } + } + } + + @Test + public void testMismatchClasses() throws Exception { + assertNull(match.eval(false, "false")); + assertNull(match.eval("false", null)); + } + + @Test + public void testStringMatch() throws Exception { + assertEquals(Boolean.TRUE, match.eval("", "")); + assertEquals(Boolean.TRUE, match.eval("true", "true")); + assertEquals(Boolean.FALSE, match.eval("true", "false")); + } + + @Test + public void testBooleanMatch() throws Exception { + assertEquals(Boolean.TRUE, match.eval(true, true)); + assertEquals(Boolean.TRUE, match.eval(false, false)); + assertEquals(Boolean.FALSE, match.eval(true, false)); + } + + @Test + public void testNumberMatch() throws UnexpectedValueTypeException { + assertEquals(Boolean.TRUE, match.eval(1, 1)); + assertEquals(Boolean.TRUE, match.eval(1L, 1L)); + assertEquals(Boolean.TRUE, match.eval(1.0, 1.0)); + assertEquals(Boolean.TRUE, match.eval(1, 1.0)); + assertEquals(Boolean.TRUE, match.eval(1L, 1.0)); + + assertEquals(Boolean.FALSE, match.eval(1, 2)); + assertEquals(Boolean.FALSE, match.eval(1L, 2L)); + assertEquals(Boolean.FALSE, match.eval(1.0, 2.0)); + assertEquals(Boolean.FALSE, match.eval(1, 1.1)); + assertEquals(Boolean.FALSE, match.eval(1L, 1.1)); + } +} diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/SubstringMatchTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/SubstringMatchTest.java new file mode 100644 index 000000000..0d417eefe --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/SubstringMatchTest.java @@ -0,0 +1,65 @@ +/** + * + * Copyright 2020, 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 org.junit.Before; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Collections; +import java.util.List; + +import static org.junit.Assert.*; + +public class SubstringMatchTest { + + private SubstringMatch match; + private static final List INVALIDS = Collections.unmodifiableList(Arrays.asList(new byte[0], new Object(), null)); + + @Before + public void setUp() { + match = new SubstringMatch(); + } + + @Test + public void testInvalidConditionValues() { + for (Object invalid : INVALIDS) { + try { + match.eval(invalid, "valid"); + fail("should have raised exception"); + } catch (UnexpectedValueTypeException e) { + //pass + } + } + } + + @Test + public void testInvalidAttributesValues() throws UnexpectedValueTypeException { + for (Object invalid : INVALIDS) { + assertNull(match.eval("valid", invalid)); + } + } + + @Test + public void testStringMatch() throws Exception { + assertEquals(Boolean.TRUE, match.eval("", "any")); + assertEquals(Boolean.TRUE, match.eval("same", "same")); + assertEquals(Boolean.TRUE, match.eval("a", "ab")); + assertEquals(Boolean.FALSE, match.eval("ab", "a")); + assertEquals(Boolean.FALSE, match.eval("a", "b")); + } +} From 74c13e9f775ae06d3b94459909045fa554394d0c Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 17:03:12 -0700 Subject: [PATCH 10/15] Add unit test for matcher registry. --- .../audience/match/MatchRegistryTest.java | 44 +++++++++++++++++++ 1 file changed, 44 insertions(+) create mode 100644 core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java new file mode 100644 index 000000000..4bef4b3c9 --- /dev/null +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java @@ -0,0 +1,44 @@ +package com.optimizely.ab.config.audience.match; + +import org.junit.Test; + +import static com.optimizely.ab.config.audience.match.MatchRegistry.*; +import static org.hamcrest.Matchers.instanceOf; +import static org.junit.Assert.*; + +public class MatchRegistryTest { + + @Test + public void testDefaultMatchers() throws UnknownMatchTypeException { + assertThat(MatchRegistry.getMatch(EXISTS), instanceOf(ExistsMatch.class)); + assertThat(MatchRegistry.getMatch(EXACT), instanceOf(ExactMatch.class)); + assertThat(MatchRegistry.getMatch(GREATER_THAN), instanceOf(GTMatch.class)); + assertThat(MatchRegistry.getMatch(LESS_THAN), instanceOf(LTMatch.class)); + assertThat(MatchRegistry.getMatch(LESS_THAN_EQ), instanceOf(LEMatch.class)); + assertThat(MatchRegistry.getMatch(GREATER_THAN_EQ), instanceOf(GEMatch.class)); + assertThat(MatchRegistry.getMatch(SUBSTRING), instanceOf(SubstringMatch.class)); + assertThat(MatchRegistry.getMatch(SEMVER_EQ), instanceOf(SemanticVersionEqualsMatch.class)); + assertThat(MatchRegistry.getMatch(SEMVER_GE), instanceOf(SemanticVersionGEMatch.class)); + assertThat(MatchRegistry.getMatch(SEMVER_GT), instanceOf(SemanticVersionGTMatch.class)); + assertThat(MatchRegistry.getMatch(SEMVER_LE), instanceOf(SemanticVersionLEMatch.class)); + assertThat(MatchRegistry.getMatch(SEMVER_LT), instanceOf(SemanticVersionLTMatch.class)); + } + + @Test(expected = UnknownMatchTypeException.class) + public void testUnknownMatcher() throws UnknownMatchTypeException { + MatchRegistry.getMatch("UNKNOWN"); + } + + @Test + public void testRegister() throws UnknownMatchTypeException { + class TestMatcher implements Match { + @Override + public Boolean eval(Object conditionValue, Object attributeValue) { + return null; + } + } + + MatchRegistry.register("test-matcher", new TestMatcher()); + assertThat(MatchRegistry.getMatch("test-matcher"), instanceOf(TestMatcher.class)); + } +} From fd41198ca09d2a64d45eee05ce4fad574805630f Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Wed, 9 Sep 2020 17:04:03 -0700 Subject: [PATCH 11/15] Fix linter. --- .../config/audience/match/MatchRegistryTest.java | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java index 4bef4b3c9..a41f2d17a 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java @@ -1,3 +1,19 @@ +/** + * + * Copyright 2020, 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 org.junit.Test; From f3ec4626caf75fd970669aba7c49f21faac36b4f Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 15 Sep 2020 11:29:40 -0700 Subject: [PATCH 12/15] Restore protected constructors. --- .../ab/config/audience/match/SemanticVersionEqualsMatch.java | 4 ++++ .../ab/config/audience/match/SemanticVersionGEMatch.java | 4 ++++ .../ab/config/audience/match/SemanticVersionGTMatch.java | 4 ++++ .../ab/config/audience/match/SemanticVersionLEMatch.java | 4 ++++ .../ab/config/audience/match/SemanticVersionLTMatch.java | 4 ++++ 5 files changed, 20 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java index b66710a31..059e1ac0d 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java @@ -19,6 +19,10 @@ import javax.annotation.Nullable; class SemanticVersionEqualsMatch implements Match { + + protected SemanticVersionEqualsMatch() { + } + @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) == 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java index 883ed5d87..205e32dd0 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java @@ -19,6 +19,10 @@ import javax.annotation.Nullable; class SemanticVersionGEMatch implements Match { + + protected SemanticVersionGEMatch() { + } + @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) >= 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java index 777b883f9..5dfe2d10c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java @@ -19,6 +19,10 @@ import javax.annotation.Nullable; class SemanticVersionGTMatch implements Match { + + protected SemanticVersionGTMatch() { + } + @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) > 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java index 736222877..41363230c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java @@ -19,6 +19,10 @@ import javax.annotation.Nullable; class SemanticVersionLEMatch implements Match { + + protected SemanticVersionLEMatch() { + } + @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) <= 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java index af71206bc..c22fa5a6d 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java @@ -19,6 +19,10 @@ import javax.annotation.Nullable; class SemanticVersionLTMatch implements Match { + + protected SemanticVersionLTMatch() { + } + @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) < 0; From 95fc33cb31ef5e0f4fe2986ee13a1fe80abf5664 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 15 Sep 2020 11:54:13 -0700 Subject: [PATCH 13/15] Remove constructors add doc strings. --- .../match/DefaultMatchForLegacyAttributes.java | 4 ---- .../ab/config/audience/match/ExactMatch.java | 9 +++++---- .../ab/config/audience/match/ExistsMatch.java | 7 +++---- .../ab/config/audience/match/GEMatch.java | 7 +++---- .../ab/config/audience/match/GTMatch.java | 7 +++---- .../ab/config/audience/match/LEMatch.java | 7 +++---- .../ab/config/audience/match/LTMatch.java | 7 +++---- .../ab/config/audience/match/NumberComparator.java | 10 +++++++++- .../ab/config/audience/match/SemanticVersion.java | 7 +++++++ .../audience/match/SemanticVersionEqualsMatch.java | 7 +++---- .../audience/match/SemanticVersionGEMatch.java | 8 ++++---- .../audience/match/SemanticVersionGTMatch.java | 7 +++---- .../audience/match/SemanticVersionLEMatch.java | 8 ++++---- .../audience/match/SemanticVersionLTMatch.java | 7 +++---- .../ab/config/audience/match/SubstringMatch.java | 14 ++++---------- 15 files changed, 57 insertions(+), 59 deletions(-) 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 e68f260c3..5375fd264 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 @@ -23,10 +23,6 @@ * legacy custom attributes. This will be dropped for ExactMatch and the unit tests need to be fixed. */ class DefaultMatchForLegacyAttributes implements Match { - - protected DefaultMatchForLegacyAttributes() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (!(conditionValue instanceof String)) { 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 119cbaa4b..f5a869f67 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 @@ -20,11 +20,12 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; +/** + * ExactMatch supports matching Numbers, Strings and Booleans. Numbers are first converted to doubles + * before the comparison is evaluated. See {@link NumberComparator} Strings and Booleans are evaulated + * via the Object equals method. + */ class ExactMatch implements Match { - - protected ExactMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (isValidNumber(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 1ffbc6fd0..95bc5c1cb 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 @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * ExistsMatch checks that the attribute value is NOT null. + */ class ExistsMatch implements Match { - - protected ExistsMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) { return attributeValue != null; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java index ec8a4296c..e66012cba 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/GEMatch.java @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * GEMatch performs a "greater than or equal to" number comparison via {@link NumberComparator}. + */ class GEMatch implements Match { - - protected GEMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { return NumberComparator.compare(attributeValue, conditionValue) >= 0; 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 422f71768..3824a007d 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,11 +18,10 @@ import javax.annotation.Nullable; +/** + * GTMatch performs a "greater than" number comparison via {@link NumberComparator}. + */ class GTMatch implements Match { - - protected GTMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { return NumberComparator.compare(attributeValue, conditionValue) > 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java index caac2872e..b222fa022 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/LEMatch.java @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * GEMatch performs a "less than or equal to" number comparison via {@link NumberComparator}. + */ class LEMatch implements Match { - - protected LEMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { return NumberComparator.compare(attributeValue, conditionValue) <= 0; 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 2cab6ddff..0b0ecea13 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,11 +18,10 @@ import javax.annotation.Nullable; +/** + * GTMatch performs a "less than" number comparison via {@link NumberComparator}. + */ class LTMatch implements Match { - - protected LTMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { return NumberComparator.compare(attributeValue, conditionValue) < 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java index ba5e08213..c7996b1e7 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java @@ -18,6 +18,10 @@ import static com.optimizely.ab.internal.AttributesUtil.isValidNumber; +/** + * NumberComparator performs a numeric comparison. The input values are assumed to be numbers else + * compare will throw an {@link UnknownValueTypeException}. + */ public class NumberComparator { public static int compare(Object o1, Object o2) throws UnknownValueTypeException { if (!isValidNumber(o1)) { @@ -31,7 +35,11 @@ public static int compare(Object o1, Object o2) throws UnknownValueTypeException return compareUnsafe(o1, o2); } - public static int compareUnsafe(Object o1, Object o2) { + /** + * compareUnsafe is provided to avoid checking the input values are numbers. It's assumed that the inputs + * are known to be Numbers. + */ + static int compareUnsafe(Object o1, Object o2) { return Double.compare(((Number) o1).doubleValue(), ((Number) o2).doubleValue()); } } diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java index 972f2b0f6..d963e7702 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersion.java @@ -26,6 +26,9 @@ import static com.optimizely.ab.internal.AttributesUtil.parseNumeric; import static com.optimizely.ab.internal.AttributesUtil.stringIsNullOrEmpty; +/** + * SemanticVersion implements the specification for the purpose of comparing two Versions. + */ public final class SemanticVersion { private static final Logger logger = LoggerFactory.getLogger(SemanticVersion.class); @@ -38,6 +41,10 @@ public SemanticVersion(String version) { this.version = version; } + /** + * compare takes object inputs and coerces them into SemanticVersion objects before performing the comparison. + * If the input values cannot be coerced then an {@link UnexpectedValueTypeException} is thrown. + */ public static int compare(Object o1, Object o2) throws UnexpectedValueTypeException { if (o1 instanceof String && o2 instanceof String) { SemanticVersion v1 = new SemanticVersion((String) o1); diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java index 059e1ac0d..ac0c8310b 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionEqualsMatch.java @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * SemanticVersionEqualsMatch performs a equality comparison via {@link SemanticVersion#compare(Object, Object)}. + */ class SemanticVersionEqualsMatch implements Match { - - protected SemanticVersionEqualsMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) == 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java index 205e32dd0..91f95d4cd 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGEMatch.java @@ -18,11 +18,11 @@ import javax.annotation.Nullable; +/** + * SemanticVersionGEMatch performs a "greater than or equal to" comparison + * via {@link SemanticVersion#compare(Object, Object)}. + */ class SemanticVersionGEMatch implements Match { - - protected SemanticVersionGEMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) >= 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java index 5dfe2d10c..52513024c 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionGTMatch.java @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * SemanticVersionGTMatch performs a "greater than" comparison via {@link SemanticVersion#compare(Object, Object)}. + */ class SemanticVersionGTMatch implements Match { - - protected SemanticVersionGTMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) > 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java index 41363230c..4297d4545 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLEMatch.java @@ -18,11 +18,11 @@ import javax.annotation.Nullable; +/** + * SemanticVersionLEMatch performs a "less than or equal to" comparison + * via {@link SemanticVersion#compare(Object, Object)}. + */ class SemanticVersionLEMatch implements Match { - - protected SemanticVersionLEMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) <= 0; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java index c22fa5a6d..a35dcd2da 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/SemanticVersionLTMatch.java @@ -18,11 +18,10 @@ import javax.annotation.Nullable; +/** + * SemanticVersionLTMatch performs a "less than" comparison via {@link SemanticVersion#compare(Object, Object)}. + */ class SemanticVersionLTMatch implements Match { - - protected SemanticVersionLTMatch() { - } - @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { return SemanticVersion.compare(attributeValue, conditionValue) < 0; 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 7a4427993..5565040dc 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,17 +18,11 @@ import javax.annotation.Nullable; +/** + * SubstringMatch checks if the attribute value contains the condition value. + * This assumes both the condition and attribute values are provided as Strings. + */ class SubstringMatch implements Match { - - protected SubstringMatch() { - } - - /** - * This matches the same substring matching logic in the Web client. - * - * @param attributeValue - * @return true/false if the user attribute string value contains the condition string value - */ @Nullable public Boolean eval(Object conditionValue, Object attributeValue) throws UnexpectedValueTypeException { if (!(conditionValue instanceof String)) { From b656b0c2f7212b5440f6975ee035a03bf420f363 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 15 Sep 2020 13:11:23 -0700 Subject: [PATCH 14/15] Address PR comments. --- .../match/DefaultMatchForLegacyAttributes.java | 2 +- .../ab/config/audience/match/ExactMatch.java | 2 +- .../ab/config/audience/match/ExistsMatch.java | 2 +- .../ab/config/audience/match/GTMatch.java | 4 ++-- .../ab/config/audience/match/LTMatch.java | 2 +- .../ab/config/audience/match/Match.java | 2 +- .../ab/config/audience/match/MatchRegistry.java | 17 ++++++++--------- .../config/audience/match/NumberComparator.java | 6 +----- .../config/audience/match/SubstringMatch.java | 2 +- .../audience/match/MatchRegistryTest.java | 7 ++++--- 10 files changed, 21 insertions(+), 25 deletions(-) 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 5375fd264..c3c970541 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. 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 f5a869f67..5781ac892 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. 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 95bc5c1cb..38fb5a884 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. 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 3824a007d..ba6689c9e 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. @@ -26,4 +26,4 @@ class GTMatch implements Match { public Boolean eval(Object conditionValue, Object attributeValue) throws UnknownValueTypeException { return NumberComparator.compare(attributeValue, conditionValue) > 0; } -} \ No newline at end of file +} 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 0b0ecea13..3000aedff 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. 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 ff3516470..7bef74e6c 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java index bb0458280..a468bc5e2 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/MatchRegistry.java @@ -28,35 +28,34 @@ public class MatchRegistry { private static final Map registry = new ConcurrentHashMap<>(); - public static final String EXISTS = "exists"; public static final String EXACT = "exact"; + public static final String EXISTS = "exists"; public static final String GREATER_THAN = "gt"; + public static final String GREATER_THAN_EQ = "ge"; + public static final String LEGACY = "legacy"; public static final String LESS_THAN = "lt"; public static final String LESS_THAN_EQ = "le"; - public static final String GREATER_THAN_EQ = "ge"; - public static final String SUBSTRING = "substring"; public static final String SEMVER_EQ = "semver_eq"; public static final String SEMVER_GE = "semver_ge"; public static final String SEMVER_GT = "semver_gt"; public static final String SEMVER_LE = "semver_le"; public static final String SEMVER_LT = "semver_lt"; - - public static final String LEGACY = "legacy"; + public static final String SUBSTRING = "substring"; static { - register(EXISTS, new ExistsMatch()); register(EXACT, new ExactMatch()); + register(EXISTS, new ExistsMatch()); register(GREATER_THAN, new GTMatch()); + register(GREATER_THAN_EQ, new GEMatch()); + register(LEGACY, new DefaultMatchForLegacyAttributes()); register(LESS_THAN, new LTMatch()); register(LESS_THAN_EQ, new LEMatch()); - register(GREATER_THAN_EQ, new GEMatch()); - register(SUBSTRING, new SubstringMatch()); register(SEMVER_EQ, new SemanticVersionEqualsMatch()); register(SEMVER_GE, new SemanticVersionGEMatch()); register(SEMVER_GT, new SemanticVersionGTMatch()); register(SEMVER_LE, new SemanticVersionLEMatch()); register(SEMVER_LT, new SemanticVersionLTMatch()); - register(LEGACY, new DefaultMatchForLegacyAttributes()); + register(SUBSTRING, new SubstringMatch()); } // TODO rename Match to Matcher diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java index c7996b1e7..49ce94eab 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/NumberComparator.java @@ -24,11 +24,7 @@ */ public class NumberComparator { public static int compare(Object o1, Object o2) throws UnknownValueTypeException { - if (!isValidNumber(o1)) { - throw new UnknownValueTypeException(); - } - - if (!isValidNumber(o2)) { + if (!isValidNumber(o1) || !isValidNumber(o2)) { throw new UnknownValueTypeException(); } 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 5565040dc..5a573e495 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 @@ -1,6 +1,6 @@ /** * - * Copyright 2018-2019, Optimizely and contributors + * Copyright 2018-2020, 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. diff --git a/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java index a41f2d17a..cb6f2059e 100644 --- a/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java +++ b/core-api/src/test/java/com/optimizely/ab/config/audience/match/MatchRegistryTest.java @@ -26,18 +26,19 @@ public class MatchRegistryTest { @Test public void testDefaultMatchers() throws UnknownMatchTypeException { - assertThat(MatchRegistry.getMatch(EXISTS), instanceOf(ExistsMatch.class)); assertThat(MatchRegistry.getMatch(EXACT), instanceOf(ExactMatch.class)); + assertThat(MatchRegistry.getMatch(EXISTS), instanceOf(ExistsMatch.class)); assertThat(MatchRegistry.getMatch(GREATER_THAN), instanceOf(GTMatch.class)); assertThat(MatchRegistry.getMatch(LESS_THAN), instanceOf(LTMatch.class)); - assertThat(MatchRegistry.getMatch(LESS_THAN_EQ), instanceOf(LEMatch.class)); assertThat(MatchRegistry.getMatch(GREATER_THAN_EQ), instanceOf(GEMatch.class)); - assertThat(MatchRegistry.getMatch(SUBSTRING), instanceOf(SubstringMatch.class)); + assertThat(MatchRegistry.getMatch(LESS_THAN_EQ), instanceOf(LEMatch.class)); + assertThat(MatchRegistry.getMatch(LEGACY), instanceOf(DefaultMatchForLegacyAttributes.class)); assertThat(MatchRegistry.getMatch(SEMVER_EQ), instanceOf(SemanticVersionEqualsMatch.class)); assertThat(MatchRegistry.getMatch(SEMVER_GE), instanceOf(SemanticVersionGEMatch.class)); assertThat(MatchRegistry.getMatch(SEMVER_GT), instanceOf(SemanticVersionGTMatch.class)); assertThat(MatchRegistry.getMatch(SEMVER_LE), instanceOf(SemanticVersionLEMatch.class)); assertThat(MatchRegistry.getMatch(SEMVER_LT), instanceOf(SemanticVersionLTMatch.class)); + assertThat(MatchRegistry.getMatch(SUBSTRING), instanceOf(SubstringMatch.class)); } @Test(expected = UnknownMatchTypeException.class) From 2bf7f0a689bec53df7fc02a70e754786d5d8a894 Mon Sep 17 00:00:00 2001 From: Mike Davis Date: Tue, 15 Sep 2020 14:54:36 -0700 Subject: [PATCH 15/15] Add doc strings for the Match exceptions. --- .../config/audience/match/UnexpectedValueTypeException.java | 4 ++++ .../ab/config/audience/match/UnknownMatchTypeException.java | 3 +++ .../ab/config/audience/match/UnknownValueTypeException.java | 4 ++++ 3 files changed, 11 insertions(+) diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnexpectedValueTypeException.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnexpectedValueTypeException.java index cf513bc7d..39cde7a21 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnexpectedValueTypeException.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnexpectedValueTypeException.java @@ -17,6 +17,10 @@ package com.optimizely.ab.config.audience.match; +/** + * UnexpectedValueTypeException is thrown when the condition value found in the datafile is + * not one of an expected type for this version of the SDK. + */ public class UnexpectedValueTypeException extends Exception { private static String message = "has an unsupported condition value. You may need to upgrade to a newer release of the Optimizely SDK."; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownMatchTypeException.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownMatchTypeException.java index 0c5a972a7..1f371586b 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownMatchTypeException.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownMatchTypeException.java @@ -17,6 +17,9 @@ package com.optimizely.ab.config.audience.match; +/** + * UnknownMatchTypeException is thrown when the specified match type cannot be mapped via the MatchRegistry. + */ public class UnknownMatchTypeException extends Exception { private static String message = "uses an unknown match type. You may need to upgrade to a newer release of the Optimizely SDK."; diff --git a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java index 3fc93b8d8..6df4ef1e1 100644 --- a/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java +++ b/core-api/src/main/java/com/optimizely/ab/config/audience/match/UnknownValueTypeException.java @@ -17,6 +17,10 @@ package com.optimizely.ab.config.audience.match; +/** + * UnknownValueTypeException is thrown when the passed in value for a user attribute does + * not map to a known allowable type. + */ public class UnknownValueTypeException extends Exception { private static String message = "has an unsupported attribute value.";