From 0801c9cc5fbf6c2ad91ce68e6b7df6605720be59 Mon Sep 17 00:00:00 2001 From: Pijush Chakraborty Date: Thu, 9 Jan 2025 22:23:50 +0530 Subject: [PATCH 1/4] Updating ServerTemplate to accomodate to_json() method --- firebase_admin/remote_config.py | 44 ++++++++++++------ tests/test_remote_config.py | 82 +++++++++++++++++++++++---------- 2 files changed, 88 insertions(+), 38 deletions(-) diff --git a/firebase_admin/remote_config.py b/firebase_admin/remote_config.py index 119f41e3..7a060105 100644 --- a/firebase_admin/remote_config.py +++ b/firebase_admin/remote_config.py @@ -17,6 +17,7 @@ """ import asyncio +import json import logging from typing import Dict, Optional, Literal, Union, Any from enum import Enum @@ -63,13 +64,12 @@ class CustomSignalOperator(Enum): SEMANTIC_VERSION_GREATER_EQUAL = "SEMANTIC_VERSION_GREATER_EQUAL" UNKNOWN = "UNKNOWN" -class ServerTemplateData: +class _ServerTemplateData: """Parses, validates and encapsulates template data and metadata.""" - def __init__(self, etag, template_data): + def __init__(self, template_data): """Initializes a new ServerTemplateData instance. Args: - etag: The string to be used for initialize the ETag property. template_data: The data to be parsed for getting the parameters and conditions. Raises: @@ -96,8 +96,10 @@ def __init__(self, etag, template_data): self._version = template_data['version'] self._etag = '' - if etag is not None and isinstance(etag, str): - self._etag = etag + if 'etag' in template_data and isinstance(template_data['etag'], str): + self._etag = template_data['etag'] + + self._template_data_json = json.dumps(template_data) @property def parameters(self): @@ -115,6 +117,10 @@ def version(self): def conditions(self): return self._conditions + @property + def template_data_json(self): + return self._template_data_json + class ServerTemplate: """Represents a Server Template with implementations for loading and evaluting the template.""" @@ -170,13 +176,21 @@ def evaluate(self, context: Optional[Dict[str, Union[str, int]]] = None) -> 'Ser config_values) return ServerConfig(config_values=self._evaluator.evaluate()) - def set(self, template: ServerTemplateData): + def set(self, template_data_json: str): """Updates the cache to store the given template is of type ServerTemplateData. Args: - template: An object of type ServerTemplateData to be cached. + template_data_json: A json string representing ServerTemplateData to be cached. """ - self._cache = template + template_data_map = json.loads(template_data_json) + self._cache = _ServerTemplateData(template_data_map) + + def to_json(self): + """Provides the server template in a JSON format to be used for initialization later.""" + if not self._cache: + raise ValueError("""No Remote Config Server template in cache. + Call load() before calling toJSON().""") + return self._cache.template_data_json class ServerConfig: @@ -196,6 +210,9 @@ def get_int(self, key): def get_float(self, key): return self._get_value(key).as_float() + def get_value_source(self, key): + return self._get_value(key).get_source() + def _get_value(self, key): return self._config_values.get(key, _Value('static')) @@ -233,7 +250,8 @@ async def get_server_template(self): except requests.exceptions.RequestException as error: raise self._handle_remote_config_error(error) else: - return ServerTemplateData(headers.get('etag'), template_data) + template_data['etag'] = headers.get('etag') + return _ServerTemplateData(template_data) def _get_url(self): """Returns project prefix for url, in the format of /v1/projects/${projectId}""" @@ -633,22 +651,22 @@ async def get_server_template(app: App = None, default_config: Optional[Dict[str return template def init_server_template(app: App = None, default_config: Optional[Dict[str, str]] = None, - template_data: Optional[ServerTemplateData] = None): + template_data_json: Optional[str] = None): """Initializes a new ServerTemplate instance. Args: app: App instance to be used. This is optional and the default app instance will be used if not present. default_config: The default config to be used in the evaluated config. - template_data: An optional template data to be set on initialization. + template_data_json: An optional template data JSON to be set on initialization. Returns: ServerTemplate: A new ServerTemplate instance initialized with an optional template and config. """ template = ServerTemplate(app=app, default_config=default_config) - if template_data is not None: - template.set(template_data) + if template_data_json is not None: + template.set(template_data_json) return template class _Value: diff --git a/tests/test_remote_config.py b/tests/test_remote_config.py index 2eabe15e..842648dc 100644 --- a/tests/test_remote_config.py +++ b/tests/test_remote_config.py @@ -21,8 +21,7 @@ CustomSignalOperator, PercentConditionOperator, _REMOTE_CONFIG_ATTRIBUTE, - _RemoteConfigService, - ServerTemplateData) + _RemoteConfigService) from firebase_admin import remote_config, _utils from tests import testutils @@ -121,12 +120,12 @@ def test_evaluate_or_and_true_condition_true(self): }, 'parameterGroups': '', 'version': '', - 'etag': '123' + 'etag': 'etag' } server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() @@ -165,12 +164,12 @@ def test_evaluate_or_and_false_condition_false(self): }, 'parameterGroups': '', 'version': '', - 'etag': '123' + 'etag': 'etag' } server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() @@ -196,12 +195,12 @@ def test_evaluate_non_or_condition(self): }, 'parameterGroups': '', 'version': '', - 'etag': '123' + 'etag': 'etag' } server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() @@ -262,12 +261,12 @@ def test_evaluate_return_conditional_values_honor_order(self): }, 'parameterGroups':'', 'version':'', - 'etag': '123' + 'etag': 'etag' } server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_string('dog_type') == 'corgi' @@ -280,7 +279,7 @@ def test_evaluate_default_when_no_param(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_boolean('promo_enabled') == default_config.get('promo_enabled') @@ -296,7 +295,7 @@ def test_evaluate_default_when_no_default_value(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_string('default_value') == default_config.get('default_value') @@ -313,7 +312,7 @@ def test_evaluate_default_when_in_default(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_string('inapp_default') == default_config.get('inapp_default') @@ -328,7 +327,7 @@ def test_evaluate_default_when_defined(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_string('dog_type') == 'shiba' @@ -342,7 +341,7 @@ def test_evaluate_return_numeric_value(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_int('dog_age') == int(default_config.get('dog_age')) @@ -356,7 +355,7 @@ def test_evaluate_return_boolean_value(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate() assert server_config.get_boolean('dog_is_cute') @@ -398,7 +397,7 @@ def test_evaluate_unknown_operator_to_false(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert not server_config.get_boolean('is_enabled') @@ -442,7 +441,7 @@ def test_evaluate_less_or_equal_to_max_to_true(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert server_config.get_boolean('is_enabled') @@ -485,7 +484,7 @@ def test_evaluate_undefined_micropercent_to_false(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert not server_config.get_boolean('is_enabled') @@ -528,7 +527,7 @@ def test_evaluate_undefined_micropercentrange_to_false(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert not server_config.get_boolean('is_enabled') @@ -575,7 +574,7 @@ def test_evaluate_between_min_max_to_true(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert server_config.get_boolean('is_enabled') @@ -622,7 +621,7 @@ def test_evaluate_between_equal_bounds_to_false(self): server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert not server_config.get_boolean('is_enabled') @@ -750,7 +749,7 @@ def evaluate_random_assignments(self, condition, num_of_assignments, mock_app, d server_template = remote_config.init_server_template( app=mock_app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) for _ in range(num_of_assignments): @@ -814,7 +813,7 @@ def test_evaluate_custom_signal_semantic_version(self, server_template = remote_config.init_server_template( app=app, default_config=default_config, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) server_config = server_template.evaluate(context) assert server_config.get_boolean('is_enabled') == parameter_value @@ -917,7 +916,7 @@ def test_init_server_template(self): template = remote_config.init_server_template( app=app, default_config={'default_test': 'default_value'}, - template_data=ServerTemplateData('etag', template_data) + template_data_json=json.dumps(template_data) ) config = template.evaluate() @@ -949,3 +948,36 @@ async def test_get_server_template(self): config = template.evaluate() assert config.get_string('test_key') == 'test_value' + + @pytest.mark.asyncio + async def test_server_template_to_json(self): + app = firebase_admin.get_app() + rc_instance = _utils.get_app_service(app, + _REMOTE_CONFIG_ATTRIBUTE, _RemoteConfigService) + + recorder = [] + response = json.dumps({ + 'parameters': { + 'test_key': { + 'defaultValue': {'value': 'test_value'}, + 'conditionalValues': {} + } + }, + 'conditions': [], + 'version': 'test' + }) + + expected_template_json = '{"parameters": {' \ + '"test_key": {' \ + '"defaultValue": {' \ + '"value": "test_value"}, ' \ + '"conditionalValues": {}}}, "conditions": [], ' \ + '"version": "test", "etag": "etag"}' + + rc_instance._client.session.mount( + 'https://firebaseremoteconfig.googleapis.com', + MockAdapter(response, 200, recorder)) + template = await remote_config.get_server_template(app=app) + + template_json = template.to_json() + assert template_json == expected_template_json From b6766d834147da458a927a92dc58e17927234aca Mon Sep 17 00:00:00 2001 From: Pijush Chakraborty Date: Thu, 9 Jan 2025 22:34:37 +0530 Subject: [PATCH 2/4] Updating unit tests --- tests/test_remote_config.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/test_remote_config.py b/tests/test_remote_config.py index 842648dc..8c6248e1 100644 --- a/tests/test_remote_config.py +++ b/tests/test_remote_config.py @@ -130,6 +130,7 @@ def test_evaluate_or_and_true_condition_true(self): server_config = server_template.evaluate() assert server_config.get_boolean('is_enabled') + assert server_config.get_value_source('is_enabled') == 'remote' def test_evaluate_or_and_false_condition_false(self): app = firebase_admin.get_app() From 67ef90929e0380151c5c07e30f8c387f5ed5503d Mon Sep 17 00:00:00 2001 From: Pijush Chakraborty Date: Thu, 9 Jan 2025 22:43:51 +0530 Subject: [PATCH 3/4] Updating docstrings --- firebase_admin/remote_config.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/firebase_admin/remote_config.py b/firebase_admin/remote_config.py index 7a060105..e5b18986 100644 --- a/firebase_admin/remote_config.py +++ b/firebase_admin/remote_config.py @@ -199,18 +199,23 @@ def __init__(self, config_values): self._config_values = config_values # dictionary of param key to values def get_boolean(self, key): + """Returns the value as a boolean.""" return self._get_value(key).as_boolean() def get_string(self, key): + """Returns the value as a string.""" return self._get_value(key).as_string() def get_int(self, key): + """Returns the value as an integer.""" return self._get_value(key).as_int() def get_float(self, key): + """Returns the value as a float.""" return self._get_value(key).as_float() def get_value_source(self, key): + """Returns the source of the value.""" return self._get_value(key).get_source() def _get_value(self, key): From 65c3d55515615ac38089197e231f084705d00810 Mon Sep 17 00:00:00 2001 From: Pijush Chakraborty Date: Wed, 15 Jan 2025 12:57:43 +0530 Subject: [PATCH 4/4] Adding re-entrant lock to make template cache updates/reads atomic --- firebase_admin/remote_config.py | 24 +++++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/firebase_admin/remote_config.py b/firebase_admin/remote_config.py index e5b18986..4b5d4611 100644 --- a/firebase_admin/remote_config.py +++ b/firebase_admin/remote_config.py @@ -19,6 +19,7 @@ import asyncio import json import logging +import threading from typing import Dict, Optional, Literal, Union, Any from enum import Enum import re @@ -138,6 +139,7 @@ def __init__(self, app: App = None, default_config: Optional[Dict[str, str]] = N # fetched from RC servers via the load API, or via the set API. self._cache = None self._stringified_default_config: Dict[str, str] = {} + self._lock = threading.RLock() # RC stores all remote values as string, but it's more intuitive # to declare default values with specific types, so this converts @@ -148,7 +150,9 @@ def __init__(self, app: App = None, default_config: Optional[Dict[str, str]] = N async def load(self): """Fetches the server template and caches the data.""" - self._cache = await self._rc_service.get_server_template() + rc_server_template = await self._rc_service.get_server_template() + with self._lock: + self._cache = rc_server_template def evaluate(self, context: Optional[Dict[str, Union[str, int]]] = None) -> 'ServerConfig': """Evaluates the cached server template to produce a ServerConfig. @@ -167,12 +171,17 @@ def evaluate(self, context: Optional[Dict[str, Union[str, int]]] = None) -> 'Ser Call load() before calling evaluate().""") context = context or {} config_values = {} + + with self._lock: + template_conditions = self._cache.conditions + template_parameters = self._cache.parameters + # Initializes config Value objects with default values. if self._stringified_default_config is not None: for key, value in self._stringified_default_config.items(): config_values[key] = _Value('default', value) - self._evaluator = _ConditionEvaluator(self._cache.conditions, - self._cache.parameters, context, + self._evaluator = _ConditionEvaluator(template_conditions, + template_parameters, context, config_values) return ServerConfig(config_values=self._evaluator.evaluate()) @@ -183,14 +192,19 @@ def set(self, template_data_json: str): template_data_json: A json string representing ServerTemplateData to be cached. """ template_data_map = json.loads(template_data_json) - self._cache = _ServerTemplateData(template_data_map) + template_data = _ServerTemplateData(template_data_map) + + with self._lock: + self._cache = template_data def to_json(self): """Provides the server template in a JSON format to be used for initialization later.""" if not self._cache: raise ValueError("""No Remote Config Server template in cache. Call load() before calling toJSON().""") - return self._cache.template_data_json + with self._lock: + template_json = self._cache.template_data_json + return template_json class ServerConfig: