Add semver support (FF-1573) #37
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
leoromanovsky
commented
Mar 7, 2024
| try { | ||
| switch (condition.getOperator()) { | ||
| case GreaterThanEqualTo: | ||
| return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue() |
There was a problem hiding this comment.
deleted compareNumber helper
| } | ||
|
|
||
| if (isValueSemVer && isConditionSemVer) { | ||
| return Version.parse(value.stringValue()).isHigherThanOrEquivalentTo(Version.parse(condition.getValue().stringValue())); |
There was a problem hiding this comment.
since we passed isValid we should be confident that parse will succeed without an exception
| targetingRules.add(targetingRule); | ||
|
|
||
| SubjectAttributes subjectAttributes = new SubjectAttributes(); | ||
| subjectAttributes.put("appVersion", "1.15.5"); |
|
leoromanovsky
commented
Mar 8, 2024
| @@ -87,7 +79,7 @@ public JsonElement jsonValue() { | |||
|
|
|||
| public boolean isNumeric() { | |||
| try { | |||
| Long.parseLong(value, 10); | |||
| Double.parseDouble(value); | |||
There was a problem hiding this comment.
decimal values returned false always
Comment on lines
-60
to
-63
| public int intValue() { | ||
| return Integer.parseInt(value, 10); | ||
| } | ||
|
|
There was a problem hiding this comment.
these were unused - unclear to me why we would want to
agamkrbit
reviewed
Mar 8, 2024
| @@ -43,6 +43,21 @@ public void addNumericConditionToRule(TargetingRule TargetingRule) { | |||
| addConditionToRule(TargetingRule, condition2); | |||
| } | |||
|
|
|||
| public void addSemVerConditionToRule(TargetingRule TargetingRule) { | |||
agamkrbit
reviewed
Mar 8, 2024
agamkrbit
approved these changes
Mar 8, 2024
aarsilv
approved these changes
Mar 8, 2024
Comment on lines
-18
to
-20
| public static boolean compareNumber(double a, double b, IConditionFunc<Double> conditionFunc) { | ||
| return conditionFunc.check(a, b); | ||
| } |
Comment on lines
+54
to
+58
| // Android API version 21 does not have access to the java.util.Optional class. | ||
| // Version.tryParse returns a Optional<Version> 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. |
There was a problem hiding this comment.
Thanks for the helpful comment!
| try { | ||
| switch (condition.getOperator()) { | ||
| case GreaterThanEqualTo: | ||
| return Compare.compareNumber(value.doubleValue(), condition.getValue().doubleValue() |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
context
Related implementation to Java (Eppo-exp/java-server-sdk@11c33e3). Since we are targeting android api 21, we are limited to an older API.
isPresent, which require API level 24 or higher, you'll need to find alternative approaches. The Optional class and its methods were introduced in Java 8, and Android started supporting Java 8 language features from API level 24 onwards. This means methods like isPresent and isEmpty are not directly available on devices running lower API levels.
description
implementation of semver support using available APIs.