diff --git a/firebase_admin/remote_config.py b/firebase_admin/remote_config.py index 880804d3..ef23bfb9 100644 --- a/firebase_admin/remote_config.py +++ b/firebase_admin/remote_config.py @@ -315,7 +315,9 @@ def evaluate(self): if parameter_value_wrapper: parameter_value = parameter_value_wrapper.get('value') - self._config_values[key] = _Value('remote', parameter_value) + # A conditional value without an explicit value does not override anything. + if parameter_value is not None: + self._config_values[key] = _Value('remote', parameter_value) continue if not default_value: @@ -487,20 +489,19 @@ 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('customSignalOperator') or {} - custom_signal_key = custom_signal_condition.get('customSignalKey') or {} - target_custom_signal_values = ( - custom_signal_condition.get('targetCustomSignalValues') or {}) + custom_signal_operator = custom_signal_condition.get('customSignalOperator') + custom_signal_key = custom_signal_condition.get('customSignalKey') + target_custom_signal_values = custom_signal_condition.get('targetCustomSignalValues') 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.") return False - if not target_custom_signal_values: - return False - actual_custom_signal_value = context.get(custom_signal_key) or {} + # A signal is only absent when the context has no entry for it. Values such as 0 or '' + # are valid signals (the context is typed as Union[str, int]) and must be compared. + actual_custom_signal_value = context.get(custom_signal_key) - if not actual_custom_signal_value: + if actual_custom_signal_value is None: logger.debug("Custom signal value not found in context: %s", custom_signal_key) return False @@ -725,7 +726,8 @@ def __init__(self, source: ValueSource, value: str = DEFAULT_VALUE_FOR_STRING): value: The string value. """ self.source = source - self.value = value + # The getters assume a string payload, so a missing value must not end up as None. + self.value = value if value is not None else self.DEFAULT_VALUE_FOR_STRING def as_string(self) -> str: """Returns the value as a string.""" @@ -739,7 +741,7 @@ def as_boolean(self) -> bool: return self.DEFAULT_VALUE_FOR_BOOLEAN return str(self.value).lower() in self.BOOLEAN_TRUTHY_VALUES - def as_int(self) -> float: + def as_int(self) -> int: """Returns the value as a number.""" if self.source == 'static': return self.DEFAULT_VALUE_FOR_INTEGER diff --git a/tests/test_remote_config.py b/tests/test_remote_config.py index 7bbf9721..85641690 100644 --- a/tests/test_remote_config.py +++ b/tests/test_remote_config.py @@ -301,6 +301,40 @@ def test_evaluate_default_when_no_default_value(self): server_config = server_template.evaluate() assert server_config.get_string('default_value') == default_config.get('default_value') + def test_evaluate_conditional_value_without_explicit_value(self): + app = firebase_admin.get_app() + default_config = {'dog_type': 'shiba'} + template_data = SERVER_REMOTE_CONFIG_RESPONSE + template_data['conditions'] = [{'name': 'is_true', 'condition': {'true': {}}}] + template_data['parameters'] = { + 'dog_type': { + 'conditionalValues': {'is_true': {'useInAppDefault': False}} + } + } + server_template = remote_config.init_server_template( + app=app, + default_config=default_config, + template_data_json=json.dumps(template_data) + ) + server_config = server_template.evaluate() + assert server_config.get_string('dog_type') == default_config.get('dog_type') + + def test_evaluate_default_value_without_explicit_value(self): + app = firebase_admin.get_app() + default_config = {'dog_type': 'shiba'} + template_data = SERVER_REMOTE_CONFIG_RESPONSE + template_data['parameters'] = { + 'dog_type': {'defaultValue': {'useInAppDefault': False}} + } + server_template = remote_config.init_server_template( + app=app, + default_config=default_config, + template_data_json=json.dumps(template_data) + ) + server_config = server_template.evaluate() + assert server_config.get_string('dog_type') == '' + assert server_config.get_int('dog_type') == 0 + def test_evaluate_default_when_in_default(self): app = firebase_admin.get_app() template_data = SERVER_REMOTE_CONFIG_RESPONSE @@ -819,6 +853,58 @@ def test_evaluate_custom_signal_semantic_version(self, server_config = server_template.evaluate(context) assert server_config.get_boolean('is_enabled') == parameter_value + @pytest.mark.parametrize( + 'custom_signal_operator, target_custom_signal_values, context_value', + [ + (CustomSignalOperator.STRING_CONTAINS.value, ['0'], 0), + (CustomSignalOperator.NUMERIC_EQUAL.value, ['0'], 0), + (CustomSignalOperator.NUMERIC_GREATER_EQUAL.value, ['0'], 0), + (CustomSignalOperator.SEMANTIC_VERSION_EQUAL.value, ['0.0.0'], 0), + (CustomSignalOperator.STRING_EXACTLY_MATCHES.value, [''], ''), + ]) + def test_evaluate_custom_signal_with_falsy_value(self, + custom_signal_operator, + target_custom_signal_values, + context_value): + app = firebase_admin.get_app() + condition = { + 'name': 'is_true', + 'condition': { + 'orCondition': { + 'conditions': [{ + 'andCondition': { + 'conditions': [{ + 'customSignal': { + 'customSignalOperator': custom_signal_operator, + 'customSignalKey': 'signal_key', + 'targetCustomSignalValues': target_custom_signal_values + } + }], + } + }] + } + } + } + template_data = { + 'conditions': [condition], + 'parameters': { + 'is_enabled': { + 'defaultValue': {'value': 'false'}, + 'conditionalValues': {'is_true': {'value': 'true'}} + }, + }, + 'parameterGroups': '', + 'version': '', + 'etag': '123' + } + server_template = remote_config.init_server_template( + app=app, + default_config={}, + template_data_json=json.dumps(template_data) + ) + server_config = server_template.evaluate({'signal_key': context_value}) + assert server_config.get_boolean('is_enabled') + class MockAdapter(testutils.MockAdapter): """A Mock HTTP Adapter that provides Firebase Remote Config responses with ETag in header."""