From aa86a3a290538e34c90048ac3c185177679c882a Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Tue, 15 Apr 2025 16:36:01 -0400 Subject: [PATCH 01/10] PYTHON-5306 - Fix use of public MongoClient attributes before connection --- pymongo/asynchronous/mongo_client.py | 39 ++++++++++++++++++++++------ pymongo/synchronous/mongo_client.py | 39 ++++++++++++++++++++++------ test/asynchronous/test_client.py | 36 +++++++++++++++++++++++++ test/test_client.py | 36 +++++++++++++++++++++++++ 4 files changed, 134 insertions(+), 16 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 7744a75d9c..6771266f1c 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -109,6 +109,7 @@ ) from pymongo.read_preferences import ReadPreference, _ServerMode from pymongo.results import ClientBulkWriteResult +from pymongo.server_description import ServerDescription from pymongo.server_selectors import writable_server_selector from pymongo.server_type import SERVER_TYPE from pymongo.topology_description import TOPOLOGY_TYPE, TopologyDescription @@ -779,7 +780,7 @@ def __init__( keyword_opts["document_class"] = doc_class self._resolve_srv_info: dict[str, Any] = {"keyword_opts": keyword_opts} - seeds = set() + self._seeds = set() is_srv = False username = None password = None @@ -804,18 +805,18 @@ def __init__( srv_max_hosts=srv_max_hosts, ) is_srv = entity.startswith(SRV_SCHEME) - seeds.update(res["nodelist"]) + self._seeds.update(res["nodelist"]) username = res["username"] or username password = res["password"] or password dbase = res["database"] or dbase opts = res["options"] fqdn = res["fqdn"] else: - seeds.update(split_hosts(entity, self._port)) - if not seeds: + self._seeds.update(split_hosts(entity, self._port)) + if not self._seeds: raise ConfigurationError("need to specify at least one host") - for hostname in [node[0] for node in seeds]: + for hostname in [node[0] for node in self._seeds]: if _detect_external_db(hostname): break @@ -838,7 +839,7 @@ def __init__( srv_service_name = opts.get("srvServiceName", common.SRV_SERVICE_NAME) srv_max_hosts = srv_max_hosts or opts.get("srvmaxhosts") - opts = self._normalize_and_validate_options(opts, seeds) + opts = self._normalize_and_validate_options(opts, self._seeds) # Username and password passed as kwargs override user info in URI. username = opts.get("username", username) @@ -857,7 +858,7 @@ def __init__( "username": username, "password": password, "dbase": dbase, - "seeds": seeds, + "seeds": self._seeds, "fqdn": fqdn, "srv_service_name": srv_service_name, "pool_class": pool_class, @@ -874,7 +875,7 @@ def __init__( ) if not is_srv: - self._init_based_on_options(seeds, srv_max_hosts, srv_service_name) + self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) self._opened = False self._closed = False @@ -1205,6 +1206,18 @@ def topology_description(self) -> TopologyDescription: .. versionadded:: 4.0 """ + if self._topology is None: + servers = { + (host, self._port): ServerDescription((host, self._port)) for host in self._seeds + } + return TopologyDescription( + TOPOLOGY_TYPE.Unknown, + servers, + None, + None, + None, + TopologySettings(), + ) return self._topology.description @property @@ -1218,6 +1231,8 @@ def nodes(self) -> FrozenSet[_Address]: to any servers, or a network partition causes it to lose connection to all servers. """ + if self._topology is None: + return frozenset() description = self._topology.description return frozenset(s.address for s in description.known_servers) @@ -1576,6 +1591,8 @@ async def address(self) -> Optional[tuple[str, int]]: .. versionadded:: 3.0 """ + if self._topology is None: + await self._get_topology() topology_type = self._topology._description.topology_type if ( topology_type == TOPOLOGY_TYPE.Sharded @@ -1598,6 +1615,8 @@ async def primary(self) -> Optional[tuple[str, int]]: .. versionadded:: 3.0 AsyncMongoClient gained this property in version 3.0. """ + if self._topology is None: + await self._get_topology() return await self._topology.get_primary() # type: ignore[return-value] @property @@ -1611,6 +1630,8 @@ async def secondaries(self) -> set[_Address]: .. versionadded:: 3.0 AsyncMongoClient gained this property in version 3.0. """ + if self._topology is None: + await self._get_topology() return await self._topology.get_secondaries() @property @@ -1621,6 +1642,8 @@ async def arbiters(self) -> set[_Address]: connected to a replica set, there are no arbiters, or this client was created without the `replicaSet` option. """ + if self._topology is None: + await self._get_topology() return await self._topology.get_arbiters() @property diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 1c0adb5d6b..21368f2c05 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -101,6 +101,7 @@ ) from pymongo.read_preferences import ReadPreference, _ServerMode from pymongo.results import ClientBulkWriteResult +from pymongo.server_description import ServerDescription from pymongo.server_selectors import writable_server_selector from pymongo.server_type import SERVER_TYPE from pymongo.synchronous import client_session, database, uri_parser @@ -777,7 +778,7 @@ def __init__( keyword_opts["document_class"] = doc_class self._resolve_srv_info: dict[str, Any] = {"keyword_opts": keyword_opts} - seeds = set() + self._seeds = set() is_srv = False username = None password = None @@ -802,18 +803,18 @@ def __init__( srv_max_hosts=srv_max_hosts, ) is_srv = entity.startswith(SRV_SCHEME) - seeds.update(res["nodelist"]) + self._seeds.update(res["nodelist"]) username = res["username"] or username password = res["password"] or password dbase = res["database"] or dbase opts = res["options"] fqdn = res["fqdn"] else: - seeds.update(split_hosts(entity, self._port)) - if not seeds: + self._seeds.update(split_hosts(entity, self._port)) + if not self._seeds: raise ConfigurationError("need to specify at least one host") - for hostname in [node[0] for node in seeds]: + for hostname in [node[0] for node in self._seeds]: if _detect_external_db(hostname): break @@ -836,7 +837,7 @@ def __init__( srv_service_name = opts.get("srvServiceName", common.SRV_SERVICE_NAME) srv_max_hosts = srv_max_hosts or opts.get("srvmaxhosts") - opts = self._normalize_and_validate_options(opts, seeds) + opts = self._normalize_and_validate_options(opts, self._seeds) # Username and password passed as kwargs override user info in URI. username = opts.get("username", username) @@ -855,7 +856,7 @@ def __init__( "username": username, "password": password, "dbase": dbase, - "seeds": seeds, + "seeds": self._seeds, "fqdn": fqdn, "srv_service_name": srv_service_name, "pool_class": pool_class, @@ -872,7 +873,7 @@ def __init__( ) if not is_srv: - self._init_based_on_options(seeds, srv_max_hosts, srv_service_name) + self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) self._opened = False self._closed = False @@ -1203,6 +1204,18 @@ def topology_description(self) -> TopologyDescription: .. versionadded:: 4.0 """ + if self._topology is None: + servers = { + (host, self._port): ServerDescription((host, self._port)) for host in self._seeds + } + return TopologyDescription( + TOPOLOGY_TYPE.Unknown, + servers, + None, + None, + None, + TopologySettings(), + ) return self._topology.description @property @@ -1216,6 +1229,8 @@ def nodes(self) -> FrozenSet[_Address]: to any servers, or a network partition causes it to lose connection to all servers. """ + if self._topology is None: + return frozenset() description = self._topology.description return frozenset(s.address for s in description.known_servers) @@ -1570,6 +1585,8 @@ def address(self) -> Optional[tuple[str, int]]: .. versionadded:: 3.0 """ + if self._topology is None: + self._get_topology() topology_type = self._topology._description.topology_type if ( topology_type == TOPOLOGY_TYPE.Sharded @@ -1592,6 +1609,8 @@ def primary(self) -> Optional[tuple[str, int]]: .. versionadded:: 3.0 MongoClient gained this property in version 3.0. """ + if self._topology is None: + self._get_topology() return self._topology.get_primary() # type: ignore[return-value] @property @@ -1605,6 +1624,8 @@ def secondaries(self) -> set[_Address]: .. versionadded:: 3.0 MongoClient gained this property in version 3.0. """ + if self._topology is None: + self._get_topology() return self._topology.get_secondaries() @property @@ -1615,6 +1636,8 @@ def arbiters(self) -> set[_Address]: connected to a replica set, there are no arbiters, or this client was created without the `replicaSet` option. """ + if self._topology is None: + self._get_topology() return self._topology.get_arbiters() @property diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index c9cfca81fc..a447892e67 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -816,6 +816,30 @@ async def test_constants(self): async def test_init_disconnected(self): host, port = await async_client_context.host, await async_client_context.port c = await self.async_rs_or_single_client(connect=False) + # nodes returns an empty set if not connected + self.assertEqual(c.nodes, frozenset()) + # topology_description returns the initial seed description if not connected + topology_description = c.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + topology_description.server_descriptions(), + {(host, port): ServerDescription((host, port))}, + ) + # address causes client to block until connected + self.assertIsNotNone(await c.address) + c = await self.async_rs_or_single_client(connect=False) + # primary causes client to block until connected + await c.primary + self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) + # secondaries causes client to block until connected + await c.secondaries + self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) + # arbiters causes client to block until connected + await c.arbiters + self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(await c.is_primary, bool) c = await self.async_rs_or_single_client(connect=False) @@ -2170,6 +2194,18 @@ async def test_uuid_queries(self): self.assertEqual(2, len(docs)) await coll.drop() + async def test_unconnected_client_properties_with_srv(self): + client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False) + self.assertEqual(client.nodes, frozenset()) + topology_description = client.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + topology_description.server_descriptions(), + {("unknown", None): ServerDescription(("unknown", None))}, + ) + await client.aconnect() + self.assertEqual(await client.address, None) + class TestExhaustCursor(AsyncIntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" diff --git a/test/test_client.py b/test/test_client.py index 038ba2241b..98711c9517 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -791,6 +791,30 @@ def test_constants(self): def test_init_disconnected(self): host, port = client_context.host, client_context.port c = self.rs_or_single_client(connect=False) + # nodes returns an empty set if not connected + self.assertEqual(c.nodes, frozenset()) + # topology_description returns the initial seed description if not connected + topology_description = c.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + topology_description.server_descriptions(), + {(host, port): ServerDescription((host, port))}, + ) + # address causes client to block until connected + self.assertIsNotNone(c.address) + c = self.rs_or_single_client(connect=False) + # primary causes client to block until connected + c.primary + self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) + # secondaries causes client to block until connected + c.secondaries + self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) + # arbiters causes client to block until connected + c.arbiters + self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(c.is_primary, bool) c = self.rs_or_single_client(connect=False) @@ -2127,6 +2151,18 @@ def test_uuid_queries(self): self.assertEqual(2, len(docs)) coll.drop() + def test_unconnected_client_properties_with_srv(self): + client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False) + self.assertEqual(client.nodes, frozenset()) + topology_description = client.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + topology_description.server_descriptions(), + {("unknown", None): ServerDescription(("unknown", None))}, + ) + client._connect() + self.assertEqual(client.address, None) + class TestExhaustCursor(IntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" From c185c9563979e3611eace1b9d16f7f42ca0dccae Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Wed, 16 Apr 2025 10:19:59 -0400 Subject: [PATCH 02/10] Address review --- pymongo/asynchronous/mongo_client.py | 5 ++++- pymongo/asynchronous/settings.py | 7 +++++-- pymongo/synchronous/mongo_client.py | 5 ++++- pymongo/synchronous/settings.py | 7 +++++-- test/asynchronous/test_client.py | 21 +++++++++------------ test/test_client.py | 21 +++++++++------------ 6 files changed, 36 insertions(+), 30 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 6771266f1c..a0e0978081 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -768,6 +768,7 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None + self._initial_topology_id: Optional[ObjectId] = None # type: ignore[assignment] # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. @@ -976,6 +977,7 @@ def _init_based_on_options( srv_service_name=srv_service_name, srv_max_hosts=srv_max_hosts, server_monitoring_mode=self._options.server_monitoring_mode, + topology_id=self._initial_topology_id, ) if self._options.auto_encryption_opts: from pymongo.asynchronous.encryption import _Encrypter @@ -1210,7 +1212,7 @@ def topology_description(self) -> TopologyDescription: servers = { (host, self._port): ServerDescription((host, self._port)) for host in self._seeds } - return TopologyDescription( + td = TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, None, @@ -1218,6 +1220,7 @@ def topology_description(self) -> TopologyDescription: None, TopologySettings(), ) + self._initial_topology_id = td._topology_settings._topology_id return self._topology.description @property diff --git a/pymongo/asynchronous/settings.py b/pymongo/asynchronous/settings.py index 62be853fba..9c2331971a 100644 --- a/pymongo/asynchronous/settings.py +++ b/pymongo/asynchronous/settings.py @@ -51,6 +51,7 @@ def __init__( srv_service_name: str = common.SRV_SERVICE_NAME, srv_max_hosts: int = 0, server_monitoring_mode: str = common.SERVER_MONITORING_MODE, + topology_id: Optional[ObjectId] = None, ): """Represent MongoClient's configuration. @@ -78,8 +79,10 @@ def __init__( self._srv_service_name = srv_service_name self._srv_max_hosts = srv_max_hosts or 0 self._server_monitoring_mode = server_monitoring_mode - - self._topology_id = ObjectId() + if topology_id is not None: + self._topology_id = topology_id + else: + self._topology_id = ObjectId() # Store the allocation traceback to catch unclosed clients in the # test suite. self._stack = "".join(traceback.format_stack()[:-2]) diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 21368f2c05..97aef085b2 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -766,6 +766,7 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None + self._initial_topology_id: Optional[ObjectId] = None # type: ignore[assignment] # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. @@ -974,6 +975,7 @@ def _init_based_on_options( srv_service_name=srv_service_name, srv_max_hosts=srv_max_hosts, server_monitoring_mode=self._options.server_monitoring_mode, + topology_id=self._initial_topology_id, ) if self._options.auto_encryption_opts: from pymongo.synchronous.encryption import _Encrypter @@ -1208,7 +1210,7 @@ def topology_description(self) -> TopologyDescription: servers = { (host, self._port): ServerDescription((host, self._port)) for host in self._seeds } - return TopologyDescription( + td = TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, None, @@ -1216,6 +1218,7 @@ def topology_description(self) -> TopologyDescription: None, TopologySettings(), ) + self._initial_topology_id = td._topology_settings._topology_id return self._topology.description @property diff --git a/pymongo/synchronous/settings.py b/pymongo/synchronous/settings.py index bb17de1874..61b86fa18d 100644 --- a/pymongo/synchronous/settings.py +++ b/pymongo/synchronous/settings.py @@ -51,6 +51,7 @@ def __init__( srv_service_name: str = common.SRV_SERVICE_NAME, srv_max_hosts: int = 0, server_monitoring_mode: str = common.SERVER_MONITORING_MODE, + topology_id: Optional[ObjectId] = None, ): """Represent MongoClient's configuration. @@ -78,8 +79,10 @@ def __init__( self._srv_service_name = srv_service_name self._srv_max_hosts = srv_max_hosts or 0 self._server_monitoring_mode = server_monitoring_mode - - self._topology_id = ObjectId() + if topology_id is not None: + self._topology_id = topology_id + else: + self._topology_id = ObjectId() # Store the allocation traceback to catch unclosed clients in the # test suite. self._stack = "".join(traceback.format_stack()[:-2]) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index a447892e67..e664dcbf9f 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -825,20 +825,29 @@ async def test_init_disconnected(self): topology_description.server_descriptions(), {(host, port): ServerDescription((host, port))}, ) + # address causes client to block until connected self.assertIsNotNone(await c.address) + # Initial seed topology and connected topology have the same ID + self.assertEqual( + c._topology._topology_id, topology_description._topology_settings._topology_id + ) + c = await self.async_rs_or_single_client(connect=False) # primary causes client to block until connected await c.primary self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) # secondaries causes client to block until connected await c.secondaries self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) # arbiters causes client to block until connected await c.arbiters self.assertIsNotNone(c._topology) + c = await self.async_rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(await c.is_primary, bool) @@ -2194,18 +2203,6 @@ async def test_uuid_queries(self): self.assertEqual(2, len(docs)) await coll.drop() - async def test_unconnected_client_properties_with_srv(self): - client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False) - self.assertEqual(client.nodes, frozenset()) - topology_description = client.topology_description - self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) - self.assertEqual( - topology_description.server_descriptions(), - {("unknown", None): ServerDescription(("unknown", None))}, - ) - await client.aconnect() - self.assertEqual(await client.address, None) - class TestExhaustCursor(AsyncIntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" diff --git a/test/test_client.py b/test/test_client.py index 98711c9517..cc78868dd5 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -800,20 +800,29 @@ def test_init_disconnected(self): topology_description.server_descriptions(), {(host, port): ServerDescription((host, port))}, ) + # address causes client to block until connected self.assertIsNotNone(c.address) + # Initial seed topology and connected topology have the same ID + self.assertEqual( + c._topology._topology_id, topology_description._topology_settings._topology_id + ) + c = self.rs_or_single_client(connect=False) # primary causes client to block until connected c.primary self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) # secondaries causes client to block until connected c.secondaries self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) # arbiters causes client to block until connected c.arbiters self.assertIsNotNone(c._topology) + c = self.rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(c.is_primary, bool) @@ -2151,18 +2160,6 @@ def test_uuid_queries(self): self.assertEqual(2, len(docs)) coll.drop() - def test_unconnected_client_properties_with_srv(self): - client = self.simple_client("mongodb+srv://test1.test.build.10gen.cc/", connect=False) - self.assertEqual(client.nodes, frozenset()) - topology_description = client.topology_description - self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) - self.assertEqual( - topology_description.server_descriptions(), - {("unknown", None): ServerDescription(("unknown", None))}, - ) - client._connect() - self.assertEqual(client.address, None) - class TestExhaustCursor(IntegrationTest): """Test that clients properly handle errors from exhaust cursors.""" From 66b7e8d4ffd6110b1a0c67d2092e7493bc699f6d Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Wed, 16 Apr 2025 14:31:29 -0400 Subject: [PATCH 03/10] Fix topology_description --- pymongo/asynchronous/mongo_client.py | 1 + pymongo/synchronous/mongo_client.py | 1 + 2 files changed, 2 insertions(+) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index a0e0978081..2bde416abb 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -1221,6 +1221,7 @@ def topology_description(self) -> TopologyDescription: TopologySettings(), ) self._initial_topology_id = td._topology_settings._topology_id + return td return self._topology.description @property diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 97aef085b2..47213b3f45 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -1219,6 +1219,7 @@ def topology_description(self) -> TopologyDescription: TopologySettings(), ) self._initial_topology_id = td._topology_settings._topology_id + return td return self._topology.description @property From 5f7f84ad1bc156f487f8c4a38f79514a462a5e1b Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Wed, 16 Apr 2025 16:36:49 -0400 Subject: [PATCH 04/10] Fix SRV test --- pymongo/asynchronous/mongo_client.py | 4 +- pymongo/synchronous/mongo_client.py | 4 +- test/asynchronous/test_client.py | 81 ++++++++++++++++------------ test/test_client.py | 81 ++++++++++++++++------------ 4 files changed, 98 insertions(+), 72 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 2bde416abb..26b9fe055b 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -1209,9 +1209,7 @@ def topology_description(self) -> TopologyDescription: .. versionadded:: 4.0 """ if self._topology is None: - servers = { - (host, self._port): ServerDescription((host, self._port)) for host in self._seeds - } + servers = {(host, port): ServerDescription((host, port)) for host, port in self._seeds} td = TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 47213b3f45..2f84405ab7 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -1207,9 +1207,7 @@ def topology_description(self) -> TopologyDescription: .. versionadded:: 4.0 """ if self._topology is None: - servers = { - (host, self._port): ServerDescription((host, self._port)) for host in self._seeds - } + servers = {(host, port): ServerDescription((host, port)) for host, port in self._seeds} td = TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index e664dcbf9f..5c68674eff 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -815,39 +815,6 @@ async def test_constants(self): async def test_init_disconnected(self): host, port = await async_client_context.host, await async_client_context.port - c = await self.async_rs_or_single_client(connect=False) - # nodes returns an empty set if not connected - self.assertEqual(c.nodes, frozenset()) - # topology_description returns the initial seed description if not connected - topology_description = c.topology_description - self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) - self.assertEqual( - topology_description.server_descriptions(), - {(host, port): ServerDescription((host, port))}, - ) - - # address causes client to block until connected - self.assertIsNotNone(await c.address) - # Initial seed topology and connected topology have the same ID - self.assertEqual( - c._topology._topology_id, topology_description._topology_settings._topology_id - ) - - c = await self.async_rs_or_single_client(connect=False) - # primary causes client to block until connected - await c.primary - self.assertIsNotNone(c._topology) - - c = await self.async_rs_or_single_client(connect=False) - # secondaries causes client to block until connected - await c.secondaries - self.assertIsNotNone(c._topology) - - c = await self.async_rs_or_single_client(connect=False) - # arbiters causes client to block until connected - await c.arbiters - self.assertIsNotNone(c._topology) - c = await self.async_rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(await c.is_primary, bool) @@ -882,6 +849,54 @@ async def test_init_disconnected_with_auth(self): with self.assertRaises(ConnectionFailure): await c.pymongo_test.test.find_one() + @async_client_context.require_replica_set + @async_client_context.require_tls + async def test_init_disconnected_with_srv(self): + c = await self.async_rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # nodes returns an empty set if not connected + self.assertEqual(c.nodes, frozenset()) + # topology_description returns the initial seed description if not connected + topology_description = c.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + { + ("test1.test.build.10gen.cc", None): ServerDescription( + ("test1.test.build.10gen.cc", None) + ) + }, + topology_description.server_descriptions(), + ) + + # address causes client to block until connected + self.assertIsNotNone(await c.address) + # Initial seed topology and connected topology have the same ID + self.assertEqual( + c._topology._topology_id, topology_description._topology_settings._topology_id + ) + + c = await self.async_rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # primary causes client to block until connected + await c.primary + self.assertIsNotNone(c._topology) + + c = await self.async_rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # secondaries causes client to block until connected + await c.secondaries + self.assertIsNotNone(c._topology) + + c = await self.async_rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # arbiters causes client to block until connected + await c.arbiters + self.assertIsNotNone(c._topology) + async def test_equality(self): seed = "{}:{}".format(*list(self.client._topology_settings.seeds)[0]) c = await self.async_rs_or_single_client(seed, connect=False) diff --git a/test/test_client.py b/test/test_client.py index cc78868dd5..42aa3c0e3d 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -790,39 +790,6 @@ def test_constants(self): def test_init_disconnected(self): host, port = client_context.host, client_context.port - c = self.rs_or_single_client(connect=False) - # nodes returns an empty set if not connected - self.assertEqual(c.nodes, frozenset()) - # topology_description returns the initial seed description if not connected - topology_description = c.topology_description - self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) - self.assertEqual( - topology_description.server_descriptions(), - {(host, port): ServerDescription((host, port))}, - ) - - # address causes client to block until connected - self.assertIsNotNone(c.address) - # Initial seed topology and connected topology have the same ID - self.assertEqual( - c._topology._topology_id, topology_description._topology_settings._topology_id - ) - - c = self.rs_or_single_client(connect=False) - # primary causes client to block until connected - c.primary - self.assertIsNotNone(c._topology) - - c = self.rs_or_single_client(connect=False) - # secondaries causes client to block until connected - c.secondaries - self.assertIsNotNone(c._topology) - - c = self.rs_or_single_client(connect=False) - # arbiters causes client to block until connected - c.arbiters - self.assertIsNotNone(c._topology) - c = self.rs_or_single_client(connect=False) # is_primary causes client to block until connected self.assertIsInstance(c.is_primary, bool) @@ -857,6 +824,54 @@ def test_init_disconnected_with_auth(self): with self.assertRaises(ConnectionFailure): c.pymongo_test.test.find_one() + @client_context.require_replica_set + @client_context.require_tls + def test_init_disconnected_with_srv(self): + c = self.rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # nodes returns an empty set if not connected + self.assertEqual(c.nodes, frozenset()) + # topology_description returns the initial seed description if not connected + topology_description = c.topology_description + self.assertEqual(topology_description.topology_type, TOPOLOGY_TYPE.Unknown) + self.assertEqual( + { + ("test1.test.build.10gen.cc", None): ServerDescription( + ("test1.test.build.10gen.cc", None) + ) + }, + topology_description.server_descriptions(), + ) + + # address causes client to block until connected + self.assertIsNotNone(c.address) + # Initial seed topology and connected topology have the same ID + self.assertEqual( + c._topology._topology_id, topology_description._topology_settings._topology_id + ) + + c = self.rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # primary causes client to block until connected + c.primary + self.assertIsNotNone(c._topology) + + c = self.rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # secondaries causes client to block until connected + c.secondaries + self.assertIsNotNone(c._topology) + + c = self.rs_or_single_client( + "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True + ) + # arbiters causes client to block until connected + c.arbiters + self.assertIsNotNone(c._topology) + def test_equality(self): seed = "{}:{}".format(*list(self.client._topology_settings.seeds)[0]) c = self.rs_or_single_client(seed, connect=False) From ed3dea5f072023990611624fe47071827e9a6cb9 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Thu, 17 Apr 2025 10:19:29 -0400 Subject: [PATCH 05/10] Remove type: ignore --- pymongo/asynchronous/mongo_client.py | 2 +- pymongo/synchronous/mongo_client.py | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 26b9fe055b..40f5420e8c 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -768,7 +768,7 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None - self._initial_topology_id: Optional[ObjectId] = None # type: ignore[assignment] + self._initial_topology_id: Optional[ObjectId] = None # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 2f84405ab7..7ebd3b9d4e 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -766,7 +766,7 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None - self._initial_topology_id: Optional[ObjectId] = None # type: ignore[assignment] + self._initial_topology_id: Optional[ObjectId] = None # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. From 99802beee3b1b8333b4adc0ee9dc85e852aff30c Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Fri, 18 Apr 2025 10:33:26 -0400 Subject: [PATCH 06/10] Address review --- test/asynchronous/test_client.py | 6 +++++- test/test_client.py | 6 +++++- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/test/asynchronous/test_client.py b/test/asynchronous/test_client.py index 5c68674eff..b278d684cb 100644 --- a/test/asynchronous/test_client.py +++ b/test/asynchronous/test_client.py @@ -849,7 +849,8 @@ async def test_init_disconnected_with_auth(self): with self.assertRaises(ConnectionFailure): await c.pymongo_test.test.find_one() - @async_client_context.require_replica_set + @async_client_context.require_no_standalone + @async_client_context.require_no_load_balancer @async_client_context.require_tls async def test_init_disconnected_with_srv(self): c = await self.async_rs_or_single_client( @@ -875,6 +876,7 @@ async def test_init_disconnected_with_srv(self): self.assertEqual( c._topology._topology_id, topology_description._topology_settings._topology_id ) + await c.close() c = await self.async_rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True @@ -882,6 +884,7 @@ async def test_init_disconnected_with_srv(self): # primary causes client to block until connected await c.primary self.assertIsNotNone(c._topology) + await c.close() c = await self.async_rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True @@ -889,6 +892,7 @@ async def test_init_disconnected_with_srv(self): # secondaries causes client to block until connected await c.secondaries self.assertIsNotNone(c._topology) + await c.close() c = await self.async_rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True diff --git a/test/test_client.py b/test/test_client.py index 42aa3c0e3d..8a50c90afb 100644 --- a/test/test_client.py +++ b/test/test_client.py @@ -824,7 +824,8 @@ def test_init_disconnected_with_auth(self): with self.assertRaises(ConnectionFailure): c.pymongo_test.test.find_one() - @client_context.require_replica_set + @client_context.require_no_standalone + @client_context.require_no_load_balancer @client_context.require_tls def test_init_disconnected_with_srv(self): c = self.rs_or_single_client( @@ -850,6 +851,7 @@ def test_init_disconnected_with_srv(self): self.assertEqual( c._topology._topology_id, topology_description._topology_settings._topology_id ) + c.close() c = self.rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True @@ -857,6 +859,7 @@ def test_init_disconnected_with_srv(self): # primary causes client to block until connected c.primary self.assertIsNotNone(c._topology) + c.close() c = self.rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True @@ -864,6 +867,7 @@ def test_init_disconnected_with_srv(self): # secondaries causes client to block until connected c.secondaries self.assertIsNotNone(c._topology) + c.close() c = self.rs_or_single_client( "mongodb+srv://test1.test.build.10gen.cc", connect=False, tlsInsecure=True From 14bb119e56e75d857ee051fd829d8cae6a59b020 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Fri, 18 Apr 2025 14:38:39 -0400 Subject: [PATCH 07/10] Create TopologySettings always --- pymongo/asynchronous/mongo_client.py | 12 ++++-------- pymongo/synchronous/mongo_client.py | 12 ++++-------- 2 files changed, 8 insertions(+), 16 deletions(-) diff --git a/pymongo/asynchronous/mongo_client.py b/pymongo/asynchronous/mongo_client.py index 40f5420e8c..a236b21348 100644 --- a/pymongo/asynchronous/mongo_client.py +++ b/pymongo/asynchronous/mongo_client.py @@ -768,7 +768,6 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None - self._initial_topology_id: Optional[ObjectId] = None # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. @@ -875,8 +874,7 @@ def __init__( self._options.read_concern, ) - if not is_srv: - self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) + self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) self._opened = False self._closed = False @@ -977,7 +975,7 @@ def _init_based_on_options( srv_service_name=srv_service_name, srv_max_hosts=srv_max_hosts, server_monitoring_mode=self._options.server_monitoring_mode, - topology_id=self._initial_topology_id, + topology_id=self._topology_settings._topology_id if self._topology_settings else None, ) if self._options.auto_encryption_opts: from pymongo.asynchronous.encryption import _Encrypter @@ -1210,16 +1208,14 @@ def topology_description(self) -> TopologyDescription: """ if self._topology is None: servers = {(host, port): ServerDescription((host, port)) for host, port in self._seeds} - td = TopologyDescription( + return TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, None, None, None, - TopologySettings(), + self._topology_settings, ) - self._initial_topology_id = td._topology_settings._topology_id - return td return self._topology.description @property diff --git a/pymongo/synchronous/mongo_client.py b/pymongo/synchronous/mongo_client.py index 7ebd3b9d4e..99a517e5c1 100644 --- a/pymongo/synchronous/mongo_client.py +++ b/pymongo/synchronous/mongo_client.py @@ -766,7 +766,6 @@ def __init__( self._timeout: float | None = None self._topology_settings: TopologySettings = None # type: ignore[assignment] self._event_listeners: _EventListeners | None = None - self._initial_topology_id: Optional[ObjectId] = None # _pool_class, _monitor_class, and _condition_class are for deep # customization of PyMongo, e.g. Motor. @@ -873,8 +872,7 @@ def __init__( self._options.read_concern, ) - if not is_srv: - self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) + self._init_based_on_options(self._seeds, srv_max_hosts, srv_service_name) self._opened = False self._closed = False @@ -975,7 +973,7 @@ def _init_based_on_options( srv_service_name=srv_service_name, srv_max_hosts=srv_max_hosts, server_monitoring_mode=self._options.server_monitoring_mode, - topology_id=self._initial_topology_id, + topology_id=self._topology_settings._topology_id if self._topology_settings else None, ) if self._options.auto_encryption_opts: from pymongo.synchronous.encryption import _Encrypter @@ -1208,16 +1206,14 @@ def topology_description(self) -> TopologyDescription: """ if self._topology is None: servers = {(host, port): ServerDescription((host, port)) for host, port in self._seeds} - td = TopologyDescription( + return TopologyDescription( TOPOLOGY_TYPE.Unknown, servers, None, None, None, - TopologySettings(), + self._topology_settings, ) - self._initial_topology_id = td._topology_settings._topology_id - return td return self._topology.description @property From c1e6684d88a5c7ec8460ffbddffebc6b1aac26f1 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 21 Apr 2025 08:43:12 -0400 Subject: [PATCH 08/10] Update changelog --- doc/changelog.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/doc/changelog.rst b/doc/changelog.rst index 597f75f873..c9c02821a6 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,6 +10,7 @@ Version 4.12.1 is a bug fix release. - Fixed a bug that could raise ``UnboundLocalError`` when creating asynchronous connections over SSL. - Fixed a bug causing SRV hostname validation to fail when resolver and resolved hostnames are identical with three domain levels. - Fixed a bug that caused direct use of ``pymongo.uri_parser`` to raise an ``AttributeError``. +- Fixed a bug that could cause public ``pymongo.MongoClient`` and ``pymongo.AsyncMongoClient`` attributes to raise errors when accessed before a connection was established. Issues Resolved ............... From 835e56077484bdca832c7cc7ad76c8da615c211f Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 21 Apr 2025 14:25:15 -0400 Subject: [PATCH 09/10] Update changelog --- doc/changelog.rst | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 2d35dc8330..5d7805a5ae 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -10,11 +10,13 @@ Version 4.12.1 is a bug fix release. - Fixed a bug that could raise ``UnboundLocalError`` when creating asynchronous connections over SSL. - Fixed a bug causing SRV hostname validation to fail when resolver and resolved hostnames are identical with three domain levels. - Fixed a bug that caused direct use of ``pymongo.uri_parser`` to raise an ``AttributeError``. -- Fixed a bug that could cause public ``pymongo.MongoClient`` and ``pymongo.AsyncMongoClient`` attributes to raise errors when accessed before a connection was established. +- Fixed a bug where clients created with connect=False and a "mongodb+srv://" connection string + could cause public ``pymongo.MongoClient`` and ``pymongo.AsyncMongoClient`` attributes (topology_description, + nodes, address, primary, secondaries, arbiters) to incorrectly return a Database, leading to type + errors such as: "NotImplementedError: Database objects do not implement truth value testing or bool(). - Removed Eventlet testing against Python versions newer than 3.9 since Eventlet is actively being sunset by its maintainers and has compatibility issues with PyMongo's dnspython dependency. - Issues Resolved ............... From e5e6dc82d60e2c7ae93510229c4cbcace9720257 Mon Sep 17 00:00:00 2001 From: Noah Stapp Date: Mon, 21 Apr 2025 15:47:06 -0400 Subject: [PATCH 10/10] Trailing quote --- doc/changelog.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/changelog.rst b/doc/changelog.rst index 5d7805a5ae..4fff06c9cb 100644 --- a/doc/changelog.rst +++ b/doc/changelog.rst @@ -13,7 +13,7 @@ Version 4.12.1 is a bug fix release. - Fixed a bug where clients created with connect=False and a "mongodb+srv://" connection string could cause public ``pymongo.MongoClient`` and ``pymongo.AsyncMongoClient`` attributes (topology_description, nodes, address, primary, secondaries, arbiters) to incorrectly return a Database, leading to type - errors such as: "NotImplementedError: Database objects do not implement truth value testing or bool(). + errors such as: "NotImplementedError: Database objects do not implement truth value testing or bool()". - Removed Eventlet testing against Python versions newer than 3.9 since Eventlet is actively being sunset by its maintainers and has compatibility issues with PyMongo's dnspython dependency.