Skip to content

feat: Semantic Versioning #293

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 39 commits into from
Sep 17, 2020
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
39 commits
Select commit Hold shift + click to select a range
ac7e81f
feat:semantic versioning
Jul 30, 2020
ee29674
semver updated
Jul 30, 2020
4a5e75d
more validation for invalid cases added
Aug 5, 2020
01ed4da
feat: Semantic Version
oakbani Aug 21, 2020
796af4d
GE and LE with test cases
Aug 24, 2020
f318995
Update test_condition.py
Aug 24, 2020
e016cd8
added ge le
msohailhussain Aug 24, 2020
857e8de
PR comments resolved
Aug 25, 2020
5f793a3
comments resolved
Aug 25, 2020
cfb12df
Update condition.py
Aug 25, 2020
379d637
Merge branch 'uzair/semver' into amna/semver
oakbani Aug 26, 2020
1c37fbf
invalid test case for semver and comment fixed
Aug 26, 2020
dcd391f
Update test_condition.py
Aug 26, 2020
2affb9d
Update test_condition.py
Aug 26, 2020
032d9c1
Merge branch 'uzair/semver' into amna/semver
oakbani Aug 26, 2020
8577f7c
compare implemetation and invalid testcase fixed
Aug 26, 2020
32bed45
Merge branch 'uzair/semver' into amna/semver
oakbani Aug 27, 2020
426d2b7
Revert "compare implemetation and invalid testcase fixed"
oakbani Aug 27, 2020
ac8113e
passes fsc at this point
oakbani Aug 27, 2020
84f6c26
fix:lint
oakbani Aug 27, 2020
ca7984a
remove: additional lint fixes
oakbani Aug 27, 2020
9256e6a
additional removal
oakbani Aug 27, 2020
15fa71b
further removal
oakbani Aug 27, 2020
8414a92
address most comments
oakbani Aug 28, 2020
78ce524
reorganize
oakbani Aug 28, 2020
aa1fbaf
tests: revised all unit tests
oakbani Aug 31, 2020
c8c0e75
Merge branch 'master' into amna/semver
oakbani Sep 2, 2020
907fcf2
address comments
oakbani Sep 2, 2020
8b4567c
add further checks
oakbani Sep 3, 2020
8856665
comments resolved
Sep 9, 2020
7ed20f0
comments resolved
Sep 9, 2020
4f2c05e
Update test_condition.py
Sep 14, 2020
4070805
Revert "Update test_condition.py"
Sep 14, 2020
4c471bd
Update test_condition.py
Sep 14, 2020
68708d9
Update test_condition.py
Sep 16, 2020
09533b1
Merge branch 'master' into amna/semver
msohailhussain Sep 16, 2020
ad17c02
testcase fixed
Sep 17, 2020
93cbb2a
Update condition.py
Sep 17, 2020
25ab44d
fix condition
oakbani Sep 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
217 changes: 216 additions & 1 deletion optimizely/helpers/condition.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@

from . import validator
from .enums import CommonAudienceEvaluationLogs as audience_logs
from .enums import Errors, SemverType
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Convention is to import on separate lines. So please change this to:

from .enums import Errors
from .enums import SemverType



class ConditionOperatorTypes(object):
Expand All @@ -31,6 +32,11 @@ class ConditionMatchTypes(object):
EXISTS = 'exists'
GREATER_THAN = 'gt'
LESS_THAN = 'lt'
SEMVER_EQ = 'semver_eq'
SEMVER_GE = 'semver_ge'
SEMVER_GT = 'semver_gt'
SEMVER_LE = 'semver_le'
SEMVER_LT = 'semver_lt'
SUBSTRING = 'substring'


Expand Down Expand Up @@ -233,12 +239,221 @@ def substring_evaluator(self, index):

return condition_value in user_value

def semver_equal_evaluator(self, index):
""" Evaluate the given semantic version equal match target version for the user version.

Args:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit. Indentation is off. This needs to align with the quotes above.

index: Index of the condition to be evaluated.

Returns:
Boolean:
- True if the user version is equal (==) to the target version.
- False if the user version is not equal (!=) to the target version.
None:
- if the user version value is not string type or is null.
"""
return self.compare_user_version_with_target_version(index) == 0

def semver_greater_than_evaluator(self, index):
""" Evaluate the given semantic version greater than match target version for the user version.

Args:
index: Index of the condition to be evaluated.

Returns:
Boolean:
- True if the user version is greater than the target version.
- False if the user version is less than or equal to the target version.
None:
- if the user version value is not string type or is null.
"""
return self.compare_user_version_with_target_version(index) > 0

def semver_less_than_evaluator(self, index):
""" Evaluate the given semantic version less than match target version for the user version.

Args:
index: Index of the condition to be evaluated.

Returns:
Boolean:
- True if the user version is less than the target version.
- False if the user version is greater than or equal to the target version.
None:
- if the user version value is not string type or is null.
"""
return self.compare_user_version_with_target_version(index) < 0

def semver_less_than_or_equal_evaluator(self, index):
""" Evaluate the given semantic version less than or equal to match target version for the user version.

Args:
index: Index of the condition to be evaluated.

Returns:
Boolean:
- True if the user version is less than or equal to the target version.
- False if the user version is greater than the target version.
None:
- if the user version value is not string type or is null.
"""
return self.compare_user_version_with_target_version(index) <= 0

def semver_greater_than_or_equal_evaluator(self, index):
""" Evaluate the given semantic version greater than or equal to match target version for the user version.

Args:
index: Index of the condition to be evaluated.

Returns:
Boolean:
- True if the user version is greater than or equal to the target version.
- False if the user version is less than the target version.
None:
- if the user version value is not string type or is null.
"""
return self.compare_user_version_with_target_version(index) >= 0

def split_semantic_version(self, target):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More appropriate name will be split_version

""" Method to split the given version.

Args:
target: Given version.

Returns:
List:
- The array of version split into smaller parts i.e major, minor, patch etc
Exception:
- if the given version is invalid in format
"""
target_prefix = target
target_suffix = ""
target_parts = []

if self.has_white_space(target):
raise Exception(Errors.INVALID_ATTRIBUTE_FORMAT)

if self.is_pre_release(target):
target_parts = target.split(SemverType.IS_PRE_RELEASE)
elif self.is_build(target):
target_parts = target.split(SemverType.IS_BUILD)

if target_parts:
if len(target_parts) < 1:
raise Exception(Errors.INVALID_ATTRIBUTE_FORMAT)
target_prefix = str(target_parts[0])
target_suffix = target_parts[1:]

dot_count = target_prefix.count(".")
if dot_count > 2:
raise Exception(Errors.INVALID_ATTRIBUTE_FORMAT)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add comments over each of these about what is being checked in the if condition?


target_version_parts = target_prefix.split(".")
if len(target_version_parts) != dot_count + 1:
raise Exception(Errors.INVALID_ATTRIBUTE_FORMAT)
for part in target_version_parts:
if not part.isdigit():
raise Exception(Errors.INVALID_ATTRIBUTE_FORMAT)

if target_suffix:
target_version_parts.extend(target_suffix)
return target_version_parts

def is_pre_release(self, target):
""" Method to check if the given version contains "-"

Args:
target: Given version in string.

Returns:
Boolean:
- True if the given version does contain "-"
- False if it doesn't
"""
return SemverType.IS_PRE_RELEASE in target

def is_patch_pre_release(self, idx, idx_value):
return idx == SemverType.PATCH_INDEX and idx_value in SemverType.IS_PATCH_PRE_RELEASE

def is_build(self, target):
""" Method to check if the given version contains "+"

Args:
target: Given version in string.

Returns:
Boolean:
- True if the given version does contain "+"
- False if it doesn't
"""
return SemverType.IS_BUILD in target

def has_white_space(self, target):
""" Method to check if the given version contains " " (white space)

Args:
target: Given version in string.

Returns:
Boolean:
- True if the given version does contain " "
- False if it doesn't
"""
return SemverType.HAS_WHITE_SPACE in target

def compare_user_version_with_target_version(self, index):
""" Method to compare user version with target version.

Args:
index: Index of the condition to be evaluated.

Returns:
Int:
- 0 if user version is equal to target version.
- 1 if user version is greater than target version.
- -1 if user version is less than target version or, in case of exact string match, doesn't match the target
version.
None:
- if the user version value is not string type or is null.
"""
condition_name = self.condition_data[index][0]
target_version = self.condition_data[index][1]
user_version = self.attributes.get(condition_name)

target_version_parts = self.split_semantic_version(target_version)
user_version_parts = self.split_semantic_version(user_version)
user_version_parts_len = len(user_version_parts)

for (idx, _) in enumerate(target_version_parts):
if user_version_parts_len <= idx:
return 1 if self.is_pre_release(target_version) else -1
elif not user_version_parts[idx].isdigit():
if user_version_parts[idx] < target_version_parts[idx]:
return -1
elif user_version_parts[idx] > target_version_parts[idx]:
return 1
else:
user_version_part = int(user_version_parts[idx])
target_version_part = int(target_version_parts[idx])
if user_version_part > target_version_part:
return 1
elif user_version_part < target_version_part:
return -1
if self.is_pre_release(user_version) and not self.is_pre_release(target_version):
return -1
return 0

EVALUATORS_BY_MATCH_TYPE = {
ConditionMatchTypes.EXACT: exact_evaluator,
ConditionMatchTypes.EXISTS: exists_evaluator,
ConditionMatchTypes.GREATER_THAN: greater_than_evaluator,
ConditionMatchTypes.LESS_THAN: less_than_evaluator,
ConditionMatchTypes.SUBSTRING: substring_evaluator,
ConditionMatchTypes.SEMVER_EQ: semver_equal_evaluator,
ConditionMatchTypes.SEMVER_GE: semver_greater_than_or_equal_evaluator,
ConditionMatchTypes.SEMVER_GT: semver_greater_than_evaluator,
ConditionMatchTypes.SEMVER_LE: semver_less_than_or_equal_evaluator,
ConditionMatchTypes.SEMVER_LT: semver_less_than_evaluator,
ConditionMatchTypes.SUBSTRING: substring_evaluator
}

def evaluate(self, index):
Expand Down
6 changes: 6 additions & 0 deletions optimizely/helpers/enums.py
Original file line number Diff line number Diff line change
Expand Up @@ -157,3 +157,9 @@ class NotificationTypes(object):
OPTIMIZELY_CONFIG_UPDATE = 'OPTIMIZELY_CONFIG_UPDATE'
TRACK = 'TRACK:event_key, user_id, attributes, event_tags, event'
LOG_EVENT = 'LOG_EVENT:log_event'


class SemverType(object):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this should be VersionType and not SemverType. Semantic versioning is a type of versioning and not a type of version.

IS_PRE_RELEASE = '-'
HAS_WHITE_SPACE = " "
IS_BUILD = '+'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should use single quotes for all strings.

42 changes: 39 additions & 3 deletions tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,7 @@ def setUp(self, config_dict='config_dict'):
'3468206647',
'3468206644',
'3468206643',
'18278344267'
],
'variations': [
{'variables': [], 'id': '11557362669', 'key': '11557362669', 'featureEnabled': True}
Expand Down Expand Up @@ -556,7 +557,8 @@ def setUp(self, config_dict='config_dict'):
'audienceConditions': [
'and',
['or', '3468206642', '3988293898'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643',
'18278344267'],
],
'variations': [
{'variables': [], 'id': '11557362670', 'key': '11557362670', 'featureEnabled': True}
Expand Down Expand Up @@ -626,6 +628,7 @@ def setUp(self, config_dict='config_dict'):
'3468206647',
'3468206644',
'3468206643',
'18278344267'
],
'variations': [
{
Expand Down Expand Up @@ -653,6 +656,7 @@ def setUp(self, config_dict='config_dict'):
'3468206647',
'3468206644',
'3468206643',
'18278344267'
],
'forcedVariations': {},
},
Expand All @@ -667,7 +671,7 @@ def setUp(self, config_dict='config_dict'):
'audienceConditions': [
'and',
['or', '3468206642', '3988293898'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643', '18278344267'],
],
'forcedVariations': {},
},
Expand All @@ -689,7 +693,7 @@ def setUp(self, config_dict='config_dict'):
'audienceConditions': [
'and',
['or', '3468206642', '3988293898'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643'],
['or', '3988293899', '3468206646', '3468206647', '3468206644', '3468206643', '18278344267'],
],
'forcedVariations': {},
},
Expand Down Expand Up @@ -837,13 +841,45 @@ def setUp(self, config_dict='config_dict'):
],
],
},
{
"id": "18278344267",
"name": "semverReleaseLt1.2.3Gt1.0.0",
"conditions": [
"and",
[
"or",
[
"or",
{
"value": "1.2.3",
"type": "custom_attribute",
"name": "android-release",
"match": "semver_lt"
}
]
],
[
"or",
[
"or",
{
"value": "1.0.0",
"type": "custom_attribute",
"name": "android-release",
"match": "semver_gt"
}
]
]
]
}
],
'groups': [],
'attributes': [
{'key': 'house', 'id': '594015'},
{'key': 'lasers', 'id': '594016'},
{'key': 'should_do_it', 'id': '594017'},
{'key': 'favorite_ice_cream', 'id': '594018'},
{'key': 'android-release', 'id': '594019'},
],
'botFiltering': False,
'accountId': '4879520872',
Expand Down
Loading