From e701593e4037f825d2994f2d92ba1dd2b2650896 Mon Sep 17 00:00:00 2001 From: Minseul Kim Date: Fri, 26 Jun 2026 13:21:53 -0700 Subject: [PATCH 1/9] 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 d1f6af4e782c3a367e7549301d94b30cd85b1fb6 Mon Sep 17 00:00:00 2001 From: Minseul Kim Date: Fri, 26 Jun 2026 13:39:55 -0700 Subject: [PATCH 2/9] 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 83fc0db48f1b9cec51d60754cec58de40c4e6aac Mon Sep 17 00:00:00 2001 From: mk2023 Date: Mon, 29 Jun 2026 15:32:43 -0700 Subject: [PATCH 3/9] 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 5f677e5fd1de4089d43a4750b5df2a6b0a58c0f4 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Tue, 30 Jun 2026 10:43:23 -0700 Subject: [PATCH 4/9] 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 82fe3f490c2fc121863c6264cb6099d19cd011a1 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Tue, 30 Jun 2026 10:48:27 -0700 Subject: [PATCH 5/9] 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 f2b87ec32bc86d800b76c4a20aa04c8a5199f3a2 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Wed, 1 Jul 2026 15:08:47 -0700 Subject: [PATCH 6/9] 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 From 572194e9cc60112b8ea6f22b0b47ba9a928d1b88 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Mon, 6 Jul 2026 11:09:48 -0700 Subject: [PATCH 7/9] chore(fdc): Updated documentation and moved integration tests Added missing copyright headers and improved docstrings with proper formatting in dataconnect.py. Moved TestDataConnectServiceIntegration from tests/test_data_connect.py into a dedicated integration test file under integration/test_data_connect.py. --- firebase_admin/dataconnect.py | 36 +++++++++++++- integration/test_data_connect.py | 85 ++++++++++++++++++++++++++++++++ tests/test_data_connect.py | 64 +++++++----------------- 3 files changed, 136 insertions(+), 49 deletions(-) create mode 100644 integration/test_data_connect.py diff --git a/firebase_admin/dataconnect.py b/firebase_admin/dataconnect.py index 94812254..201e3b12 100644 --- a/firebase_admin/dataconnect.py +++ b/firebase_admin/dataconnect.py @@ -1,3 +1,17 @@ +# Copyright 2026 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + """Firebase Data Connect module. This module contains utilities for accessing Firebase Data Connect services associated with @@ -11,7 +25,7 @@ __all__ = ['ConnectorConfig', 'DataConnect', 'client'] -_DATA_CONNECT_ATTRIBUTE = '_data_connect_service' +_DATA_CONNECT_ATTRIBUTE = '_data_connect' @dataclass(frozen=True) class ConnectorConfig: @@ -44,6 +58,7 @@ 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. @@ -51,6 +66,7 @@ class DataConnect: 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. """ self._app: App = app @@ -82,7 +98,23 @@ def get_client(self, config: ConnectorConfig) -> DataConnect: def client(config: ConnectorConfig, app: Optional[App] = None) -> DataConnect: - """Returns a DataConnect client for the specified configuration.""" + """Returns a DataConnect client for the specified configuration. + + This function does not make any RPC calls. + + Args: + config: A ConnectorConfig instance specifying the service ID, location, + and connector name. + app: An App instance (optional). Defaults to the default Firebase App. + + Returns: + DataConnect: A handle to the specified DataConnect client instance. + + Raises: + ValueError: If config argument is not an instance of ConnectorConfig, or if + app is an invalid instance of App. + """ + if not isinstance(config, ConnectorConfig): raise ValueError("Config must be of type firebase_admin.dataconnect.ConnectorConfig") diff --git a/integration/test_data_connect.py b/integration/test_data_connect.py new file mode 100644 index 00000000..cd10fa8b --- /dev/null +++ b/integration/test_data_connect.py @@ -0,0 +1,85 @@ +# Copyright 2026 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Integration tests for firebase_admin.dataconnect module.""" + +import pytest + +import firebase_admin +from firebase_admin import dataconnect +from tests import testutils + +BASE_CONFIG = dataconnect.ConnectorConfig( + service_id="starterproject", + location="us-east4", + connector="my_connector", +) + + +@pytest.fixture(scope='module', autouse=True) +def default_app(): + # Overwrites the default_app fixture in conftest.py. + # This test suite does not use the default app. + pass + + +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 + for app in [getattr(self, 'app1', None), getattr(self, 'app2', None)]: + if app: + try: + firebase_admin.delete_app(app) + except ValueError: + pass + + 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 diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index 23dc8ddd..5680f20c 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -1,3 +1,19 @@ +# Copyright 2026 Google Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +"""Test cases for the firebase_admin.dataconnect module.""" + from unittest import mock import pytest @@ -161,7 +177,7 @@ def test_uses_app_service_mechanism(self, mock_get_app_service): 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[1] == dataconnect._DATA_CONNECT_ATTRIBUTE # pylint: disable=protected-access assert args[2] == dataconnect._DataConnectService # pylint: disable=protected-access @@ -269,49 +285,3 @@ 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) - 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 From dcda193796799b6311beb1dbb896bf6711e69230 Mon Sep 17 00:00:00 2001 From: mk2023 Date: Mon, 6 Jul 2026 12:41:10 -0700 Subject: [PATCH 8/9] refactor(fdc): Returned TestDataConnectServiceIntegration to tests/test_data_connect.py Moved TestDataConnectServiceIntegration back to tests/test_data_connect.py and removed integration/test_data_connect.py. --- integration/test_data_connect.py | 85 -------------------------------- tests/test_data_connect.py | 46 +++++++++++++++++ 2 files changed, 46 insertions(+), 85 deletions(-) delete mode 100644 integration/test_data_connect.py diff --git a/integration/test_data_connect.py b/integration/test_data_connect.py deleted file mode 100644 index cd10fa8b..00000000 --- a/integration/test_data_connect.py +++ /dev/null @@ -1,85 +0,0 @@ -# Copyright 2026 Google Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -"""Integration tests for firebase_admin.dataconnect module.""" - -import pytest - -import firebase_admin -from firebase_admin import dataconnect -from tests import testutils - -BASE_CONFIG = dataconnect.ConnectorConfig( - service_id="starterproject", - location="us-east4", - connector="my_connector", -) - - -@pytest.fixture(scope='module', autouse=True) -def default_app(): - # Overwrites the default_app fixture in conftest.py. - # This test suite does not use the default app. - pass - - -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 - for app in [getattr(self, 'app1', None), getattr(self, 'app2', None)]: - if app: - try: - firebase_admin.delete_app(app) - except ValueError: - pass - - 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 diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index 5680f20c..79b01f64 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -285,3 +285,49 @@ 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) + 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 From 66885adc3e17aad10ee8cdf610a47b213ebf92ab Mon Sep 17 00:00:00 2001 From: mk2023 Date: Mon, 6 Jul 2026 12:50:28 -0700 Subject: [PATCH 9/9] test(fdc): Renamed integration test class to TestDataConnectServiceWorkflow Renamed TestDataConnectServiceIntegration to TestDataConnectServiceWorkflow in tests/test_data_connect.py to clarify that it is an in-memory unit/functional test rather than a network integration test. --- tests/test_data_connect.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_data_connect.py b/tests/test_data_connect.py index 79b01f64..c226b12b 100644 --- a/tests/test_data_connect.py +++ b/tests/test_data_connect.py @@ -287,7 +287,7 @@ def test_invalid_config_in_service(self): self.service.get_client(None) -class TestDataConnectServiceIntegration: +class TestDataConnectServiceWorkflow: def setup_method(self): self.cred = testutils.MockCredential()