Fixing SSRC Typos and Minor Bugs (#841)

* Changes for percent comparison

* Fixing semantic version issues with invalid version

* Fixing Config values must retrun default values from invalid get operations

* Updating tolerance for percentage evaluation

* Removing dependency changes from fix branch

* Updating ServerConfig methods as per review changes

* Updating comments and vars for readability

* Added unit and integration tests

* Refactor and add unit test

* Implementation for Fetching and Caching Server Side Remote Config (#825)

* Minor update to API signature

* Updating init params for ServerTemplateData

* Adding validation errors and test

* Removing parameter groups

* Addressing PR comments and fixing async flow during fetch call

* Fixing lint issues

---------

Co-authored-by: Jonathan Edey <[email protected]>
Co-authored-by: Lahiru Maramba <[email protected]>
Co-authored-by: Pijush Chakraborty <[email protected]>
Co-authored-by: varun rathore <[email protected]>
Co-authored-by: Varun Rathore <[email protected]>

Author: 6 people

firebase_admin/remote_config.py

@@ -1,4 +1,4 @@
-# Copyright 2017 Google Inc.
+# Copyright 2024 Google Inc.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -185,21 +185,19 @@ def __init__(self, config_values):
         self._config_values = config_values # dictionary of param key to values
 
     def get_boolean(self, key):
-        return self.get_value(key).as_boolean()
+        return self._get_value(key).as_boolean()
 
     def get_string(self, key):
-        return self.get_value(key).as_string()
+        return self._get_value(key).as_string()
 
     def get_int(self, key):
-        return self.get_value(key).as_int()
+        return self._get_value(key).as_int()
 
     def get_float(self, key):
-        return self.get_value(key).as_float()
+        return self._get_value(key).as_float()
 
-    def get_value(self, key):
-        if self._config_values[key]:
-            return self._config_values[key]
-        return _Value('static')
+    def _get_value(self, key):
+        return self._config_values.get(key, _Value('static'))
 
 
 class _RemoteConfigService:
@@ -421,11 +419,11 @@ def evaluate_percent_condition(self, percent_condition,
 
         hash64 = self.hash_seeded_randomization_id(string_to_hash)
         instance_micro_percentile = hash64 % (100 * 1000000)
-        if percent_operator == PercentConditionOperator.LESS_OR_EQUAL:
+        if percent_operator == PercentConditionOperator.LESS_OR_EQUAL.value:
             return instance_micro_percentile <= norm_micro_percent
-        if percent_operator == PercentConditionOperator.GREATER_THAN:
+        if percent_operator == PercentConditionOperator.GREATER_THAN.value:
             return instance_micro_percentile > norm_micro_percent
-        if percent_operator == PercentConditionOperator.BETWEEN:
+        if percent_operator == PercentConditionOperator.BETWEEN.value:
             return norm_percent_lower_bound < instance_micro_percentile <= norm_percent_upper_bound
         logger.warning("Unknown percent operator: %s", percent_operator)
         return False
@@ -454,10 +452,10 @@ def evaluate_custom_signal_condition(self, custom_signal_condition,
         Returns:
           True if the condition is met, False otherwise.
         """
-        custom_signal_operator = custom_signal_condition.get('custom_signal_operator') or {}
-        custom_signal_key = custom_signal_condition.get('custom_signal_key') or {}
+        custom_signal_operator = custom_signal_condition.get('customSignalOperator') or {}
+        custom_signal_key = custom_signal_condition.get('customSignalKey') or {}
         target_custom_signal_values = (
-            custom_signal_condition.get('target_custom_signal_values') or {})
+            custom_signal_condition.get('targetCustomSignalValues') or {})
 
         if not all([custom_signal_operator, custom_signal_key, target_custom_signal_values]):
             logger.warning("Missing operator, key, or target values for custom signal condition.")
@@ -471,71 +469,71 @@ def evaluate_custom_signal_condition(self, custom_signal_condition,
             logger.warning("Custom signal value not found in context: %s", custom_signal_key)
             return False
 
-        if custom_signal_operator == CustomSignalOperator.STRING_CONTAINS:
+        if custom_signal_operator == CustomSignalOperator.STRING_CONTAINS.value:
             return self._compare_strings(target_custom_signal_values,
                                          actual_custom_signal_value,
                                          lambda target, actual: target in actual)
-        if custom_signal_operator == CustomSignalOperator.STRING_DOES_NOT_CONTAIN:
+        if custom_signal_operator == CustomSignalOperator.STRING_DOES_NOT_CONTAIN.value:
             return not self._compare_strings(target_custom_signal_values,
                                              actual_custom_signal_value,
                                              lambda target, actual: target in actual)
-        if custom_signal_operator == CustomSignalOperator.STRING_EXACTLY_MATCHES:
+        if custom_signal_operator == CustomSignalOperator.STRING_EXACTLY_MATCHES.value:
             return self._compare_strings(target_custom_signal_values,
                                          actual_custom_signal_value,
                                          lambda target, actual: target.strip() == actual.strip())
-        if custom_signal_operator == CustomSignalOperator.STRING_CONTAINS_REGEX:
+        if custom_signal_operator == CustomSignalOperator.STRING_CONTAINS_REGEX.value:
             return self._compare_strings(target_custom_signal_values,
                                          actual_custom_signal_value,
                                          re.search)
 
         # For numeric operators only one target value is allowed.
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_LESS_THAN:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_LESS_THAN.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r < 0)
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_LESS_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_LESS_EQUAL.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r <= 0)
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_EQUAL.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r == 0)
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_NOT_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_NOT_EQUAL.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r != 0)
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_GREATER_THAN:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_GREATER_THAN.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r > 0)
-        if custom_signal_operator == CustomSignalOperator.NUMERIC_GREATER_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.NUMERIC_GREATER_EQUAL.value:
             return self._compare_numbers(target_custom_signal_values[0],
                                          actual_custom_signal_value,
                                          lambda r: r >= 0)
 
         # For semantic operators only one target value is allowed.
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_LESS_THAN.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r < 0)
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_LESS_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_LESS_EQUAL.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r <= 0)
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_EQUAL.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r == 0)
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_NOT_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_NOT_EQUAL.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r != 0)
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_GREATER_THAN:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_GREATER_THAN.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r > 0)
-        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_GREATER_EQUAL:
+        if custom_signal_operator == CustomSignalOperator.SEMANTIC_VERSION_GREATER_EQUAL.value:
             return self._compare_semantic_versions(target_custom_signal_values[0],
                                                    actual_custom_signal_value,
                                                    lambda r: r >= 0)
@@ -589,25 +587,27 @@ def _compare_semantic_versions(self, target_value, actual_value, predicate_fn) -
         return self._compare_versions(str(actual_value),
                                       str(target_value), predicate_fn)
 
-    def _compare_versions(self, version1, version2, predicate_fn) -> bool:
+    def _compare_versions(self, sem_version_1, sem_version_2, predicate_fn) -> bool:
         """Compares two semantic version strings.
 
         Args:
-        version1: The first semantic version string.
-        version2: The second semantic version string.
-        predicate_fn: A function that takes an integer and returns a boolean.
+            sem_version_1: The first semantic version string.
+            sem_version_2: The second semantic version string.
+            predicate_fn: A function that takes an integer and returns a boolean.
 
         Returns:
             bool: The result of the predicate function.
         """
         try:
-            v1_parts = [int(part) for part in version1.split('.')]
-            v2_parts = [int(part) for part in version2.split('.')]
+            v1_parts = [int(part) for part in sem_version_1.split('.')]
+            v2_parts = [int(part) for part in sem_version_2.split('.')]
             max_length = max(len(v1_parts), len(v2_parts))
             v1_parts.extend([0] * (max_length - len(v1_parts)))
             v2_parts.extend([0] * (max_length - len(v2_parts)))
 
             for part1, part2 in zip(v1_parts, v2_parts):
+                if any((part1 < 0, part2 < 0)):
+                    raise ValueError
                 if part1 < part2:
                     return predicate_fn(-1)
                 if part1 > part2:
@@ -674,7 +674,7 @@ def as_string(self) -> str:
         """Returns the value as a string."""
         if self.source == 'static':
             return self.DEFAULT_VALUE_FOR_STRING
-        return self.value
+        return str(self.value)
 
     def as_boolean(self) -> bool:
         """Returns the value as a boolean."""
@@ -686,13 +686,19 @@ def as_int(self) -> float:
         """Returns the value as a number."""
         if self.source == 'static':
             return self.DEFAULT_VALUE_FOR_INTEGER
-        return self.value
+        try:
+            return int(self.value)
+        except ValueError:
+            return self.DEFAULT_VALUE_FOR_INTEGER
 
     def as_float(self) -> float:
         """Returns the value as a number."""
         if self.source == 'static':
             return self.DEFAULT_VALUE_FOR_FLOAT_NUMBER
-        return float(self.value)
+        try:
+            return float(self.value)
+        except ValueError:
+            return self.DEFAULT_VALUE_FOR_FLOAT_NUMBER
 
     def get_source(self) -> ValueSource:
         """Returns the source of the value."""