From e84a98959d1188b8ad206f5033703ec6029fd551 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Thu, 7 Mar 2024 15:07:32 -0800 Subject: [PATCH 1/4] Add semver support (FF-1573) --- eppo/build.gradle | 1 + .../cloud/eppo/android/RuleEvaluator.java | 53 +++++++++++++++---- .../cloud/eppo/android/RuleEvaluatorTest.java | 27 ++++++++++ 3 files changed, 71 insertions(+), 10 deletions(-) diff --git a/eppo/build.gradle b/eppo/build.gradle index 2a668cf..6e7c87c 100644 --- a/eppo/build.gradle +++ b/eppo/build.gradle @@ -71,6 +71,7 @@ dependencies { androidTestImplementation "commons-io:commons-io:${versions.commonsio}" implementation("com.google.code.gson:gson:${versions.gson}") implementation("com.squareup.okhttp3:okhttp:${versions.okhttp}") + implementation("com.github.zafarkhaja:java-semver:0.10.2") } publishing { diff --git a/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java b/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java index 780f540..d8e47bc 100644 --- a/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java +++ b/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java @@ -1,9 +1,10 @@ package cloud.eppo.android; +import com.github.zafarkhaja.semver.Version; + import java.util.ArrayList; import java.util.List; import java.util.regex.Pattern; -import java.util.stream.Collectors; import cloud.eppo.android.dto.EppoValue; import cloud.eppo.android.dto.SubjectAttributes; @@ -15,10 +16,6 @@ interface IConditionFunc { } class Compare { - public static boolean compareNumber(double a, double b, IConditionFunc conditionFunc) { - return conditionFunc.check(a, b); - } - public static boolean compareRegex(String a, Pattern pattern) { return pattern.matcher(a).matches(); } @@ -48,21 +45,57 @@ private static boolean matchesRule(SubjectAttributes subjectAttributes, Targetin return !conditionEvaluations.contains(false); } + + private static boolean evaluateCondition(SubjectAttributes subjectAttributes, TargetingCondition condition ) { if (subjectAttributes.containsKey(condition.getAttribute())) { EppoValue value = subjectAttributes.get(condition.getAttribute()); + Boolean isValueSemVer = Version.isValid(value.stringValue()); + Boolean isConditionSemVer = Version.isValid(condition.getValue().stringValue()); + try { switch (condition.getOperator()) { case GreaterThanEqualTo: - return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue() - , (a, b) -> a >= b); + if (value.isNumeric() && condition.getValue().isNumeric()) { + return value.doubleValue() >= condition.getValue().doubleValue(); + } + + if (isValueSemVer && isConditionSemVer) { + return Version.parse(value.stringValue()).isHigherThanOrEquivalentTo(Version.parse(condition.getValue().stringValue())); + } + + return false; case GreaterThan: - return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue(), (a, b) -> a > b); + if (value.isNumeric() && condition.getValue().isNumeric()) { + return value.doubleValue() > condition.getValue().doubleValue(); + } + + if (isValueSemVer && isConditionSemVer) { + return Version.parse(value.stringValue()).isHigherThan(Version.parse(condition.getValue().stringValue())); + } + + return false; case LessThanEqualTo: - return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue(), (a, b) -> a <= b); + if (value.isNumeric() && condition.getValue().isNumeric()) { + return value.doubleValue() <= condition.getValue().doubleValue(); + } + + if (isValueSemVer && isConditionSemVer) { + return Version.parse(value.stringValue()).isLowerThanOrEquivalentTo(Version.parse(condition.getValue().stringValue())); + } + + return false; case LessThan: - return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue(), (a, b) -> a < b); + if (value.isNumeric() && condition.getValue().isNumeric()) { + return value.doubleValue() < condition.getValue().doubleValue(); + } + + if (isValueSemVer && isConditionSemVer) { + return Version.parse(value.stringValue()).isLowerThan(Version.parse(condition.getValue().stringValue())); + } + + return false; case Matches: return Compare.compareRegex(value.stringValue(), Pattern.compile(condition.getValue().stringValue())); case OneOf: diff --git a/eppo/src/test/java/cloud/eppo/android/RuleEvaluatorTest.java b/eppo/src/test/java/cloud/eppo/android/RuleEvaluatorTest.java index 387651f..ce157ce 100644 --- a/eppo/src/test/java/cloud/eppo/android/RuleEvaluatorTest.java +++ b/eppo/src/test/java/cloud/eppo/android/RuleEvaluatorTest.java @@ -43,6 +43,21 @@ public void addNumericConditionToRule(TargetingRule TargetingRule) { addConditionToRule(TargetingRule, condition2); } + public void addSemVerConditionToRule(TargetingRule TargetingRule) { + TargetingCondition condition1 = new TargetingCondition(); + condition1.setValue(EppoValue.valueOf("1.5.0")); + condition1.setAttribute("appVersion"); + condition1.setOperator(OperatorType.GreaterThanEqualTo); + + TargetingCondition condition2 = new TargetingCondition(); + condition2.setValue(EppoValue.valueOf("2.2.0")); + condition2.setAttribute("appVersion"); + condition2.setOperator(OperatorType.LessThan); + + addConditionToRule(TargetingRule, condition1); + addConditionToRule(TargetingRule, condition2); + } + public void addRegexConditionToRule(TargetingRule TargetingRule) { TargetingCondition condition = new TargetingCondition(); condition.setValue(EppoValue.valueOf("[a-z]+")); @@ -131,6 +146,18 @@ public void testMatchesAnyRuleWhenRuleMatches() { assertEquals(targetingRule, RuleEvaluator.findMatchingRule(subjectAttributes, targetingRules)); } + @Test + public void testMatchesAnyRuleWhenRuleMatchesWithSemVer() { + List targetingRules = new ArrayList<>(); + TargetingRule targetingRule = createRule(new ArrayList<>()); + addSemVerConditionToRule(targetingRule); + targetingRules.add(targetingRule); + + SubjectAttributes subjectAttributes = new SubjectAttributes(); + subjectAttributes.put("appVersion", "1.15.5"); + + assertEquals(targetingRule, RuleEvaluator.findMatchingRule(subjectAttributes, targetingRules)); + } @Test public void testMatchesAnyRuleWhenThrowInvalidSubjectAttribute() { From c3eb842791fb3430e4a1afef427de5fb08daf879 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Thu, 7 Mar 2024 18:12:25 -0800 Subject: [PATCH 2/4] Fix bug in isNumeric unable to parse decimal values. --- .../main/java/cloud/eppo/android/dto/EppoValue.java | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/eppo/src/main/java/cloud/eppo/android/dto/EppoValue.java b/eppo/src/main/java/cloud/eppo/android/dto/EppoValue.java index b244376..564199f 100644 --- a/eppo/src/main/java/cloud/eppo/android/dto/EppoValue.java +++ b/eppo/src/main/java/cloud/eppo/android/dto/EppoValue.java @@ -57,18 +57,10 @@ public static EppoValue valueOf() { return new EppoValue(EppoValueType.Null); } - public int intValue() { - return Integer.parseInt(value, 10); - } - public double doubleValue() { return Double.parseDouble(value); } - public long longValue() { - return Long.parseLong(value, 10); - } - public String stringValue() { return value; } @@ -87,7 +79,7 @@ public JsonElement jsonValue() { public boolean isNumeric() { try { - Long.parseLong(value, 10); + Double.parseDouble(value); return true; } catch (Exception e) { return false; From af7a2ea7a105605bd20228a3d3dacf777438bc12 Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Thu, 7 Mar 2024 18:14:21 -0800 Subject: [PATCH 3/4] sleep for 10 seconds --- .../src/androidTest/java/cloud/eppo/android/EppoClientTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java b/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java index eaa7437..1ade4f7 100644 --- a/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java +++ b/eppo/src/androidTest/java/cloud/eppo/android/EppoClientTest.java @@ -262,7 +262,7 @@ public void testCachedAssignments() { // wait for a bit since cache file is loaded asynchronously System.out.println("Sleeping for a bit to wait for cache population to complete"); - Thread.sleep(5000); + Thread.sleep(10000); // Then reinitialize with a bad host so we know it's using the cached RAC built from the first initialization initClient(INVALID_HOST, false, false, false); // invalid port to force to use cache From 20579efd130de79932c5c9ace20dcbd66430bdcd Mon Sep 17 00:00:00 2001 From: Leo Romanovsky Date: Fri, 8 Mar 2024 10:57:02 -0800 Subject: [PATCH 4/4] refactor semver and numeric checks to DRY the code. --- .../cloud/eppo/android/RuleEvaluator.java | 55 ++++++++++++------- 1 file changed, 35 insertions(+), 20 deletions(-) diff --git a/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java b/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java index d8e47bc..4075cd1 100644 --- a/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java +++ b/eppo/src/main/java/cloud/eppo/android/RuleEvaluator.java @@ -11,10 +11,6 @@ import cloud.eppo.android.dto.TargetingCondition; import cloud.eppo.android.dto.TargetingRule; -interface IConditionFunc { - boolean check(T a, T b); -} - class Compare { public static boolean compareRegex(String a, Pattern pattern) { return pattern.matcher(a).matches(); @@ -45,54 +41,73 @@ private static boolean matchesRule(SubjectAttributes subjectAttributes, Targetin return !conditionEvaluations.contains(false); } - - private static boolean evaluateCondition(SubjectAttributes subjectAttributes, TargetingCondition condition ) { if (subjectAttributes.containsKey(condition.getAttribute())) { EppoValue value = subjectAttributes.get(condition.getAttribute()); - Boolean isValueSemVer = Version.isValid(value.stringValue()); - Boolean isConditionSemVer = Version.isValid(condition.getValue().stringValue()); + if (value == null) { + return false; + } + + boolean numericComparison = value.isNumeric() && condition.getValue().isNumeric(); + + // Android API version 21 does not have access to the java.util.Optional class. + // Version.tryParse returns a Optional would be ideal. + // Instead use Version.parse which throws an exception if the string is not a valid SemVer. + // We front-load the parsing here so many evaluation of gte, gt, lte, lt operations + // more straight-forward. + Version valueSemVer = null; + Version conditionSemVer = null; + try { + valueSemVer = Version.parse(value.stringValue()); + conditionSemVer = Version.parse(condition.getValue().stringValue()); + } catch (Exception e) { + // no-op + } + + // Performing this check satisfies the compiler that the possibly + // null value can be safely accessed later. + boolean semVerComparison = valueSemVer != null && conditionSemVer != null; try { switch (condition.getOperator()) { case GreaterThanEqualTo: - if (value.isNumeric() && condition.getValue().isNumeric()) { + if (numericComparison) { return value.doubleValue() >= condition.getValue().doubleValue(); } - if (isValueSemVer && isConditionSemVer) { - return Version.parse(value.stringValue()).isHigherThanOrEquivalentTo(Version.parse(condition.getValue().stringValue())); + if (semVerComparison) { + return valueSemVer.isHigherThanOrEquivalentTo(conditionSemVer); } return false; case GreaterThan: - if (value.isNumeric() && condition.getValue().isNumeric()) { + if (numericComparison) { return value.doubleValue() > condition.getValue().doubleValue(); } - if (isValueSemVer && isConditionSemVer) { - return Version.parse(value.stringValue()).isHigherThan(Version.parse(condition.getValue().stringValue())); + if (semVerComparison) { + return valueSemVer.isHigherThan(conditionSemVer); } return false; case LessThanEqualTo: - if (value.isNumeric() && condition.getValue().isNumeric()) { + if (numericComparison) { return value.doubleValue() <= condition.getValue().doubleValue(); } - if (isValueSemVer && isConditionSemVer) { - return Version.parse(value.stringValue()).isLowerThanOrEquivalentTo(Version.parse(condition.getValue().stringValue())); + if (semVerComparison) { + return valueSemVer.isLowerThanOrEquivalentTo(conditionSemVer); } return false; case LessThan: - if (value.isNumeric() && condition.getValue().isNumeric()) { + if (numericComparison) { return value.doubleValue() < condition.getValue().doubleValue(); } - if (isValueSemVer && isConditionSemVer) { - return Version.parse(value.stringValue()).isLowerThan(Version.parse(condition.getValue().stringValue())); + if (semVerComparison) { + return valueSemVer.isLowerThan(conditionSemVer); } return false;