Skip to content

Commit 5f6f03e

Browse files
author
Pijush Chakraborty
committed
Fixing semantic version issues with invalid version
1 parent f9f0635 commit 5f6f03e

File tree

2 files changed

+74
-12
lines changed

2 files changed

+74
-12
lines changed

‎firebase_admin/remote_config.py

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -593,9 +593,9 @@ def _compare_versions(self, version1, version2, predicate_fn) -> bool:
593593
"""Compares two semantic version strings.
594594
595595
Args:
596-
version1: The first semantic version string.
597-
version2: The second semantic version string.
598-
predicate_fn: A function that takes an integer and returns a boolean.
596+
version1: The first semantic version string.
597+
version2: The second semantic version string.
598+
predicate_fn: A function that takes an integer and returns a boolean.
599599
600600
Returns:
601601
bool: The result of the predicate function.
@@ -608,6 +608,8 @@ def _compare_versions(self, version1, version2, predicate_fn) -> bool:
608608
v2_parts.extend([0] * (max_length - len(v2_parts)))
609609

610610
for part1, part2 in zip(v1_parts, v2_parts):
611+
if any((part1 < 0, part2 < 0)):
612+
raise ValueError
611613
if part1 < part2:
612614
return predicate_fn(-1)
613615
if part1 > part2:

‎tests/test_remote_config.py

Lines changed: 69 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import pytest
1919
import firebase_admin
2020
from firebase_admin.remote_config import (
21+
CustomSignalOperator,
2122
PercentConditionOperator,
2223
_REMOTE_CONFIG_ATTRIBUTE,
2324
_RemoteConfigService,
@@ -66,6 +67,12 @@
6667
'version': VERSION_INFO,
6768
}
6869

70+
SEMENTIC_VERSION_LESS_THAN_TRUE = [CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN.value, ['12.1.3.444'], '12.1.3.443', True]
71+
SEMENTIC_VERSION_EQUAL_TRUE = [CustomSignalOperator.SEMANTIC_VERSION_EQUAL.value, ['12.1.3.444'], '12.1.3.444', True]
72+
SEMANTIC_VERSION_GREATER_THAN_FALSE = [CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN.value, ['12.1.3.4'], '12.1.3.4', False]
73+
SEMANTIC_VERSION_INVALID_FORMAT_STRING = [CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN.value, ['12.1.3.444'], '12.1.3.abc', False]
74+
SEMANTIC_VERSION_INVALID_FORMAT_NEGATIVE_INTEGER = [CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN.value, ['12.1.3.444'], '12.1.3.-2', False]
75+
6976
class TestEvaluate:
7077
@classmethod
7178
def setup_class(cls):
@@ -360,7 +367,7 @@ def test_evaluate_unknown_operator_to_false(self):
360367
'andCondition': {
361368
'conditions': [{
362369
'percent': {
363-
'percentOperator': PercentConditionOperator.UNKNOWN
370+
'percentOperator': PercentConditionOperator.UNKNOWN.value
364371
}
365372
}],
366373
}
@@ -402,7 +409,7 @@ def test_evaluate_less_or_equal_to_max_to_true(self):
402409
'andCondition': {
403410
'conditions': [{
404411
'percent': {
405-
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL,
412+
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL.value,
406413
'seed': 'abcdef',
407414
'microPercent': 100_000_000
408415
}
@@ -446,7 +453,7 @@ def test_evaluate_undefined_micropercent_to_false(self):
446453
'andCondition': {
447454
'conditions': [{
448455
'percent': {
449-
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL,
456+
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL.value,
450457
# Leaves microPercent undefined
451458
}
452459
}],
@@ -489,7 +496,7 @@ def test_evaluate_undefined_micropercentrange_to_false(self):
489496
'andCondition': {
490497
'conditions': [{
491498
'percent': {
492-
'percentOperator': PercentConditionOperator.BETWEEN,
499+
'percentOperator': PercentConditionOperator.BETWEEN.value,
493500
# Leaves microPercent undefined
494501
}
495502
}],
@@ -532,7 +539,7 @@ def test_evaluate_between_min_max_to_true(self):
532539
'andCondition': {
533540
'conditions': [{
534541
'percent': {
535-
'percentOperator': PercentConditionOperator.BETWEEN,
542+
'percentOperator': PercentConditionOperator.BETWEEN.value,
536543
'seed': 'abcdef',
537544
'microPercentRange': {
538545
'microPercentLowerBound': 0,
@@ -579,7 +586,7 @@ def test_evaluate_between_equal_bounds_to_false(self):
579586
'andCondition': {
580587
'conditions': [{
581588
'percent': {
582-
'percentOperator': PercentConditionOperator.BETWEEN,
589+
'percentOperator': PercentConditionOperator.BETWEEN.value,
583590
'seed': 'abcdef',
584591
'microPercentRange': {
585592
'microPercentLowerBound': 50000000,
@@ -626,7 +633,7 @@ def test_evaluate_less_or_equal_to_approx(self):
626633
'andCondition': {
627634
'conditions': [{
628635
'percent': {
629-
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL,
636+
'percentOperator': PercentConditionOperator.LESS_OR_EQUAL.value,
630637
'seed': 'abcdef',
631638
'microPercent': 10_000_000 # 10%
632639
}
@@ -656,7 +663,7 @@ def test_evaluate_between_approx(self):
656663
'andCondition': {
657664
'conditions': [{
658665
'percent': {
659-
'percentOperator': PercentConditionOperator.BETWEEN,
666+
'percentOperator': PercentConditionOperator.BETWEEN.value,
660667
'seed': 'abcdef',
661668
'microPercentRange': {
662669
'microPercentLowerBound': 40_000_000,
@@ -689,7 +696,7 @@ def test_evaluate_between_interquartile_range_accuracy(self):
689696
'andCondition': {
690697
'conditions': [{
691698
'percent': {
692-
'percentOperator': PercentConditionOperator.BETWEEN,
699+
'percentOperator': PercentConditionOperator.BETWEEN.value,
693700
'seed': 'abcdef',
694701
'microPercentRange': {
695702
'microPercentLowerBound': 25_000_000,
@@ -750,6 +757,59 @@ def evaluate_random_assignments(self, condition, num_of_assignments, mock_app, d
750757

751758
return eval_true_count
752759

760+
@pytest.mark.parametrize('custom_signal_opearator, target_custom_signal_value, actual_custom_signal_value, parameter_value',
761+
[
762+
SEMENTIC_VERSION_LESS_THAN_TRUE,
763+
SEMANTIC_VERSION_GREATER_THAN_FALSE,
764+
SEMENTIC_VERSION_EQUAL_TRUE,
765+
SEMANTIC_VERSION_INVALID_FORMAT_NEGATIVE_INTEGER,
766+
SEMANTIC_VERSION_INVALID_FORMAT_STRING
767+
])
768+
def test_evaluate_custom_signal_semantic_version(self, custom_signal_opearator,
769+
target_custom_signal_value, actual_custom_signal_value, parameter_value):
770+
app = firebase_admin.get_app()
771+
condition = {
772+
'name': 'is_true',
773+
'condition': {
774+
'orCondition': {
775+
'conditions': [{
776+
'andCondition': {
777+
'conditions': [{
778+
'customSignal': {
779+
'customSignalOperator': custom_signal_opearator,
780+
'customSignalKey': 'sementic_version_key',
781+
'targetCustomSignalValues': target_custom_signal_value
782+
}
783+
}],
784+
}
785+
}]
786+
}
787+
}
788+
}
789+
default_config = {
790+
'dog_is_cute': True
791+
}
792+
template_data = {
793+
'conditions': [condition],
794+
'parameters': {
795+
'is_enabled': {
796+
'defaultValue': {'value': 'false'},
797+
'conditionalValues': {'is_true': {'value': 'true'}}
798+
},
799+
},
800+
'parameterGroups':'',
801+
'version':'',
802+
'etag': '123'
803+
}
804+
context = {'randomization_id': '123', 'sementic_version_key': actual_custom_signal_value}
805+
server_template = remote_config.init_server_template(
806+
app=app,
807+
default_config=default_config,
808+
template_data=ServerTemplateData('etag', template_data)
809+
)
810+
server_config = server_template.evaluate(context)
811+
assert server_config.get_boolean('is_enabled') == parameter_value
812+
753813

754814
class MockAdapter(testutils.MockAdapter):
755815
"""A Mock HTTP Adapter that Firebase Remote Config with ETag in header."""

0 commit comments

Comments
 (0)