Skip to content

Commit 78ce524

Browse files
oakbanioakbani
committed
reorganize
1 parent 8414a92 commit 78ce524

File tree

2 files changed

+113
-46
lines changed

2 files changed

+113
-46
lines changed

‎optimizely/helpers/condition.py

Lines changed: 100 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,12 @@ def has_white_space(self, version):
136136
"""
137137
return VersionType.HAS_WHITE_SPACE in version
138138

139-
def compare_user_version_with_target_version(self, index):
139+
def compare_user_version_with_target_version(self, target_version, user_version):
140140
""" Method to compare user version with target version.
141141
142142
Args:
143-
index: Index of the condition to be evaluated.
143+
target_version: String representing condition value
144+
user_version: String representing user value
144145
145146
Returns:
146147
Int:
@@ -149,23 +150,8 @@ def compare_user_version_with_target_version(self, index):
149150
- -1 if user version is less than target version or, in case of exact string match, doesn't match the target
150151
version.
151152
None:
152-
- if the user version value is not string type or is null.
153+
- if the user version value format is not a valid semantic version.
153154
"""
154-
condition_name = self.condition_data[index][0]
155-
target_version = self.condition_data[index][1]
156-
user_version = self.attributes.get(condition_name)
157-
158-
if not isinstance(target_version, string_types):
159-
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
160-
return None
161-
162-
if not isinstance(user_version, string_types):
163-
self.logger.warning(
164-
audience_logs.UNEXPECTED_TYPE.format(
165-
self._get_condition_json(index), type(user_version), condition_name
166-
)
167-
)
168-
return None
169155

170156
target_version_parts = self.split_version(target_version)
171157
if target_version_parts is None:
@@ -426,7 +412,24 @@ def semver_equal_evaluator(self, index):
426412
None:
427413
- if the user version value is not string type or is null.
428414
"""
429-
result = self.compare_user_version_with_target_version(index)
415+
416+
condition_name = self.condition_data[index][0]
417+
target_version = self.condition_data[index][1]
418+
user_version = self.attributes.get(condition_name)
419+
420+
if not isinstance(target_version, string_types):
421+
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
422+
return None
423+
424+
if not isinstance(user_version, string_types):
425+
self.logger.warning(
426+
audience_logs.UNEXPECTED_TYPE.format(
427+
self._get_condition_json(index), type(user_version), condition_name
428+
)
429+
)
430+
return None
431+
432+
result = self.compare_user_version_with_target_version(target_version, user_version)
430433
if result is None:
431434
return None
432435

@@ -445,7 +448,23 @@ def semver_greater_than_evaluator(self, index):
445448
None:
446449
- if the user version value is not string type or is null.
447450
"""
448-
result = self.compare_user_version_with_target_version(index)
451+
condition_name = self.condition_data[index][0]
452+
target_version = self.condition_data[index][1]
453+
user_version = self.attributes.get(condition_name)
454+
455+
if not isinstance(target_version, string_types):
456+
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
457+
return None
458+
459+
if not isinstance(user_version, string_types):
460+
self.logger.warning(
461+
audience_logs.UNEXPECTED_TYPE.format(
462+
self._get_condition_json(index), type(user_version), condition_name
463+
)
464+
)
465+
return None
466+
467+
result = self.compare_user_version_with_target_version(target_version, user_version)
449468
if result is None:
450469
return None
451470

@@ -464,7 +483,23 @@ def semver_less_than_evaluator(self, index):
464483
None:
465484
- if the user version value is not string type or is null.
466485
"""
467-
result = self.compare_user_version_with_target_version(index)
486+
condition_name = self.condition_data[index][0]
487+
target_version = self.condition_data[index][1]
488+
user_version = self.attributes.get(condition_name)
489+
490+
if not isinstance(target_version, string_types):
491+
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
492+
return None
493+
494+
if not isinstance(user_version, string_types):
495+
self.logger.warning(
496+
audience_logs.UNEXPECTED_TYPE.format(
497+
self._get_condition_json(index), type(user_version), condition_name
498+
)
499+
)
500+
return None
501+
502+
result = self.compare_user_version_with_target_version(target_version, user_version)
468503
if result is None:
469504
return None
470505

@@ -483,7 +518,23 @@ def semver_less_than_or_equal_evaluator(self, index):
483518
None:
484519
- if the user version value is not string type or is null.
485520
"""
486-
result = self.compare_user_version_with_target_version(index)
521+
condition_name = self.condition_data[index][0]
522+
target_version = self.condition_data[index][1]
523+
user_version = self.attributes.get(condition_name)
524+
525+
if not isinstance(target_version, string_types):
526+
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
527+
return None
528+
529+
if not isinstance(user_version, string_types):
530+
self.logger.warning(
531+
audience_logs.UNEXPECTED_TYPE.format(
532+
self._get_condition_json(index), type(user_version), condition_name
533+
)
534+
)
535+
return None
536+
537+
result = self.compare_user_version_with_target_version(target_version, user_version)
487538
if result is None:
488539
return None
489540

@@ -502,7 +553,23 @@ def semver_greater_than_or_equal_evaluator(self, index):
502553
None:
503554
- if the user version value is not string type or is null.
504555
"""
505-
result = self.compare_user_version_with_target_version(index)
556+
condition_name = self.condition_data[index][0]
557+
target_version = self.condition_data[index][1]
558+
user_version = self.attributes.get(condition_name)
559+
560+
if not isinstance(target_version, string_types):
561+
self.logger.warning(audience_logs.UNKNOWN_CONDITION_VALUE.format(self._get_condition_json(index), ))
562+
return None
563+
564+
if not isinstance(user_version, string_types):
565+
self.logger.warning(
566+
audience_logs.UNEXPECTED_TYPE.format(
567+
self._get_condition_json(index), type(user_version), condition_name
568+
)
569+
)
570+
return None
571+
572+
result = self.compare_user_version_with_target_version(target_version, user_version)
506573
if result is None:
507574
return None
508575

@@ -523,35 +590,35 @@ def semver_greater_than_or_equal_evaluator(self, index):
523590
ConditionMatchTypes.SUBSTRING: substring_evaluator
524591
}
525592

526-
def split_version(self, target):
593+
def split_version(self, version):
527594
""" Method to split the given version.
528595
529596
Args:
530-
target: Given version.
597+
version: Given version.
531598
532599
Returns:
533600
List:
534601
- The array of version split into smaller parts i.e major, minor, patch etc
535602
None:
536603
- if the given version is invalid in format
537604
"""
538-
target_prefix = target
605+
target_prefix = version
539606
target_suffix = ""
540607
target_parts = []
541608

542-
# check that target shouldn't have white space
543-
if self.has_white_space(target):
609+
# check that version shouldn't have white space
610+
if self.has_white_space(version):
544611
self.logger.warning(Errors.INVALID_ATTRIBUTE_FORMAT)
545612
return None
546613

547614
# check for pre release e.g. 1.0.0-alpha where 'alpha' is a pre release
548615
# otherwise check for build e.g. 1.0.0+001 where 001 is a build metadata
549-
if self.is_pre_release(target):
550-
target_parts = target.split(VersionType.IS_PRE_RELEASE)
551-
elif self.is_build(target):
552-
target_parts = target.split(VersionType.IS_BUILD)
616+
if self.is_pre_release(version):
617+
target_parts = version.split(VersionType.IS_PRE_RELEASE)
618+
elif self.is_build(version):
619+
target_parts = version.split(VersionType.IS_BUILD)
553620

554-
# split target version into prefix and suffix
621+
# split version into prefix and suffix
555622
if target_parts:
556623
if len(target_parts) < 1:
557624
self.logger.warning(Errors.INVALID_ATTRIBUTE_FORMAT)

‎tests/helpers_tests/test_condition.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1350,6 +1350,19 @@ def is_finite_number__accepting_both_values(value):
13501350
):
13511351
self.assertTrue(evaluator.evaluate(0))
13521352

1353+
def test_invalid_semver__returns_None__when_semver_is_invalid(self):
1354+
invalid_test_cases = ["-", ".", "..", "+", "+test", " ", "2 .0. 0",
1355+
"2.", ".0.0", "1.2.2.2", "2.x", ",",
1356+
"+build-prerelease", "2..0", "2.2.0+build-prerelease"]
1357+
1358+
for user_version in invalid_test_cases:
1359+
evaluator = condition_helper.CustomAttributeConditionEvaluator(
1360+
semver_less_than_or_equal_2_0_1_condition_list, {'Android': user_version}, self.mock_client_logger)
1361+
1362+
result = evaluator.evaluate(0)
1363+
custom_err_msg = "Got {} in result. Failed for user version: {}".format(result, user_version)
1364+
self.assertIsNone(result, custom_err_msg)
1365+
13531366

13541367
class ConditionDecoderTests(base.BaseTest):
13551368
def test_loads(self):
@@ -2008,16 +2021,3 @@ def test_substring__condition_value_invalid(self):
20082021
'newer release of the Optimizely SDK.'
20092022
).format(json.dumps(expected_condition_log))
20102023
)
2011-
2012-
def test_invalid_semver__returns_None__when_semver_is_invalid(self):
2013-
invalid_test_cases = ["-", ".", "..", "+", "+test", " ", "2 .0. 0",
2014-
"2.", ".0.0", "1.2.2.2", "2.x", ",",
2015-
"+build-prerelease", "2..0", "2.2.0+build-prerelease"]
2016-
2017-
for user_version in invalid_test_cases:
2018-
evaluator = condition_helper.CustomAttributeConditionEvaluator(
2019-
semver_less_than_or_equal_2_0_1_condition_list, {'Android': user_version}, self.mock_client_logger)
2020-
2021-
result = evaluator.evaluate(0)
2022-
custom_err_msg = "Got {} in result. Failed for user version: {}".format(result, user_version)
2023-
self.assertIsNone(result, custom_err_msg)

0 commit comments

Comments
 (0)