From 950f37d4d024645725ff28fb6a4746a4eee66592 Mon Sep 17 00:00:00 2001 From: Minseul Kim Date: Fri, 26 Jun 2026 13:21:53 -0700 Subject: [PATCH 1/6] feat(fdc): Added unit tests for Data Connect client factory and _DataConnectService --- tests/test_data_connect.py | 287 +++++++++++++++++++++++++++++++++++++ 1 file changed, 287 insertions(+) create mode 100644 tests/test_data_connect.py diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py new file mode 100644 index 00000000..b20154aa --- /dev/null +++ b/tests/test_data_connect.py @@ -0,0 +1,287 @@ +import pytest + +import firebase_admin +from firebase_admin import _utils +from firebase_admin import dataconnect +from firebase_admin import credentials +from tests import testutils +from unittest import mock + +class TestConnectorConfig: + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_connector_config_initialization(self): + config = dataconnect.ConnectorConfig( + service_id="starterproject", + location = "us-east4", + connector="my_connector" + ) + assert config.service_id=="starterproject" + assert config.location=="us-east4" + assert config.connector=="my_connector" + + def test_connector_config_is_frozen(self): + config = dataconnect.ConnectorConfig( + service_id="starterproject", + location = "us-east4", + connector="my_connector" + ) + + with pytest.raises(AttributeError, match="cannot assign to field 'service_id'"): + config.service_id = "changed_id" + with pytest.raises(AttributeError, match="cannot assign to field 'location'"): + config.location = "us-central1" + with pytest.raises(AttributeError, match="cannot assign to field 'connector'"): + config.connector = "changed_connector" + + def testing_connector_config_string_written(self): + config = dataconnect.ConnectorConfig( + service_id="starterproject", + location = "us-east4", + connector="my_connector" + ) + repr_str=repr(config) + assert "service_id='starterproject'" in repr_str + assert "location='us-east4'" in repr_str + assert "connector='my_connector'" in repr_str + + def test_connector_config_empty_strings(self): + with pytest.raises(ValueError, match="service_id cannot be empty"): + dataconnect.ConnectorConfig(service_id="", location="us-east4", connector="my_connector") + + with pytest.raises(ValueError, match="location cannot be empty"): + dataconnect.ConnectorConfig(service_id="starterproject", location="", connector="my_connector") + + with pytest.raises(ValueError, match="connector cannot be empty"): + dataconnect.ConnectorConfig(service_id="starterproject", location="us-east4", connector="") + + def test_connector_config_invalid_types(self): + with pytest.raises(ValueError, match="service_id cannot be empty"): + dataconnect.ConnectorConfig(service_id=None, location="us-east4", connector="my_connector") + with pytest.raises(ValueError, match="location cannot be empty"): + dataconnect.ConnectorConfig(service_id="starterproject", location=123, connector="my_connector") + +class TestDataConnect: + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_init_property_assignment(self): + cred = testutils.MockCredential() + try: + app = firebase_admin.initialize_app(cred, name = "starter_app") + except Exception: + pytest.fail("initialize app has an error") + + config = dataconnect.ConnectorConfig(service_id="starterproject", location = "us-east4", connector="my_connector") + + try: + data_connect_instance = dataconnect.DataConnect(app, config) + except Exception: + pytest.fail("DataConnect initialization failed.") + + assert data_connect_instance._app is app + assert data_connect_instance._config is config + assert data_connect_instance.app is app + assert data_connect_instance.config is config + + assert data_connect_instance._app.name == "starter_app" + assert data_connect_instance._config.service_id == "starterproject" + +class TestDataConnectClientFactory: + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def setup_method(self): + self.cred = testutils.MockCredential() + self.app = firebase_admin.initialize_app(self.cred, name = 'starter_app') + self.config1 = dataconnect.ConnectorConfig(service_id='starterproject', location='us-east3', connector='my_connector') + self.config2 = dataconnect.ConnectorConfig(service_id='starterproject2', location='us-east4', connector='my_connector2') + + @mock.patch('firebase_admin.dataconnect._DataConnectService.get_client', wraps=dataconnect._DataConnectService.get_client) + def test_client_successful(self, mock_get_client): + client_instance = dataconnect.client(self.config1, app=self.app) + mock_get_client.assert_called_once_with(self.config1) + assert isinstance(client_instance, dataconnect.DataConnect) + assert client_instance.config is self.config1 + assert client_instance.app is self.app + + def test_client_retrieval_diff_configs(self): + client1 = dataconnect.client(self.config1, app=self.app) + client2 = dataconnect.client(self.config2, app=self.app) + + assert client1 is not client2 + assert client1.config is self.config1 + assert client2.config is self.config2 + assert client1.app is self.app + assert client2.app is self.app + + def test_client_retrieval_same_config_cached(self): + client1 = dataconnect.client(self.config1, app=self.app) + client2 = dataconnect.client(self.config1, app=self.app) + + assert client1 is client2 + + def test_client_retrieval_different_apps_same_config(self): + app2 = firebase_admin.initialize_app(self.cred, name='app2') + + client1 = dataconnect.client(self.config1, app=self.app) + client2 = dataconnect.client(self.config1, app=app2) + + assert client1 is not client2 + assert client1.app is self.app + assert client1.app is not client2.app + + def test_invalid_config_type(self): + with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): + dataconnect.client('not-a-config', app=self.app) + + def test_invalid_app_type(self): + with pytest.raises(ValueError, match="App must be of type firebase_admin.App"): + dataconnect.client(self.config1, 'not-a-app') + + def test_client_default_app(self): + default_app = firebase_admin.initialize_app(self.cred) + client_instance = dataconnect.client(self.config1) + assert client_instance.app is default_app + + def test_client_none_config(self): + with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): + dataconnect.client(None, app=self.app) + +class TestDataConnectService: + def setup_method(self): + self.cred = testutils.MockCredential() + self.app = firebase_admin.initialize_app(self.cred, name = 'starter_app') + self.service = dataconnect._DataConnectService(self.app) + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_cache_hit(self): + config = dataconnect.ConnectorConfig('s1', 'l1', 'c1') + client1 = self.service.get_client(config) + client2 = self.service.get_client(config) + assert client1 is client2 + + assert isinstance(client1, dataconnect.DataConnect) + assert client1.config is config + + def test_cache_miss_on_different_config(self): + config1 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') + config2 = dataconnect.ConnectorConfig('s2', 'l2', 'c2') + client1 = self.service.get_client(config1) + client2 = self.service.get_client(config2) + assert client1 is not client2 + + @pytest.mark.parametrize("config_a, config_b, expect_same", [ + (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l', 'c_diff'), False), + (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l_diff', 'c'), False), + (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s_diff', 'l', 'c'), False), + (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l', 'c'), True), + ]) + def test_complex_cache_key(self, config_a, config_b, expect_same): + client_a = self.service.get_client(config_a) + client_b = self.service.get_client(config_b) + if expect_same: + assert client_a is client_b + else: + assert client_a is not client_b + + def test_config_equivalence(self): + config1 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') + config2 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') + client1 = self.service.get_client(config1) + client2 = self.service.get_client(config2) + assert client1 is client2 + + @mock.patch('firebase_admin.dataconnect.DataConnect', autospec=True) + def test_client_creation_mocking(self, MockDataConnect): + config1 = dataconnect.ConnectorConfig('s_mock', 'l_mock', 'c_mock1') + config2 = dataconnect.ConnectorConfig('s_mock', 'l_mock', 'c_mock2') + + self.service.get_client(config1) + MockDataConnect.assert_called_once_with(app=self.app, config=config1) + + MockDataConnect.reset_mock() + + self.service.get_client(config1) + MockDataConnect.assert_not_called() + + MockDataConnect.reset_mock() + + # first call using config2 + self.service.get_client(config2) + MockDataConnect.assert_called_once_with(app=self.app, config=config2) + + @mock.patch('firebase_admin.dataconnect.DataConnect', autospec=True) + def test_error_handling_in_creation(self, MockDataConnect): + config = dataconnect.ConnectorConfig('s_err', 'l_err', 'c_err') + test_error = RuntimeError("Failed to create client") + MockDataConnect.side_effect = test_error + + with pytest.raises(RuntimeError, match="Failed to create client"): + self.service.get_client(config) + + # Ensure the failed creation wasn't cached + MockDataConnect.side_effect = None + self.service.get_client(config) + assert MockDataConnect.call_count == 2 + + def test_invalid_config_in_service(self): + with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): + self.service.get_client(None) + +class TestDataConnectServiceIntegration: + def setup_method(self): + self.cred = testutils.MockCredential() + self.app1 = firebase_admin.initialize_app(self.cred, name='integ_app1') + self.app2 = firebase_admin.initialize_app(self.cred, name='integ_app2') + + self.config1 = dataconnect.ConnectorConfig( service_id='service1', location='us-central1', connector='conn1') + self.config2 = dataconnect.ConnectorConfig(service_id='service2', location='us-east4', connector='conn2') + self.config1_copy = dataconnect.ConnectorConfig(service_id='service1', location='us-central1', connector='conn1') + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_overall_client_retrieval_and_caching(self): + client1a = dataconnect.client(self.config1, app=self.app1) + client1b = dataconnect.client(self.config1_copy, app=self.app1) + client2 = dataconnect.client(self.config2, app=self.app1) + + assert isinstance(client1a, dataconnect.DataConnect), "Client should be a DataConnect instance" + assert client1a.app is self.app1, "Client should be associated with app1" + assert client1a.config is self.config1, "Client should hold the specific config1" + + # Same config + assert client1b is client1a, "Client should be cached for an equivalent config on the same app" + + # Different config + assert isinstance(client2, dataconnect.DataConnect), "Client should be a DataConnect instance" + assert client2.app is self.app1, "Client should be associated with app1" + assert client2.config is self.config2, "Client should hold the specific config2" + assert client2 is not client1a, "Clients with different configs should be different instances" + + # Different app + client1_app2 = dataconnect.client(self.config1, app=self.app2) + + assert isinstance(client1_app2, dataconnect.DataConnect), "Client on app2 should be a DataConnect instance" + assert client1_app2.app is self.app2, "Client should be associated with app2" + assert client1_app2.config is self.config1, "Client on app2 should hold config1" + assert client1_app2 is not client1a, "Clients on different apps should be different instances" + + @mock.patch.object(_utils, 'get_app_service', wraps=_utils.get_app_service) + def test_uses_app_service_mechanism(self, mock_get_app_service): + """Ensures dataconnect.client uses the standard app service loader.""" + dataconnect.client(self.config1, app=self.app1) + mock_get_app_service.assert_called_once() + args, _ = mock_get_app_service.call_args + assert args[0] is self.app1 + assert args[1] == '_data_connect_service' + assert args[2] == dataconnect._DataConnectService From 23ca2b0851ce5ab6d02415afffa287fed151a978 Mon Sep 17 00:00:00 2001 From: Minseul Kim Date: Fri, 26 Jun 2026 13:39:55 -0700 Subject: [PATCH 2/6] Testing ConnectorConfig, client function, and service --- firebase_admin/dataconnect.py | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) create mode 100644 firebase_admin/dataconnect.py diff --git a/firebase_admin/dataconnect.py b/firebase_admin/dataconnect.py new file mode 100644 index 00000000..66a208fb --- /dev/null +++ b/firebase_admin/dataconnect.py @@ -0,0 +1,34 @@ +from dataclasses import dataclass +from firebase_admin import App +from typing import Any, Dict, Generic, Optional, TypeVar, Union + +@dataclass(frozen=True) +class ConnectorConfig: + """ + Attributes: + service_id (str): The Google Cloud project ID of the service. + location (str): The region of the service. + connector (str): The name of the connector. + """ + + service_id: str + location: str + connector: str + + def __post_init__(self): + if not self.service_id: + raise ValueError("service_id cannot be empty") + if not self.location: + raise ValueError("location cannot be empty") + if not self.connector: + raise ValueError("connector cannot be empty") + + +class DataConnect: + def __init__(self, app: App, config: ConnectorConfig) -> None: + """Initializes a DataConnect client instance""" + self._app: App = app + self._config = config + +def client(config: ConnectorConfig, app: Optional[App] = None) -> DataConnect: + """Returns a DataConnect client for the specified configuration""" \ No newline at end of file From 84e097e32df7693ea07e034a769dea9a9ba3882d Mon Sep 17 00:00:00 2001 From: mk2023 Date: Mon, 29 Jun 2026 15:32:43 -0700 Subject: [PATCH 3/6] refactor(dataconnect): Addressed code review feedback and standardized docstrings/synatx Refactored test suite to use module-level BASE_CONFIG and added parameterized client caching tests. --- firebase_admin/dataconnect.py | 64 +++++++++++++++++++++--- tests/test_data_connect.py | 94 +++++++++++++++-------------------- 2 files changed, 98 insertions(+), 60 deletions(-) diff --git a/firebase_admin/dataconnect.py b/firebase_admin/dataconnect.py index 66a208fb..3bcb20b4 100644 --- a/firebase_admin/dataconnect.py +++ b/firebase_admin/dataconnect.py @@ -1,14 +1,21 @@ from dataclasses import dataclass -from firebase_admin import App -from typing import Any, Dict, Generic, Optional, TypeVar, Union +from typing import Dict, Optional + +from firebase_admin import _utils, App + +__all__ = ['ConnectorConfig', 'DataConnect', 'client'] + +_DATA_CONNECT_ATTRIBUTE = '_data_connect_service' + @dataclass(frozen=True) class ConnectorConfig: - """ + """A configuration object for DataConnect. + Attributes: - service_id (str): The Google Cloud project ID of the service. - location (str): The region of the service. - connector (str): The name of the connector. + service_id: A string representing the Google Cloud project ID of the service. + location: A string representing the region of the service. + connector: A string representing the name of the connector. """ service_id: str @@ -16,19 +23,62 @@ class ConnectorConfig: connector: str def __post_init__(self): + if not isinstance(self.service_id, str): + raise ValueError("service_id must be a string") if not self.service_id: raise ValueError("service_id cannot be empty") + if not isinstance(self.location, str): + raise ValueError("location must be a string") if not self.location: raise ValueError("location cannot be empty") + if not isinstance(self.connector, str): + raise ValueError("connector must be a string") if not self.connector: raise ValueError("connector cannot be empty") class DataConnect: + """Represents a Firebase Data Connect client instance. + This client provides access to the Firebase Data Connect service for a specific Firebase app and connector configuration. + + Attributes: + app: The Firebase App instance for this client. + config: The ConnectorConfig object specifying the service ID, location, and connector name. + """ def __init__(self, app: App, config: ConnectorConfig) -> None: - """Initializes a DataConnect client instance""" + """Initializes a DataConnect client instance. + + Args: + app: An App instance. + config: A ConnectorConfig instance. + """ self._app: App = app self._config = config + @property + def app(self) -> App: + return self._app + + @property + def config(self) -> ConnectorConfig: + return self._config + + +class _DataConnectService: + """Service that maintains a collection of DataConnect clients.""" + + def __init__(self, app: App) -> None: + self._app: App = app + self._clients: Dict[ConnectorConfig, DataConnect] = {} + + def get_client(self, config: ConnectorConfig) -> DataConnect: + """Creates a client based on the ConnectorConfig. These clients are cached.""" + if not isinstance(config, ConnectorConfig): + raise ValueError("Config must be of type firebase_admin.dataconnect.ConnectorConfig") + if config not in self._clients: + self._clients[config] = DataConnect(app=self._app, config=config) + return self._clients[config] + + def client(config: ConnectorConfig, app: Optional[App] = None) -> DataConnect: """Returns a DataConnect client for the specified configuration""" \ No newline at end of file diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index b20154aa..be5d98fb 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -7,42 +7,33 @@ from tests import testutils from unittest import mock +BASE_CONFIG = dataconnect.ConnectorConfig( + service_id="starterproject", + location="us-east4", + connector="my_connector", +) + + class TestConnectorConfig: def teardown_method(self, method): del method testutils.cleanup_apps() def test_connector_config_initialization(self): - config = dataconnect.ConnectorConfig( - service_id="starterproject", - location = "us-east4", - connector="my_connector" - ) - assert config.service_id=="starterproject" - assert config.location=="us-east4" - assert config.connector=="my_connector" + assert BASE_CONFIG.service_id == "starterproject" + assert BASE_CONFIG.location == "us-east4" + assert BASE_CONFIG.connector == "my_connector" def test_connector_config_is_frozen(self): - config = dataconnect.ConnectorConfig( - service_id="starterproject", - location = "us-east4", - connector="my_connector" - ) - with pytest.raises(AttributeError, match="cannot assign to field 'service_id'"): - config.service_id = "changed_id" + BASE_CONFIG.service_id = "changed_id" with pytest.raises(AttributeError, match="cannot assign to field 'location'"): - config.location = "us-central1" + BASE_CONFIG.location = "us-central1" with pytest.raises(AttributeError, match="cannot assign to field 'connector'"): - config.connector = "changed_connector" - - def testing_connector_config_string_written(self): - config = dataconnect.ConnectorConfig( - service_id="starterproject", - location = "us-east4", - connector="my_connector" - ) - repr_str=repr(config) + BASE_CONFIG.connector = "changed_connector" + + def test_connector_config_string_written(self): + repr_str = repr(BASE_CONFIG) assert "service_id='starterproject'" in repr_str assert "location='us-east4'" in repr_str assert "connector='my_connector'" in repr_str @@ -58,9 +49,9 @@ def test_connector_config_empty_strings(self): dataconnect.ConnectorConfig(service_id="starterproject", location="us-east4", connector="") def test_connector_config_invalid_types(self): - with pytest.raises(ValueError, match="service_id cannot be empty"): + with pytest.raises(ValueError, match="service_id must be a string"): dataconnect.ConnectorConfig(service_id=None, location="us-east4", connector="my_connector") - with pytest.raises(ValueError, match="location cannot be empty"): + with pytest.raises(ValueError, match="location must be a string"): dataconnect.ConnectorConfig(service_id="starterproject", location=123, connector="my_connector") class TestDataConnect: @@ -71,21 +62,19 @@ def teardown_method(self, method): def test_init_property_assignment(self): cred = testutils.MockCredential() try: - app = firebase_admin.initialize_app(cred, name = "starter_app") + app = firebase_admin.initialize_app(cred, name="starter_app") except Exception: pytest.fail("initialize app has an error") - config = dataconnect.ConnectorConfig(service_id="starterproject", location = "us-east4", connector="my_connector") - try: - data_connect_instance = dataconnect.DataConnect(app, config) + data_connect_instance = dataconnect.DataConnect(app, BASE_CONFIG) except Exception: pytest.fail("DataConnect initialization failed.") assert data_connect_instance._app is app - assert data_connect_instance._config is config + assert data_connect_instance._config is BASE_CONFIG assert data_connect_instance.app is app - assert data_connect_instance.config is config + assert data_connect_instance.config is BASE_CONFIG assert data_connect_instance._app.name == "starter_app" assert data_connect_instance._config.service_id == "starterproject" @@ -97,8 +86,8 @@ def teardown_method(self, method): def setup_method(self): self.cred = testutils.MockCredential() - self.app = firebase_admin.initialize_app(self.cred, name = 'starter_app') - self.config1 = dataconnect.ConnectorConfig(service_id='starterproject', location='us-east3', connector='my_connector') + self.app = firebase_admin.initialize_app(self.cred, name='starter_app') + self.config1 = BASE_CONFIG self.config2 = dataconnect.ConnectorConfig(service_id='starterproject2', location='us-east4', connector='my_connector2') @mock.patch('firebase_admin.dataconnect._DataConnectService.get_client', wraps=dataconnect._DataConnectService.get_client) @@ -109,21 +98,19 @@ def test_client_successful(self, mock_get_client): assert client_instance.config is self.config1 assert client_instance.app is self.app - def test_client_retrieval_diff_configs(self): - client1 = dataconnect.client(self.config1, app=self.app) - client2 = dataconnect.client(self.config2, app=self.app) - - assert client1 is not client2 - assert client1.config is self.config1 - assert client2.config is self.config2 - assert client1.app is self.app - assert client2.app is self.app - - def test_client_retrieval_same_config_cached(self): - client1 = dataconnect.client(self.config1, app=self.app) - client2 = dataconnect.client(self.config1, app=self.app) - - assert client1 is client2 + @pytest.mark.parametrize("config_a, config_b, expect_same", [ + (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l", "c_diff"), False), + (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l_diff", "c"), False), + (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s_diff", "l", "c"), False), + (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l", "c"), True), + ]) + def test_client_caching_permutations(self, config_a, config_b, expect_same): + client_a = dataconnect.client(config_a, app=self.app) + client_b = dataconnect.client(config_b, app=self.app) + if expect_same: + assert client_a is client_b + else: + assert client_a is not client_b def test_client_retrieval_different_apps_same_config(self): app2 = firebase_admin.initialize_app(self.cred, name='app2') @@ -155,7 +142,7 @@ def test_client_none_config(self): class TestDataConnectService: def setup_method(self): self.cred = testutils.MockCredential() - self.app = firebase_admin.initialize_app(self.cred, name = 'starter_app') + self.app = firebase_admin.initialize_app(self.cred, name='starter_app') self.service = dataconnect._DataConnectService(self.app) def teardown_method(self, method): @@ -242,9 +229,9 @@ def setup_method(self): self.app1 = firebase_admin.initialize_app(self.cred, name='integ_app1') self.app2 = firebase_admin.initialize_app(self.cred, name='integ_app2') - self.config1 = dataconnect.ConnectorConfig( service_id='service1', location='us-central1', connector='conn1') + self.config1 = BASE_CONFIG self.config2 = dataconnect.ConnectorConfig(service_id='service2', location='us-east4', connector='conn2') - self.config1_copy = dataconnect.ConnectorConfig(service_id='service1', location='us-central1', connector='conn1') + self.config1_copy = dataconnect.ConnectorConfig(service_id='starterproject', location='us-east4', connector='my_connector') def teardown_method(self, method): del method @@ -285,3 +272,4 @@ def test_uses_app_service_mechanism(self, mock_get_app_service): assert args[0] is self.app1 assert args[1] == '_data_connect_service' assert args[2] == dataconnect._DataConnectService + \ No newline at end of file From 4febd162ea8f84a018347a5313a7d682f1933113 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Tue, 30 Jun 2026 10:43:23 -0700 Subject: [PATCH 4/6] feat(dataconnect): Implemented foundational Data Connect client and service architecture Implemented ConnectorConfig dataclass, DataConnect client instance, _DataConnectService caching layer, and public client() factory function. Fixed mock recursion in test_client_successful using lambda delegation in test_data_connect.py. --- firebase_admin/dataconnect.py | 29 ++++++++++++++++++++--------- tests/test_data_connect.py | 6 +++--- 2 files changed, 23 insertions(+), 12 deletions(-) diff --git a/firebase_admin/dataconnect.py b/firebase_admin/dataconnect.py index 3bcb20b4..5188d012 100644 --- a/firebase_admin/dataconnect.py +++ b/firebase_admin/dataconnect.py @@ -1,3 +1,9 @@ +"""Firebase Data Connect module. + +This module contains utilities for accessing Firebase Data Connect services associated with +Firebase apps. +""" + from dataclasses import dataclass from typing import Dict, Optional @@ -7,7 +13,6 @@ _DATA_CONNECT_ATTRIBUTE = '_data_connect_service' - @dataclass(frozen=True) class ConnectorConfig: """A configuration object for DataConnect. @@ -39,19 +44,15 @@ def __post_init__(self): class DataConnect: """Represents a Firebase Data Connect client instance. - This client provides access to the Firebase Data Connect service for a specific Firebase app and connector configuration. + This client provides access to the Firebase Data Connect service + for a specific Firebase app and connector configuration. Attributes: app: The Firebase App instance for this client. config: The ConnectorConfig object specifying the service ID, location, and connector name. """ def __init__(self, app: App, config: ConnectorConfig) -> None: - """Initializes a DataConnect client instance. - - Args: - app: An App instance. - config: A ConnectorConfig instance. - """ + """Initializes a DataConnect client instance. """ self._app: App = app self._config = config @@ -81,4 +82,14 @@ def get_client(self, config: ConnectorConfig) -> DataConnect: def client(config: ConnectorConfig, app: Optional[App] = None) -> DataConnect: - """Returns a DataConnect client for the specified configuration""" \ No newline at end of file + """Returns a DataConnect client for the specified configuration.""" + if not isinstance(config, ConnectorConfig): + raise ValueError("Config must be of type firebase_admin.dataconnect.ConnectorConfig") + + if app is not None and not isinstance(app, App): + raise ValueError("App must be of type firebase_admin.App") + + # must check whether app has a _DataConnectService attached to it yet + dc_service = _utils.get_app_service(app, _DATA_CONNECT_ATTRIBUTE, _DataConnectService) + + return dc_service.get_client(config) diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index be5d98fb..2aab241c 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -13,7 +13,6 @@ connector="my_connector", ) - class TestConnectorConfig: def teardown_method(self, method): del method @@ -90,10 +89,11 @@ def setup_method(self): self.config1 = BASE_CONFIG self.config2 = dataconnect.ConnectorConfig(service_id='starterproject2', location='us-east4', connector='my_connector2') - @mock.patch('firebase_admin.dataconnect._DataConnectService.get_client', wraps=dataconnect._DataConnectService.get_client) + @mock.patch.object(dataconnect._DataConnectService, 'get_client', autospec=True) def test_client_successful(self, mock_get_client): + mock_get_client.side_effect = lambda service, config: dataconnect.DataConnect(service._app, config) client_instance = dataconnect.client(self.config1, app=self.app) - mock_get_client.assert_called_once_with(self.config1) + mock_get_client.assert_called_once_with(mock.ANY, self.config1) assert isinstance(client_instance, dataconnect.DataConnect) assert client_instance.config is self.config1 assert client_instance.app is self.app From aa61847c52eefe3cd1957cf9f95e50b7c72d30c6 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Tue, 30 Jun 2026 10:48:27 -0700 Subject: [PATCH 5/6] chore(dataconnect): Standardized test suite formatting to achieve 10.00/10 linter score --- tests/test_data_connect.py | 569 ++++++++++++++++++++----------------- 1 file changed, 315 insertions(+), 254 deletions(-) diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index 2aab241c..ac348b9d 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -1,11 +1,10 @@ +from unittest import mock import pytest import firebase_admin from firebase_admin import _utils from firebase_admin import dataconnect -from firebase_admin import credentials from tests import testutils -from unittest import mock BASE_CONFIG = dataconnect.ConnectorConfig( service_id="starterproject", @@ -13,263 +12,325 @@ connector="my_connector", ) + class TestConnectorConfig: - def teardown_method(self, method): - del method - testutils.cleanup_apps() - - def test_connector_config_initialization(self): - assert BASE_CONFIG.service_id == "starterproject" - assert BASE_CONFIG.location == "us-east4" - assert BASE_CONFIG.connector == "my_connector" - - def test_connector_config_is_frozen(self): - with pytest.raises(AttributeError, match="cannot assign to field 'service_id'"): - BASE_CONFIG.service_id = "changed_id" - with pytest.raises(AttributeError, match="cannot assign to field 'location'"): - BASE_CONFIG.location = "us-central1" - with pytest.raises(AttributeError, match="cannot assign to field 'connector'"): - BASE_CONFIG.connector = "changed_connector" - - def test_connector_config_string_written(self): - repr_str = repr(BASE_CONFIG) - assert "service_id='starterproject'" in repr_str - assert "location='us-east4'" in repr_str - assert "connector='my_connector'" in repr_str - - def test_connector_config_empty_strings(self): - with pytest.raises(ValueError, match="service_id cannot be empty"): - dataconnect.ConnectorConfig(service_id="", location="us-east4", connector="my_connector") - - with pytest.raises(ValueError, match="location cannot be empty"): - dataconnect.ConnectorConfig(service_id="starterproject", location="", connector="my_connector") - - with pytest.raises(ValueError, match="connector cannot be empty"): - dataconnect.ConnectorConfig(service_id="starterproject", location="us-east4", connector="") - - def test_connector_config_invalid_types(self): - with pytest.raises(ValueError, match="service_id must be a string"): - dataconnect.ConnectorConfig(service_id=None, location="us-east4", connector="my_connector") - with pytest.raises(ValueError, match="location must be a string"): - dataconnect.ConnectorConfig(service_id="starterproject", location=123, connector="my_connector") + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_connector_config_initialization(self): + assert BASE_CONFIG.service_id == "starterproject" + assert BASE_CONFIG.location == "us-east4" + assert BASE_CONFIG.connector == "my_connector" + + def test_connector_config_is_frozen(self): + with pytest.raises(AttributeError, match="cannot assign to field 'service_id'"): + BASE_CONFIG.service_id = "changed_id" + with pytest.raises(AttributeError, match="cannot assign to field 'location'"): + BASE_CONFIG.location = "us-central1" + with pytest.raises(AttributeError, match="cannot assign to field 'connector'"): + BASE_CONFIG.connector = "changed_connector" + + def test_connector_config_string_written(self): + repr_str = repr(BASE_CONFIG) + assert "service_id='starterproject'" in repr_str + assert "location='us-east4'" in repr_str + assert "connector='my_connector'" in repr_str + + def test_connector_config_empty_strings(self): + with pytest.raises(ValueError, match="service_id cannot be empty"): + dataconnect.ConnectorConfig( + service_id="", location="us-east4", connector="my_connector" + ) + + with pytest.raises(ValueError, match="location cannot be empty"): + dataconnect.ConnectorConfig( + service_id="starterproject", location="", connector="my_connector" + ) + + with pytest.raises(ValueError, match="connector cannot be empty"): + dataconnect.ConnectorConfig( + service_id="starterproject", location="us-east4", connector="" + ) + + def test_connector_config_invalid_types(self): + with pytest.raises(ValueError, match="service_id must be a string"): + dataconnect.ConnectorConfig( + service_id=None, location="us-east4", connector="my_connector" + ) + with pytest.raises(ValueError, match="location must be a string"): + dataconnect.ConnectorConfig( + service_id="starterproject", location=123, connector="my_connector" + ) + class TestDataConnect: - def teardown_method(self, method): - del method - testutils.cleanup_apps() - - def test_init_property_assignment(self): - cred = testutils.MockCredential() - try: - app = firebase_admin.initialize_app(cred, name="starter_app") - except Exception: - pytest.fail("initialize app has an error") - - try: - data_connect_instance = dataconnect.DataConnect(app, BASE_CONFIG) - except Exception: - pytest.fail("DataConnect initialization failed.") - - assert data_connect_instance._app is app - assert data_connect_instance._config is BASE_CONFIG - assert data_connect_instance.app is app - assert data_connect_instance.config is BASE_CONFIG - - assert data_connect_instance._app.name == "starter_app" - assert data_connect_instance._config.service_id == "starterproject" + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_init_property_assignment(self): + cred = testutils.MockCredential() + try: + app = firebase_admin.initialize_app(cred, name="starter_app") + except ValueError: + pytest.fail("initialize app has an error") + + try: + data_connect_instance = dataconnect.DataConnect(app, BASE_CONFIG) + except ValueError: + pytest.fail("DataConnect initialization failed.") + + assert data_connect_instance._app is app # pylint: disable=protected-access + assert data_connect_instance._config is BASE_CONFIG # pylint: disable=protected-access + assert data_connect_instance.app is app + assert data_connect_instance.config is BASE_CONFIG + + assert data_connect_instance._app.name == "starter_app" # pylint: disable=protected-access + assert data_connect_instance._config.service_id == "starterproject" # pylint: disable=protected-access + class TestDataConnectClientFactory: - def teardown_method(self, method): - del method - testutils.cleanup_apps() - - def setup_method(self): - self.cred = testutils.MockCredential() - self.app = firebase_admin.initialize_app(self.cred, name='starter_app') - self.config1 = BASE_CONFIG - self.config2 = dataconnect.ConnectorConfig(service_id='starterproject2', location='us-east4', connector='my_connector2') - - @mock.patch.object(dataconnect._DataConnectService, 'get_client', autospec=True) - def test_client_successful(self, mock_get_client): - mock_get_client.side_effect = lambda service, config: dataconnect.DataConnect(service._app, config) - client_instance = dataconnect.client(self.config1, app=self.app) - mock_get_client.assert_called_once_with(mock.ANY, self.config1) - assert isinstance(client_instance, dataconnect.DataConnect) - assert client_instance.config is self.config1 - assert client_instance.app is self.app - - @pytest.mark.parametrize("config_a, config_b, expect_same", [ - (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l", "c_diff"), False), - (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l_diff", "c"), False), - (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s_diff", "l", "c"), False), - (dataconnect.ConnectorConfig("s", "l", "c"), dataconnect.ConnectorConfig("s", "l", "c"), True), - ]) - def test_client_caching_permutations(self, config_a, config_b, expect_same): - client_a = dataconnect.client(config_a, app=self.app) - client_b = dataconnect.client(config_b, app=self.app) - if expect_same: - assert client_a is client_b - else: - assert client_a is not client_b - - def test_client_retrieval_different_apps_same_config(self): - app2 = firebase_admin.initialize_app(self.cred, name='app2') - - client1 = dataconnect.client(self.config1, app=self.app) - client2 = dataconnect.client(self.config1, app=app2) - - assert client1 is not client2 - assert client1.app is self.app - assert client1.app is not client2.app - - def test_invalid_config_type(self): - with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): - dataconnect.client('not-a-config', app=self.app) - - def test_invalid_app_type(self): - with pytest.raises(ValueError, match="App must be of type firebase_admin.App"): - dataconnect.client(self.config1, 'not-a-app') - - def test_client_default_app(self): - default_app = firebase_admin.initialize_app(self.cred) - client_instance = dataconnect.client(self.config1) - assert client_instance.app is default_app - - def test_client_none_config(self): - with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): - dataconnect.client(None, app=self.app) - + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def setup_method(self): + self.cred = testutils.MockCredential() + self.app = firebase_admin.initialize_app(self.cred, name="starter_app") + self.config1 = BASE_CONFIG + self.config2 = dataconnect.ConnectorConfig( + service_id="starterproject2", location="us-east4", connector="my_connector2" + ) + + @mock.patch.object(dataconnect._DataConnectService, "get_client", autospec=True) + def test_client_successful(self, mock_get_client): + mock_get_client.side_effect = lambda service, config: dataconnect.DataConnect( + service._app, config # pylint: disable=protected-access + ) + client_instance = dataconnect.client(self.config1, app=self.app) + mock_get_client.assert_called_once_with(mock.ANY, self.config1) + assert isinstance(client_instance, dataconnect.DataConnect) + assert client_instance.config is self.config1 + assert client_instance.app is self.app + + @pytest.mark.parametrize("config_a, config_b, expect_same", [ + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c_diff"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l_diff", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s_diff", "l", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c"), + True, + ), + ]) + def test_client_caching_permutations(self, config_a, config_b, expect_same): + client_a = dataconnect.client(config_a, app=self.app) + client_b = dataconnect.client(config_b, app=self.app) + if expect_same: + assert client_a is client_b + else: + assert client_a is not client_b + + def test_client_retrieval_different_apps_same_config(self): + app2 = firebase_admin.initialize_app(self.cred, name="app2") + + client1 = dataconnect.client(self.config1, app=self.app) + client2 = dataconnect.client(self.config1, app=app2) + + assert client1 is not client2 + assert client1.app is self.app + assert client1.app is not client2.app + + def test_invalid_config_type(self): + err_msg = "Config must be of type firebase_admin.dataconnect.ConnectorConfig" + with pytest.raises(ValueError, match=err_msg): + dataconnect.client("not-a-config", app=self.app) + + def test_invalid_app_type(self): + with pytest.raises(ValueError, match="App must be of type firebase_admin.App"): + dataconnect.client(self.config1, "not-a-app") + + def test_client_default_app(self): + default_app = firebase_admin.initialize_app(self.cred) + client_instance = dataconnect.client(self.config1) + assert client_instance.app is default_app + + def test_client_none_config(self): + err_msg = "Config must be of type firebase_admin.dataconnect.ConnectorConfig" + with pytest.raises(ValueError, match=err_msg): + dataconnect.client(None, app=self.app) + + class TestDataConnectService: - def setup_method(self): - self.cred = testutils.MockCredential() - self.app = firebase_admin.initialize_app(self.cred, name='starter_app') - self.service = dataconnect._DataConnectService(self.app) - - def teardown_method(self, method): - del method - testutils.cleanup_apps() - - def test_cache_hit(self): - config = dataconnect.ConnectorConfig('s1', 'l1', 'c1') - client1 = self.service.get_client(config) - client2 = self.service.get_client(config) - assert client1 is client2 - - assert isinstance(client1, dataconnect.DataConnect) - assert client1.config is config - - def test_cache_miss_on_different_config(self): - config1 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') - config2 = dataconnect.ConnectorConfig('s2', 'l2', 'c2') - client1 = self.service.get_client(config1) - client2 = self.service.get_client(config2) - assert client1 is not client2 - - @pytest.mark.parametrize("config_a, config_b, expect_same", [ - (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l', 'c_diff'), False), - (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l_diff', 'c'), False), - (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s_diff', 'l', 'c'), False), - (dataconnect.ConnectorConfig('s', 'l', 'c'), dataconnect.ConnectorConfig('s', 'l', 'c'), True), - ]) - def test_complex_cache_key(self, config_a, config_b, expect_same): - client_a = self.service.get_client(config_a) - client_b = self.service.get_client(config_b) - if expect_same: - assert client_a is client_b - else: - assert client_a is not client_b - - def test_config_equivalence(self): - config1 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') - config2 = dataconnect.ConnectorConfig('s1', 'l1', 'c1') - client1 = self.service.get_client(config1) - client2 = self.service.get_client(config2) - assert client1 is client2 - - @mock.patch('firebase_admin.dataconnect.DataConnect', autospec=True) - def test_client_creation_mocking(self, MockDataConnect): - config1 = dataconnect.ConnectorConfig('s_mock', 'l_mock', 'c_mock1') - config2 = dataconnect.ConnectorConfig('s_mock', 'l_mock', 'c_mock2') - - self.service.get_client(config1) - MockDataConnect.assert_called_once_with(app=self.app, config=config1) - - MockDataConnect.reset_mock() - - self.service.get_client(config1) - MockDataConnect.assert_not_called() - - MockDataConnect.reset_mock() - - # first call using config2 - self.service.get_client(config2) - MockDataConnect.assert_called_once_with(app=self.app, config=config2) - - @mock.patch('firebase_admin.dataconnect.DataConnect', autospec=True) - def test_error_handling_in_creation(self, MockDataConnect): - config = dataconnect.ConnectorConfig('s_err', 'l_err', 'c_err') - test_error = RuntimeError("Failed to create client") - MockDataConnect.side_effect = test_error - - with pytest.raises(RuntimeError, match="Failed to create client"): - self.service.get_client(config) - - # Ensure the failed creation wasn't cached - MockDataConnect.side_effect = None - self.service.get_client(config) - assert MockDataConnect.call_count == 2 - - def test_invalid_config_in_service(self): - with pytest.raises(ValueError, match="Config must be of type firebase_admin.dataconnect.ConnectorConfig"): - self.service.get_client(None) + + def setup_method(self): + self.cred = testutils.MockCredential() + self.app = firebase_admin.initialize_app(self.cred, name="starter_app") + self.service = dataconnect._DataConnectService(self.app) # pylint: disable=protected-access + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_cache_hit(self): + config = dataconnect.ConnectorConfig("s1", "l1", "c1") + client1 = self.service.get_client(config) + client2 = self.service.get_client(config) + assert client1 is client2 + + assert isinstance(client1, dataconnect.DataConnect) + assert client1.config is config + + def test_cache_miss_on_different_config(self): + config1 = dataconnect.ConnectorConfig("s1", "l1", "c1") + config2 = dataconnect.ConnectorConfig("s2", "l2", "c2") + client1 = self.service.get_client(config1) + client2 = self.service.get_client(config2) + assert client1 is not client2 + + @pytest.mark.parametrize("config_a, config_b, expect_same", [ + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c_diff"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l_diff", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s_diff", "l", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c"), + True, + ), + ]) + def test_complex_cache_key(self, config_a, config_b, expect_same): + client_a = self.service.get_client(config_a) + client_b = self.service.get_client(config_b) + if expect_same: + assert client_a is client_b + else: + assert client_a is not client_b + + def test_config_equivalence(self): + config1 = dataconnect.ConnectorConfig("s1", "l1", "c1") + config2 = dataconnect.ConnectorConfig("s1", "l1", "c1") + client1 = self.service.get_client(config1) + client2 = self.service.get_client(config2) + assert client1 is client2 + + @mock.patch("firebase_admin.dataconnect.DataConnect", autospec=True) + def test_client_creation_mocking(self, mock_data_connect): + config1 = dataconnect.ConnectorConfig("s_mock", "l_mock", "c_mock1") + config2 = dataconnect.ConnectorConfig("s_mock", "l_mock", "c_mock2") + + self.service.get_client(config1) + mock_data_connect.assert_called_once_with(app=self.app, config=config1) + + mock_data_connect.reset_mock() + + self.service.get_client(config1) + mock_data_connect.assert_not_called() + + mock_data_connect.reset_mock() + + # first call using config2 + self.service.get_client(config2) + mock_data_connect.assert_called_once_with(app=self.app, config=config2) + + @mock.patch("firebase_admin.dataconnect.DataConnect", autospec=True) + def test_error_handling_in_creation(self, mock_data_connect): + config = dataconnect.ConnectorConfig("s_err", "l_err", "c_err") + test_error = RuntimeError("Failed to create client") + mock_data_connect.side_effect = test_error + + with pytest.raises(RuntimeError, match="Failed to create client"): + self.service.get_client(config) + + # Ensure the failed creation wasn't cached + mock_data_connect.side_effect = None + self.service.get_client(config) + assert mock_data_connect.call_count == 2 + + def test_invalid_config_in_service(self): + err_msg = "Config must be of type firebase_admin.dataconnect.ConnectorConfig" + with pytest.raises(ValueError, match=err_msg): + self.service.get_client(None) + class TestDataConnectServiceIntegration: - def setup_method(self): - self.cred = testutils.MockCredential() - self.app1 = firebase_admin.initialize_app(self.cred, name='integ_app1') - self.app2 = firebase_admin.initialize_app(self.cred, name='integ_app2') - - self.config1 = BASE_CONFIG - self.config2 = dataconnect.ConnectorConfig(service_id='service2', location='us-east4', connector='conn2') - self.config1_copy = dataconnect.ConnectorConfig(service_id='starterproject', location='us-east4', connector='my_connector') - - def teardown_method(self, method): - del method - testutils.cleanup_apps() - - def test_overall_client_retrieval_and_caching(self): - client1a = dataconnect.client(self.config1, app=self.app1) - client1b = dataconnect.client(self.config1_copy, app=self.app1) - client2 = dataconnect.client(self.config2, app=self.app1) - - assert isinstance(client1a, dataconnect.DataConnect), "Client should be a DataConnect instance" - assert client1a.app is self.app1, "Client should be associated with app1" - assert client1a.config is self.config1, "Client should hold the specific config1" - - # Same config - assert client1b is client1a, "Client should be cached for an equivalent config on the same app" - - # Different config - assert isinstance(client2, dataconnect.DataConnect), "Client should be a DataConnect instance" - assert client2.app is self.app1, "Client should be associated with app1" - assert client2.config is self.config2, "Client should hold the specific config2" - assert client2 is not client1a, "Clients with different configs should be different instances" - - # Different app - client1_app2 = dataconnect.client(self.config1, app=self.app2) - - assert isinstance(client1_app2, dataconnect.DataConnect), "Client on app2 should be a DataConnect instance" - assert client1_app2.app is self.app2, "Client should be associated with app2" - assert client1_app2.config is self.config1, "Client on app2 should hold config1" - assert client1_app2 is not client1a, "Clients on different apps should be different instances" - - @mock.patch.object(_utils, 'get_app_service', wraps=_utils.get_app_service) - def test_uses_app_service_mechanism(self, mock_get_app_service): - """Ensures dataconnect.client uses the standard app service loader.""" - dataconnect.client(self.config1, app=self.app1) - mock_get_app_service.assert_called_once() - args, _ = mock_get_app_service.call_args - assert args[0] is self.app1 - assert args[1] == '_data_connect_service' - assert args[2] == dataconnect._DataConnectService - \ No newline at end of file + + def setup_method(self): + self.cred = testutils.MockCredential() + self.app1 = firebase_admin.initialize_app(self.cred, name="integ_app1") + self.app2 = firebase_admin.initialize_app(self.cred, name="integ_app2") + + self.config1 = BASE_CONFIG + self.config2 = dataconnect.ConnectorConfig( + service_id="service2", location="us-east4", connector="conn2" + ) + self.config1_copy = dataconnect.ConnectorConfig( + service_id="starterproject", location="us-east4", connector="my_connector" + ) + + def teardown_method(self, method): + del method + testutils.cleanup_apps() + + def test_overall_client_retrieval_and_caching(self): + client1a = dataconnect.client(self.config1, app=self.app1) + client1b = dataconnect.client(self.config1_copy, app=self.app1) + client2 = dataconnect.client(self.config2, app=self.app1) + + assert isinstance(client1a, dataconnect.DataConnect) + assert client1a.app is self.app1 + assert client1a.config is self.config1 + + # Same config + assert client1b is client1a + + # Different config + assert isinstance(client2, dataconnect.DataConnect) + assert client2.app is self.app1 + assert client2.config is self.config2 + assert client2 is not client1a + + # Different app + client1_app2 = dataconnect.client(self.config1, app=self.app2) + + assert isinstance(client1_app2, dataconnect.DataConnect) + assert client1_app2.app is self.app2 + assert client1_app2.config is self.config1 + assert client1_app2 is not client1a + + @mock.patch.object(_utils, "get_app_service", wraps=_utils.get_app_service) + def test_uses_app_service_mechanism(self, mock_get_app_service): + """Ensures dataconnect.client uses the standard app service loader.""" + dataconnect.client(self.config1, app=self.app1) + mock_get_app_service.assert_called_once() + args, _ = mock_get_app_service.call_args + assert args[0] is self.app1 + assert args[1] == "_data_connect_service" + assert args[2] == dataconnect._DataConnectService # pylint: disable=protected-access From dd15dc4274ed0825de705c226ff9c5c67922b8e9 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Wed, 1 Jul 2026 15:08:47 -0700 Subject: [PATCH 6/6] refactor(dataconnect): Addressed code review feedback and optimized test suite structure Removed duplicate client factory caching tests, moved app service loader test to unit test class, added connector property validation, and formatted parameter lists. --- firebase_admin/dataconnect.py | 3 - tests/test_data_connect.py | 117 ++++++++++++++-------------------- 2 files changed, 49 insertions(+), 71 deletions(-) diff --git a/firebase_admin/dataconnect.py b/firebase_admin/dataconnect.py index 5188d012..94812254 100644 --- a/firebase_admin/dataconnect.py +++ b/firebase_admin/dataconnect.py @@ -86,9 +86,6 @@ def client(config: ConnectorConfig, app: Optional[App] = None) -> DataConnect: if not isinstance(config, ConnectorConfig): raise ValueError("Config must be of type firebase_admin.dataconnect.ConnectorConfig") - if app is not None and not isinstance(app, App): - raise ValueError("App must be of type firebase_admin.App") - # must check whether app has a _DataConnectService attached to it yet dc_service = _utils.get_app_service(app, _DATA_CONNECT_ATTRIBUTE, _DataConnectService) diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index ac348b9d..23dc8ddd 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -63,6 +63,10 @@ def test_connector_config_invalid_types(self): dataconnect.ConnectorConfig( service_id="starterproject", location=123, connector="my_connector" ) + with pytest.raises(ValueError, match="connector must be a string"): + dataconnect.ConnectorConfig( + service_id="starterproject", location="us-east4", connector=456 + ) class TestDataConnect: @@ -111,41 +115,15 @@ def test_client_successful(self, mock_get_client): mock_get_client.side_effect = lambda service, config: dataconnect.DataConnect( service._app, config # pylint: disable=protected-access ) - client_instance = dataconnect.client(self.config1, app=self.app) - mock_get_client.assert_called_once_with(mock.ANY, self.config1) - assert isinstance(client_instance, dataconnect.DataConnect) - assert client_instance.config is self.config1 - assert client_instance.app is self.app - - @pytest.mark.parametrize("config_a, config_b, expect_same", [ - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l", "c_diff"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l_diff", "c"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s_diff", "l", "c"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l", "c"), - True, - ), - ]) - def test_client_caching_permutations(self, config_a, config_b, expect_same): - client_a = dataconnect.client(config_a, app=self.app) - client_b = dataconnect.client(config_b, app=self.app) - if expect_same: - assert client_a is client_b - else: - assert client_a is not client_b + client1 = dataconnect.client(self.config1, app=self.app) + client2 = dataconnect.client(self.config2, app=self.app) + assert mock_get_client.call_count == 2 + mock_get_client.assert_any_call(mock.ANY, self.config1) + mock_get_client.assert_any_call(mock.ANY, self.config2) + assert isinstance(client1, dataconnect.DataConnect) + assert client1.config is self.config1 + assert client1.app is self.app + assert client2.config is self.config2 def test_client_retrieval_different_apps_same_config(self): app2 = firebase_admin.initialize_app(self.cred, name="app2") @@ -163,7 +141,7 @@ def test_invalid_config_type(self): dataconnect.client("not-a-config", app=self.app) def test_invalid_app_type(self): - with pytest.raises(ValueError, match="App must be of type firebase_admin.App"): + with pytest.raises(ValueError, match="Illegal app argument"): dataconnect.client(self.config1, "not-a-app") def test_client_default_app(self): @@ -176,6 +154,16 @@ def test_client_none_config(self): with pytest.raises(ValueError, match=err_msg): dataconnect.client(None, app=self.app) + @mock.patch.object(_utils, "get_app_service", wraps=_utils.get_app_service) + def test_uses_app_service_mechanism(self, mock_get_app_service): + """Ensures dataconnect.client uses the standard app service loader.""" + dataconnect.client(self.config1, app=self.app) + mock_get_app_service.assert_called_once() + args, _ = mock_get_app_service.call_args + assert args[0] is self.app + assert args[1] == "_data_connect_service" + assert args[2] == dataconnect._DataConnectService # pylint: disable=protected-access + class TestDataConnectService: @@ -204,28 +192,31 @@ def test_cache_miss_on_different_config(self): client2 = self.service.get_client(config2) assert client1 is not client2 - @pytest.mark.parametrize("config_a, config_b, expect_same", [ - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l", "c_diff"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l_diff", "c"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s_diff", "l", "c"), - False, - ), - ( - dataconnect.ConnectorConfig("s", "l", "c"), - dataconnect.ConnectorConfig("s", "l", "c"), - True, - ), - ]) + @pytest.mark.parametrize( + "config_a, config_b, expect_same", + [ + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c_diff"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l_diff", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s_diff", "l", "c"), + False, + ), + ( + dataconnect.ConnectorConfig("s", "l", "c"), + dataconnect.ConnectorConfig("s", "l", "c"), + True, + ), + ], + ) def test_complex_cache_key(self, config_a, config_b, expect_same): client_a = self.service.get_client(config_a) client_b = self.service.get_client(config_b) @@ -324,13 +315,3 @@ def test_overall_client_retrieval_and_caching(self): assert client1_app2.app is self.app2 assert client1_app2.config is self.config1 assert client1_app2 is not client1a - - @mock.patch.object(_utils, "get_app_service", wraps=_utils.get_app_service) - def test_uses_app_service_mechanism(self, mock_get_app_service): - """Ensures dataconnect.client uses the standard app service loader.""" - dataconnect.client(self.config1, app=self.app1) - mock_get_app_service.assert_called_once() - args, _ = mock_get_app_service.call_args - assert args[0] is self.app1 - assert args[1] == "_data_connect_service" - assert args[2] == dataconnect._DataConnectService # pylint: disable=protected-access